[PATCH v3 1/1] libcamera: process: Pass stderr and reserve stdin and stdout fds

[Laurent Pinchart]

Hi Julien,

Thank you for the patch.

On Mon, May 19, 2025 at 02:23:52PM +0200, Julien Vuillaumier wrote:
> When a child process is started from Process::start(), the file
> descriptors inherited from the parent process are closed, except
> the ones explicitly listed in the fds[] argument.
> 
> One issue is that the file descriptors for stdin, stdout and stderr
> being closed, the subsequent file descriptors created by the child
> process will reuse the values 0, 1 and 2 that are now available.
> Thus, usage of printf(), assert() or alike may direct its output
> to the new resource bound to one of these reused file descriptors.
> The other issue is that the child process can no longer log on
> the console because stderr has been closed.
> 
> To address the 2 issues, Process:start() is amended as below:
> - Child process inherits from parent's stderr fd in order to share
> the same logging descriptor
> - Child process stdin, stdout and stderr fds are bound to /dev/null
> if not inherited from parent. That is to prevent those descriptors
> to be reused for any other resource, that could be corrupted by
> the presence of printf(), assert() or alike.
> 
> Signed-off-by: Julien Vuillaumier <julien.vuillaumier at nxp.com>
> ---
>  src/libcamera/process.cpp | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> index 68fad327..fa92f58c 100644
> --- a/src/libcamera/process.cpp
> +++ b/src/libcamera/process.cpp
> @@ -259,7 +259,21 @@ int Process::start(const std::string &path,
>  		if (isolate())
>  			_exit(EXIT_FAILURE);
>  
> -		closeAllFdsExcept(fds);
> +		std::vector<int> v(fds);
> +		v.push_back(STDERR_FILENO);

There's still a risk that STDERR_FILENO refers to a different file than
stderr. Could you test this patch (with an IPA module running in
isolated mode) with pipewire ? If that doesn't cause any issue I'll feel
better accepting the risk.

> +		closeAllFdsExcept(v);
> +
> +		const auto tryDevNullLowestFd = [](int expected, int oflag) {
> +			int fd = open("/dev/null", oflag);
> +			if (fd < 0)
> +				exit(EXIT_FAILURE);
> +			if (fd != expected)
> +				close(fd);
> +		};
> +
> +		tryDevNullLowestFd(STDIN_FILENO, O_RDONLY);
> +		tryDevNullLowestFd(STDOUT_FILENO, O_WRONLY);
> +		tryDevNullLowestFd(STDERR_FILENO, O_WRONLY);
>  
>  		const char *file = utils::secure_getenv("LIBCAMERA_LOG_FILE");
>  		if (file && strcmp(file, "syslog"))

-- 
Regards,

Laurent Pinchart