[{"id":32058,"web_url":"https://patchwork.libcamera.org/comment/32058/","msgid":"<173090878877.3353069.6283118074718503842@ping.linuxembedded.co.uk>","date":"2024-11-06T15:59:48","subject":"Re: [PATCH v1] libcamera: ipc_unixsocket: Share stdin and stdout\n\twith IPA proxy","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Julien,\n\nQuoting Julien Vuillaumier (2024-11-06 15:43:06)\n> When IPA is running in isolated mode, at IPA process creation time\n> all the inherited file descriptors are closed in the new process\n> after the fork, with the exception of the file descriptor relevant\n> to the IPC peer Unix socket.\n> \n> In order to enable the IPA logging in the console, add stdout and\n> stderr to the list of file descriptors to be shared with the\n> isolated process.\n> \n> Signed-off-by: Julien Vuillaumier \n> ---\n> src/libcamera/ipc_pipe_unixsocket.cpp | 5 +++++\n> 1 file changed, 5 insertions(+)\n> \n> diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp\n> index 668ec73b..9ca219dc 100644\n> --- a/src/libcamera/ipc_pipe_unixsocket.cpp\n> +++ b/src/libcamera/ipc_pipe_unixsocket.cpp\n> @@ -7,6 +7,7 @@\n> \n> #include \"libcamera/internal/ipc_pipe_unixsocket.h\"\n> \n> +#include \n> #include \n> \n> #include \n> @@ -42,6 +43,10 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,\n> args.push_back(std::to_string(fd.get()));\n> fds.push_back(fd.get());\n> \n> + /* Share stdout and stderr with the proxy for logging purpose */\n> + fds.push_back(STDOUT_FILENO);\n> + fds.push_back(STDERR_FILENO);\n> +\n\nIs this all that's required ? Is there a corresponding change in the\nproxy to tie the spawned process stdout/stderr to these fds ?\n\n--\nKieran\n\n\n> proc_ = std::make_unique();\n> int ret = proc_->start(ipaProxyWorkerPath, args, fds);\n> if (ret) {\n> -- \n> 2.34.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7A5D4BE173\n\tfor ;\n\tWed, 6 Nov 2024 15:59:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8380665431;\n\tWed, 6 Nov 2024 16:59:53 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A9FB8653C5\n\tfor ;\n\tWed, 6 Nov 2024 16:59:51 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5B801475;\n\tWed, 6 Nov 2024 16:59:43 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bHrQcIKz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730908783;\n\tbh=5xpJXRJEW4vpBwY1qTTbBqKJG3nHwQ+VCz6e5GSyyPg=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=bHrQcIKzrPDnuLo6qTZKyUuIR+eeCUAWNwGVWo8CRwOzu5MKlklmE9+pxPA6GRu8f\n\tKdgC7e4RCJOMDT5Iw0LEk3lzSUxtHRKPNbZETUZEMtOnGv9Wzsb1C6wmmpsNEadIld\n\th5k7wEFNAvy5KSzlSD1BaHfUaozAUvWtmpG5DDyk=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241106154306.2490129-1-julien.vuillaumier@nxp.com>","References":"<20241106154306.2490129-1-julien.vuillaumier@nxp.com>","Subject":"Re: [PATCH v1] libcamera: ipc_unixsocket: Share stdin and stdout\n\twith IPA proxy","From":"Kieran Bingham ","Cc":"Julien Vuillaumier ","To":"Julien Vuillaumier ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 06 Nov 2024 15:59:48 +0000","Message-ID":"<173090878877.3353069.6283118074718503842@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32059,"web_url":"https://patchwork.libcamera.org/comment/32059/","msgid":"<173090916100.3353069.17815553024488669502@ping.linuxembedded.co.uk>","date":"2024-11-06T16:06:01","subject":"Re: [PATCH v1] libcamera: ipc_unixsocket: Share stdin and stdout\n\twith IPA proxy","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2024-11-06 15:59:48)\n> Hi Julien,\n> \n> Quoting Julien Vuillaumier (2024-11-06 15:43:06)\n> > When IPA is running in isolated mode, at IPA process creation time\n> > all the inherited file descriptors are closed in the new process\n> > after the fork, with the exception of the file descriptor relevant\n> > to the IPC peer Unix socket.\n> > \n> > In order to enable the IPA logging in the console, add stdout and\n> > stderr to the list of file descriptors to be shared with the\n> > isolated process.\n> > \n> > Signed-off-by: Julien Vuillaumier \n> > ---\n> > src/libcamera/ipc_pipe_unixsocket.cpp | 5 +++++\n> > 1 file changed, 5 insertions(+)\n> > \n> > diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp\n> > index 668ec73b..9ca219dc 100644\n> > --- a/src/libcamera/ipc_pipe_unixsocket.cpp\n> > +++ b/src/libcamera/ipc_pipe_unixsocket.cpp\n> > @@ -7,6 +7,7 @@\n> > \n> > #include \"libcamera/internal/ipc_pipe_unixsocket.h\"\n> > \n> > +#include \n> > #include \n> > \n> > #include \n> > @@ -42,6 +43,10 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,\n> > args.push_back(std::to_string(fd.get()));\n> > fds.push_back(fd.get());\n> > \n> > + /* Share stdout and stderr with the proxy for logging purpose */\n> > + fds.push_back(STDOUT_FILENO);\n> > + fds.push_back(STDERR_FILENO);\n> > +\n> \n> Is this all that's required ? Is there a corresponding change in the\n> proxy to tie the spawned process stdout/stderr to these fds ?\n\nAha! So now I've dug deeper, indeed I see this is all that's required...\n\nThe proc_->start() calls:\n\n\tcloseAllFdsExcept(fds);\n\nSo - by adding stdout,stderr here we keep those mapped...\n\nWhich makes this ... really quite nice, simple and elegant.\n\nRight now - I don't have any reasons why we couldn't do this. I'm\ncurious to know what security implications might come up from allowing\nstdout/stderr to be written to from a potentially closed source binary\n\n... but aside from flooding a log - I can't imagine anything worse than\nthat right now ? (cue someone telling me about vulnerabilities in\nterminal emulators for some carefully crafted character sequence to\ndelete the harddrive ... in 3 ... 2 ... 1 ... )\n\nThere are already many occasions where this would have made my life much\neasier or things easier to diagnose when users had incorrectly\nconfigured their libcamera build such that the IPA became\nunnecessarily isolated, and we had no logs to determine why the IPA\ncrashed ...\n\nso for that at least - I'd be happy to see this in:\n\n\nReviewed-by: Kieran Bingham \n\n> \n> --\n> Kieran\n> \n> \n> > proc_ = std::make_unique();\n> > int ret = proc_->start(ipaProxyWorkerPath, args, fds);\n> > if (ret) {\n> > -- \n> > 2.34.1\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A358DBDB13\n\tfor ;\n\tWed, 6 Nov 2024 16:06:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C238D653C5;\n\tWed, 6 Nov 2024 17:06:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3977C653C5\n\tfor ;\n\tWed, 6 Nov 2024 17:06:03 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E913559D;\n\tWed, 6 Nov 2024 17:05:54 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"rYDXngiI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730909155;\n\tbh=2tL167R9GuJyxZQs9kDcIvDV833lKQjJE9w5P7ZaYaY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=rYDXngiItrbdJ0N5X91QktUz0ddMhNdx+ri+M9G+gwssWgtglB/9DPfEev1mlnP7I\n\tqgnONhKYpVxowDmw2ly08hXAU+qU5Iy5NH/RGEcrnA3mgDDQj6mtnzQxJ6VgxCStJY\n\txPR5rIs5PvkT0JP/J+LVlT/j2bWbC8CRJgge8uKA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<173090878877.3353069.6283118074718503842@ping.linuxembedded.co.uk>","References":"<20241106154306.2490129-1-julien.vuillaumier@nxp.com>\n\t<173090878877.3353069.6283118074718503842@ping.linuxembedded.co.uk>","Subject":"Re: [PATCH v1] libcamera: ipc_unixsocket: Share stdin and stdout\n\twith IPA proxy","From":"Kieran Bingham ","Cc":"Julien Vuillaumier ","To":"Julien Vuillaumier ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 06 Nov 2024 16:06:01 +0000","Message-ID":"<173090916100.3353069.17815553024488669502@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32621,"web_url":"https://patchwork.libcamera.org/comment/32621/","msgid":"<20241209093907.GA3076@pendragon.ideasonboard.com>","date":"2024-12-09T09:39:07","subject":"Re: [PATCH v1] libcamera: ipc_unixsocket: Share stdin and stdout\n\twith IPA proxy","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Julien,\n\nThank you for the patch.\n\nOn Wed, Nov 06, 2024 at 04:43:06PM +0100, Julien Vuillaumier wrote:\n> When IPA is running in isolated mode, at IPA process creation time\n> all the inherited file descriptors are closed in the new process\n> after the fork, with the exception of the file descriptor relevant\n> to the IPC peer Unix socket.\n> \n> In order to enable the IPA logging in the console, add stdout and\n> stderr to the list of file descriptors to be shared with the\n> isolated process.\n> \n> Signed-off-by: Julien Vuillaumier \n> ---\n> src/libcamera/ipc_pipe_unixsocket.cpp | 5 +++++\n> 1 file changed, 5 insertions(+)\n> \n> diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp\n> index 668ec73b..9ca219dc 100644\n> --- a/src/libcamera/ipc_pipe_unixsocket.cpp\n> +++ b/src/libcamera/ipc_pipe_unixsocket.cpp\n> @@ -7,6 +7,7 @@\n> \n> #include \"libcamera/internal/ipc_pipe_unixsocket.h\"\n> \n> +#include \n> #include \n> \n> #include \n> @@ -42,6 +43,10 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,\n> \targs.push_back(std::to_string(fd.get()));\n> \tfds.push_back(fd.get());\n> \n> +\t/* Share stdout and stderr with the proxy for logging purpose */\n> +\tfds.push_back(STDOUT_FILENO);\n> +\tfds.push_back(STDERR_FILENO);\n\nThose are the default file descriptors for stdout and stderr, but an\napplication may have remapped those streams, or even closed them. All of\na sudden you may end up writing to a file or any other fd that the\napplication would have opened after closing stderr or stdout.\n\n> +\n> \tproc_ = std::make_unique();\n> \tint ret = proc_->start(ipaProxyWorkerPath, args, fds);\n> \tif (ret) {","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D5081BD80A\n\tfor ;\n\tMon, 9 Dec 2024 09:39:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B8D6B67E4C;\n\tMon, 9 Dec 2024 10:39:24 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DEB6A618AE\n\tfor ;\n\tMon, 9 Dec 2024 10:39:22 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 42AE5502;\n\tMon, 9 Dec 2024 10:38:51 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"SPfIvDGJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1733737131;\n\tbh=QKXFa/Gct7EclQOYn53syH+jV7BC7POpdP1ariC75LM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SPfIvDGJrkRI9mT1tp75SkTyQ+hHXM/esZp7md8WFB7J8bpDKUhGx03P3woIMw5BY\n\tBlE8tzgmafscO4DQlKPSiZBvc0yXLKvlZ8vLgH2OYq2wxYPC/BP7X7NcFI+f/AOgeK\n\thUAJhQduXic3HxvOjYFafc1BAsw2lKqpier0wgQs=","Date":"Mon, 9 Dec 2024 11:39:07 +0200","From":"Laurent Pinchart ","To":"Julien Vuillaumier ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] libcamera: ipc_unixsocket: Share stdin and stdout\n\twith IPA proxy","Message-ID":"<20241209093907.GA3076@pendragon.ideasonboard.com>","References":"<20241106154306.2490129-1-julien.vuillaumier@nxp.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241106154306.2490129-1-julien.vuillaumier@nxp.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32635,"web_url":"https://patchwork.libcamera.org/comment/32635/","msgid":"","date":"2024-12-09T11:33:39","subject":"Re: [PATCH v1] libcamera: ipc_unixsocket: Share stdin and stdout\n\twith IPA proxy","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Wed, Nov 06, 2024 at 04:43:06PM +0100, Julien Vuillaumier wrote:\n> When IPA is running in isolated mode, at IPA process creation time\n> all the inherited file descriptors are closed in the new process\n> after the fork, with the exception of the file descriptor relevant\n> to the IPC peer Unix socket.\n> \n> In order to enable the IPA logging in the console, add stdout and\n> stderr to the list of file descriptors to be shared with the\n> isolated process.\n> \n> Signed-off-by: Julien Vuillaumier \n\nReviewed-by: Paul Elder \n\n> ---\n> src/libcamera/ipc_pipe_unixsocket.cpp | 5 +++++\n> 1 file changed, 5 insertions(+)\n> \n> diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp\n> index 668ec73b..9ca219dc 100644\n> --- a/src/libcamera/ipc_pipe_unixsocket.cpp\n> +++ b/src/libcamera/ipc_pipe_unixsocket.cpp\n> @@ -7,6 +7,7 @@\n> \n> #include \"libcamera/internal/ipc_pipe_unixsocket.h\"\n> \n> +#include \n> #include \n> \n> #include \n> @@ -42,6 +43,10 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,\n> \targs.push_back(std::to_string(fd.get()));\n> \tfds.push_back(fd.get());\n> \n> +\t/* Share stdout and stderr with the proxy for logging purpose */\n> +\tfds.push_back(STDOUT_FILENO);\n> +\tfds.push_back(STDERR_FILENO);\n> +\n> \tproc_ = std::make_unique();\n> \tint ret = proc_->start(ipaProxyWorkerPath, args, fds);\n> \tif (ret) {\n> -- \n> 2.34.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E590EBD80A\n\tfor ;\n\tMon, 9 Dec 2024 11:33:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 22A3267E5F;\n\tMon, 9 Dec 2024 12:33:47 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 448A666132\n\tfor ;\n\tMon, 9 Dec 2024 12:33:45 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:a56c:f299:b6d:3bb8])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B534D502;\n\tMon, 9 Dec 2024 12:33:12 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"u0XBH7Ks\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1733743993;\n\tbh=leUDnqxWux1MqpfAA+sGHPS3ijn+AXG2zbgJAfSyY9s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=u0XBH7KszwvLx9dNeTp0K0EUR8cm5HoyRwGOA3bYmK7e/7JUU8+BiEJuhCmMsq+zk\n\tz5ySZTNMmjqF+y4OtzhISeH/w+N3GevZ0s4q8wrWpxkiiRMxY69jgA+tVv95WzRrl1\n\tXFvlo5bxmJ5KccmkaW7Wrmz+VRjBGLKA95KLuGSM=","Date":"Mon, 9 Dec 2024 20:33:39 +0900","From":"Paul Elder ","To":"Julien Vuillaumier ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] libcamera: ipc_unixsocket: Share stdin and stdout\n\twith IPA proxy","Message-ID":"","References":"<20241106154306.2490129-1-julien.vuillaumier@nxp.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20241106154306.2490129-1-julien.vuillaumier@nxp.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32641,"web_url":"https://patchwork.libcamera.org/comment/32641/","msgid":"<65102ffc-360a-431f-bbb5-eca99ab0c4c5@nxp.com>","date":"2024-12-09T18:54:16","subject":"Re: [EXT] Re: [PATCH v1] libcamera: ipc_unixsocket: Share stdin and\n\tstdout with IPA proxy","submitter":{"id":190,"url":"https://patchwork.libcamera.org/api/people/190/","name":"Julien Vuillaumier","email":"julien.vuillaumier@nxp.com"},"content":"Hi Laurent,\n\nThank you for your review comments.\n\nOn 09/12/2024 10:39, Laurent Pinchart wrote:\n> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button\n> \n> \n> Hi Julien,\n> \n> Thank you for the patch.\n> \n> On Wed, Nov 06, 2024 at 04:43:06PM +0100, Julien Vuillaumier wrote:\n>> When IPA is running in isolated mode, at IPA process creation time\n>> all the inherited file descriptors are closed in the new process\n>> after the fork, with the exception of the file descriptor relevant\n>> to the IPC peer Unix socket.\n>>\n>> In order to enable the IPA logging in the console, add stdout and\n>> stderr to the list of file descriptors to be shared with the\n>> isolated process.\n>>\n>> Signed-off-by: Julien Vuillaumier \n>> ---\n>> src/libcamera/ipc_pipe_unixsocket.cpp | 5 +++++\n>> 1 file changed, 5 insertions(+)\n>>\n>> diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp\n>> index 668ec73b..9ca219dc 100644\n>> --- a/src/libcamera/ipc_pipe_unixsocket.cpp\n>> +++ b/src/libcamera/ipc_pipe_unixsocket.cpp\n>> @@ -7,6 +7,7 @@\n>>\n>> #include \"libcamera/internal/ipc_pipe_unixsocket.h\"\n>>\n>> +#include \n>> #include \n>>\n>> #include \n>> @@ -42,6 +43,10 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,\n>> args.push_back(std::to_string(fd.get()));\n>> fds.push_back(fd.get());\n>>\n>> + /* Share stdout and stderr with the proxy for logging purpose */\n>> + fds.push_back(STDOUT_FILENO);\n>> + fds.push_back(STDERR_FILENO);\n> \n> Those are the default file descriptors for stdout and stderr, but an\n> application may have remapped those streams, or even closed them. All of\n> a sudden you may end up writing to a file or any other fd that the\n> application would have opened after closing stderr or stdout.\n\n\nWhen a daemon closes STDIN/STDOUT/STDERR file descriptors to detach from \nthe tty it was started from, I think that a common practice is to \nreallocate them to /dev/null using dup2(). That is to remove the risk of \nhaving later a file descriptor created by opening a file or socket, \nreusing one of the 0/1/2 FDs. Such file or socket could be corrupted \nindeed in case of perror(), assert() or some sort of printf().\n\nNow libcamera also uses e.g. assert() whose output message is routed to \nSTDERR. Thus, if the calling application closes FD=2 without \nreallocating it to something, such risk of corruption is present.\n\nTo my understanding, I would think that:\n- The application linking with libcamera takes some risks if it closes \neither of STDIN/STDOUT/STDERR FDs and keeps them unallocated - also if \nreallocated, it should be to something relevant to STDIN/STDOUT/STDERR usage\n- Likewise, starting the IPA proxy with STDIN/STDOUT/STDERR unallocated \nmay not be a safe option: STDIN/STDOUT may better be reassigned to \n/dev/null, and STDERR could be inherited from the parent.\n(That is not what the current patch is doing... maybe what should be done?)\n\nThanks,\nJulien\n\n\n\n\n> \n>> +\n>> proc_ = std::make_unique();\n>> int ret = proc_->start(ipaProxyWorkerPath, args, fds);\n>> if (ret) {\n> \n> --\n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 16B9DBD80A\n\tfor ;\n\tMon, 9 Dec 2024 18:53:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E848867E73;\n\tMon, 9 Dec 2024 19:53:33 +0100 (CET)","from EUR03-DBA-obe.outbound.protection.outlook.com\n\t(mail-dbaeur03on2060c.outbound.protection.outlook.com\n\t[IPv6:2a01:111:f403:260d::60c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0854666132\n\tfor ;\n\tMon, 9 Dec 2024 19:53:32 +0100 (CET)","from AM9PR04MB8147.eurprd04.prod.outlook.com\n\t(2603:10a6:20b:3e0::22)\n\tby PAXPR04MB8670.eurprd04.prod.outlook.com (2603:10a6:102:21d::15)\n\twith Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8230.18;\n\tMon, 9 Dec 2024 18:53:29 +0000","from AM9PR04MB8147.eurprd04.prod.outlook.com\n\t([fe80::eace:e980:28a4:ef8a]) by\n\tAM9PR04MB8147.eurprd04.prod.outlook.com\n\t([fe80::eace:e980:28a4:ef8a%5]) with mapi id 15.20.8230.016;\n\tMon, 9 Dec 2024 18:53:28 +0000"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"MQ1jTzCd\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"ARC-Seal":"i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none;\n\tb=qd15Mjoc6XogtSYJMFYrVGnuOVnARBUDcf6WHl/c9ldQfgDv/yTpbx6NjVPQo1J1RurKiblTTA6p7PVo88XyVT1UJGuz2yRNJws85Gfe/Ah9K++nWPKt7VQDerwDkmoac8DdqZwBFuNeQFpFXMeCHrU+rYVpn7wR5GQYdwMML+mwbDSTm6HVKcM1ue5+BbHFJji+k6WtwtWMG9KpcqpGUhRlXIuVw+z6nFAa2vsm+/YPFebvETO1cc7myIPSCwICpnWlA08K1XkZadlIxlu9aU6dHos2YJe5U+EUqYWZbUjtM9IPWT2m3E7D7Q2JSEAIpo7zoNdQvPvPXIoBysbNAA==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n\ts=arcselector10001;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;\n\tbh=TDhIvrhCwl7Cl0ioV93Fu1UV3OHPfk+7CDH7BBTT7MY=;\n\tb=SyhNFiEcoU6eCc6eNe7VowKJlOUemBLwXpMHCn7hyjrvupmEa9hYHDlSVYaiCZGqUQUjshafg2vOUT3m7erEeHyyqRxRiHDhrSnoBRCTIyHI3pkPxYlOCyx1FjCSeNHwAz6Z79Rs9myGbv1mAJPP9e5OKBffdJkSy8A0xi+5/S4Fwc/ttrjEqa82gFEzvyHxjlYymvnkCtr9a8RZgg3znz2jJW8jSxeFT9IxUwyIiDxuaqU3+yvj30cBj134NQCyWcvcSfZeqWWZN+GGFIOrek5CMll/E0ly2rwzuTb+hV7meHlSnkmdtlU3yfAJaoYl8uInMJ9UeMV0sjFLgacXvw==","ARC-Authentication-Results":"i=1; mx.microsoft.com 1; spf=pass\n\tsmtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com;\n\tdkim=pass header.d=nxp.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n\tbh=TDhIvrhCwl7Cl0ioV93Fu1UV3OHPfk+7CDH7BBTT7MY=;\n\tb=MQ1jTzCdknlRrgNUN2PuFRyAi0itCA/hbmEpO7Eu7TmBT72VZMe6kEWQYZPvd48yCWNerKUXg6wDWNFTKqcCR0ANnDtplHatvpgoJYRd0n9A+o664UmLUrMYYslDLL1q/UIX6q6C22VLAZa1p5J4gaJf4mESrhk/K3moMmoSB8XGmUORlma7XCSdteF5tEHBKQWomP2Vw7ZY/vVi6ISfWAfDCToguJzA1UJuntWtHhZ8fTopu/6mr3h7YsVB/ipNE3MbRWIpFUihVsogYu9druTopKVPCmXUN0xP/0htvlm6Epb3voFHoN+N1+Hc/CYJy2X6Ttzq7EzBZK3NjpziBA==","Message-ID":"<65102ffc-360a-431f-bbb5-eca99ab0c4c5@nxp.com>","Date":"Mon, 9 Dec 2024 19:54:16 +0100","User-Agent":"Mozilla Thunderbird","Subject":"Re: [EXT] Re: [PATCH v1] libcamera: ipc_unixsocket: Share stdin and\n\tstdout with IPA proxy","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20241106154306.2490129-1-julien.vuillaumier@nxp.com>\n\t<20241209093907.GA3076@pendragon.ideasonboard.com>","Content-Language":"en-US","From":"Julien Vuillaumier ","In-Reply-To":"<20241209093907.GA3076@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-ClientProxiedBy":"AM9P195CA0016.EURP195.PROD.OUTLOOK.COM\n\t(2603:10a6:20b:21f::21) To AM9PR04MB8147.eurprd04.prod.outlook.com\n\t(2603:10a6:20b:3e0::22)","MIME-Version":"1.0","X-MS-PublicTrafficType":"Email","X-MS-TrafficTypeDiagnostic":"AM9PR04MB8147:EE_|PAXPR04MB8670:EE_","X-MS-Office365-Filtering-Correlation-Id":"67e60d55-8be0-4f68-b6b5-08dd1882c507","X-LD-Processed":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635,ExtAddr","X-MS-Exchange-SenderADCheck":"1","X-MS-Exchange-AntiSpam-Relay":"0","X-Microsoft-Antispam":"BCL:0;ARA:13230040|366016|1800799024|376014;","X-Microsoft-Antispam-Message-Info":"=?utf-8?q?JM4MeYVwkYUyNS/y8y/nOR8pYRPI?=\n\t=?utf-8?q?74XA4DjsIocfNOXQlkgsH1nX3M7DY+Z837E99OZVNg45M7hS/kmHsfTO?=\n\t=?utf-8?q?/vTGlIEuR/iUoj6i4J0470CjM2Bn7xuru7eBIRvEcyjfhGFZHIIy6joN?=\n\t=?utf-8?q?8085YAGZmVFR9KMAYjt5HtS3rlb+eFckVu7fyQqxTMPYBM9KwAzZWntO?=\n\t=?utf-8?q?j197NuQazIi4wWEJD92h84Ut4rwjr2a9ixJw3Roi29y7xutyDCAY93jK?=\n\t=?utf-8?q?24d19RULRWlNiEM73iimNhEHRRxbyh4dYETzD4xp6mg2Zh4WRU+Uz+Rn?=\n\t=?utf-8?q?X1GVb4jwFfPgfx0Ee0G2XMSLVRr14v4bBKZNl07TNTRsGBJ+Qs5s1AlH?=\n\t=?utf-8?q?jtjb6urB50+vXuhMMV7ss0wM3tDAzvRhqTPidQ5X3VjH1tcz9ztjY/od?=\n\t=?utf-8?q?hypfSgKJ2ODJWlyMbt86eZVFyzGG5ykPErSWNEQDEuiYWrqwYLhpsn1W?=\n\t=?utf-8?q?Wjpa17F+BYRpKagqedBOaCQfio02F7eT4CHKal4eenknyy1b+/fQBQEE?=\n\t=?utf-8?q?TTxc20Wl4XJB+9icUQ0QMjwiiyCwTIcbuKJlrmxtkFoYBd5oCNA00DHZ?=\n\t=?utf-8?q?j7z8jpt7vXhFnh9Y46QzM83MUX6P+lPOJCfxA+SHcGIRp5kKvNmvh2wc?=\n\t=?utf-8?q?IQdzfCuSunQRvTTx1UX51eNQu2ZAOK1oiz89wJZE6gDjSkRYDnPsx8Jw?=\n\t=?utf-8?q?l6PFPJnj1zNOHKhYeGi7aYPhU7nqwGg8lCN7F74cpoJVjd9eYA2/xMes?=\n\t=?utf-8?q?DvhwotK9pR4Q3cviGPc9hBUyngOtgSO6bcNnFGDPIAU3W2cdJGWzQgTb?=\n\t=?utf-8?q?WObKbJ4g2Y6wwFpifPvD5+OrFIW41Qo5ZYLxnfeQdezokA4Fq0RQWsAI?=\n\t=?utf-8?q?NrAdCUW/Ty/W+JIR4l5nEqKBTHe7xpru2Mqh7B98v0XjRgPP0T9mvXz5?=\n\t=?utf-8?q?UtaGrIUspqfI7afxmWzMrr6NbsYE5pLNxuROwe433vh++cIysfshboJg?=\n\t=?utf-8?q?cRAVtr7j30TSwaC6JsKlZUUJzY7Pt94+tKtyoFTREfmXFgN+EEtBpHW5?=\n\t=?utf-8?q?sEajdICiWTHZOM2UbyOc2dM+dPtcPwvrTw8kGI/wJMdubmwbdtEB7SL+?=\n\t=?utf-8?q?N0YaccFW+zDNKNx/QjDZF9B9Jk2stT230nibFeLF1VgYnBvbJXUUAOqz?=\n\t=?utf-8?q?GRw6j0EVZu8pyJ0zU8pC22X9cWEcmbwdzJ61V8eDGxuyHRwNpU1zzh7D?=\n\t=?utf-8?q?wYbekSRFlecalk2i9U8qq2IL0uA9gMC++HJcCKIhylSbTYf6/m8PHB0c?=\n\t=?utf-8?q?0w+1RplWlual3OeYlO4bNb561VPD4no8pV8YFYDNQmwORIwEPh+KaqiS?=\n\t=?utf-8?q?X2gRB07XlCfs8tyg0bXYXhE7d5d8jG/59hIIZbzAFsbDIQA0/A=3D=3D?=","X-Forefront-Antispam-Report":"CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;\n\tIPV:NLI; SFV:NSPM; H:AM9PR04MB8147.eurprd04.prod.outlook.com; PTR:;\n\tCAT:NONE; \n\tSFS:(13230040)(366016)(1800799024)(376014); DIR:OUT; SFP:1101; ","X-MS-Exchange-AntiSpam-MessageData-ChunkCount":"1","X-MS-Exchange-AntiSpam-MessageData-0":"=?utf-8?q?IgZ/6y76BbJHnM6xFSBNwCHwe?=\n\t=?utf-8?q?f1tWm5i5VCzNlh+USWtIp5PQCN0ROmjdnmGifbG5DeBKOcndjt/so9x4?=\n\t=?utf-8?q?d+tn/I0D72pmOBGkPANSSfZCWdGKZuSapqNbyxwrsdmNfOuP58yz2TPl?=\n\t=?utf-8?q?Q19wGAi1FjfNZF2ja//CRpR2iw/al2jXQtSrCHZaX8sKlNKz7SFaa3KP?=\n\t=?utf-8?q?vNMM/U1NoneCIqDvQAg22ZkKaYFdTIeiiHtau7xcovFitgUJDB6m8XM/?=\n\t=?utf-8?q?2p9zUDZyCsNkgFIE02rYnZoExM1/YvvGw1o1/Hedi5bancl/cxYfu+2E?=\n\t=?utf-8?q?T9b1eIiOGEZ+MUtcTVubL2/bC67sxqKbrtYqcP6CUAIKbv/FgUL0bK4x?=\n\t=?utf-8?q?QGP3arV+4h2sYtFFVrRvzaJ1S6Y0rxwy/Gce1QZDkjzZwhAQqN/0kgcb?=\n\t=?utf-8?q?/MMH2PYyzGK2wx6ZaMlyjidiCvh948M9u/vQOB9APMfbYx/MvV9xz7wV?=\n\t=?utf-8?q?/XScNj2e/lo/L65QWFJ3Eebldz2Q82u8ioujrTMY4ogzY48ko5qmrahI?=\n\t=?utf-8?q?AEiCv482HK1TdhwooF3JlcQrS+c8QoboQvOM3bgBkSXL6469EMkYpOLm?=\n\t=?utf-8?q?AkVZ39J+nL/uoj01oWnUaupOeQZJZ6DT4HdMGlCusEfq+LnDQjKxxz5/?=\n\t=?utf-8?q?AosZ5rlhJI1la3qTFJZNbtZac7Xa2Lu1QPfNzKoaGSIiZLeQLAxHej+d?=\n\t=?utf-8?q?IoeOBNL5vkAIW3R4b4qTGs/xLJiTBGEjSYHfK1MC6kWhIwCaLGOPkbqk?=\n\t=?utf-8?q?qJ1MeD3Lza5ii/d7E/lWi342C7zH9H/WKkThxC73gXzEEZAGnA2IaqfH?=\n\t=?utf-8?q?j3vxpIjEFLsdmdZ/ATzCgoPgNAO4xaHB8Z3/t6rqGGgB36VfFwHWwwFq?=\n\t=?utf-8?q?zHyWLcwqJrTKvROeG5+dsbcxGxI5+7Znpcf1TjO6TEVhRLNxpyTBrbNW?=\n\t=?utf-8?q?lbPjUL2HSqgvKBppzeghP0W+wlkrwXAYYUFCecYbDUOk2bZgqgqeKSQ8?=\n\t=?utf-8?q?IeHifiZvZ3i7VAk0iUszNeQdnnvOuDS4gmSAeuYgQP1FGUuCUwl6Jl2X?=\n\t=?utf-8?q?2MVK+H6xwFOHDxRa6omRXRCtULMi0BQsZ/OrMLwh3O7CbmjN4n4BaC15?=\n\t=?utf-8?q?bD+OeY/z61pk9fjQfjZ0mt4n6XNdT3zolihe3nT8oQ5r7CMxWkP4cjk9?=\n\t=?utf-8?q?gyM20OCpXJW6non0ndk0dloJgb4S8khYWkF9HfL82z7Ww+KFPFSvCsDD?=\n\t=?utf-8?q?Sc2EGl6DYZeUMe8AiSVFy24NA9jWOInVk2bsI5Kvpt7s86VEGxJOkLdy?=\n\t=?utf-8?q?gE3xjEOhxVabl3s7YjUczRHAhqFPr72DLCmc6zg8f6wwHRRZYhoR9qa0?=\n\t=?utf-8?q?+sj/1aORKIQomUqH3M0Lsn+bKP25459NPgqPgUHj0UgAZakFk5Ntp88j?=\n\t=?utf-8?q?fosEuOIpoLLZnVtut4tDyC7NaMJ9j9DfIOsm5jyMY7CYq2vWfstZfksY?=\n\t=?utf-8?q?HGva+1T78k25u4VBEQuJjQUd7XsfDtkCIqqYk2k4OuurboukG8Tm71YY?=\n\t=?utf-8?q?IVCRiAnSJJ1sQmTOo6mrZUF0Jf88lMQp5kpL/RN6bLWItOtxe/eCY3qt?=\n\t=?utf-8?q?dpMJJtLcRmTVBw9zTzttPTN6aEynmZcNWHBMEr071qo4p+MYi2HYXEkL?=\n\t=?utf-8?q?OS45SB4Uifit3J18w5csPkwn+TIwQ=3D=3D?=","X-OriginatorOrg":"nxp.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"67e60d55-8be0-4f68-b6b5-08dd1882c507","X-MS-Exchange-CrossTenant-AuthSource":"AM9PR04MB8147.eurprd04.prod.outlook.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-OriginalArrivalTime":"09 Dec 2024 18:53:28.8086\n\t(UTC)","X-MS-Exchange-CrossTenant-FromEntityHeader":"Hosted","X-MS-Exchange-CrossTenant-Id":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635","X-MS-Exchange-CrossTenant-MailboxType":"HOSTED","X-MS-Exchange-CrossTenant-UserPrincipalName":"+44/FjtN4E+rRYrXOabASkqhJLcm0xeQEcQffGb6aWwD+E4P/jhvpimsXPVGPYk1Gn6fMsFdnh53UDyIjuOuP2xnkVhTN3kTuROFlWRuzgU=","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"PAXPR04MB8670","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]