[libcamera-devel,v4,11/22] libcamera: ipc_unixsocket: Fix file descriptor leak
diff mbox series

Message ID 20211130033820.18235-12-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Introduce UniqueFD
Related show

Commit Message

Laurent Pinchart Nov. 30, 2021, 3:38 a.m. UTC
The file descriptor created for the remote side of the socket is passed
to the forked process, but never closed. Fix the leak.

The fix can be tested by running the unixsocket_ipc unit test under
valgrind with `valgrind --track-fds=yes ./test/ipc/unixsocket_ipc`.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/ipc_pipe_unixsocket.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Hirokazu Honda Nov. 30, 2021, 5:19 a.m. UTC | #1
Hi Laurent,

On Tue, Nov 30, 2021 at 12:39 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> The file descriptor created for the remote side of the socket is passed
> to the forked process, but never closed. Fix the leak.
>
> The fix can be tested by running the unixsocket_ipc unit test under
> valgrind with `valgrind --track-fds=yes ./test/ipc/unixsocket_ipc`.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
> ---
>  src/libcamera/ipc_pipe_unixsocket.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp
> index 65277500ff42..3ef907090131 100644
> --- a/src/libcamera/ipc_pipe_unixsocket.cpp
> +++ b/src/libcamera/ipc_pipe_unixsocket.cpp
> @@ -38,7 +38,7 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,
>         }
>         socket_->readyRead.connect(this, &IPCPipeUnixSocket::readyRead);
>         args.push_back(std::to_string(fd.get()));
> -       fds.push_back(fd.release());
> +       fds.push_back(fd.get());
>
>         proc_ = std::make_unique<Process>();
>         int ret = proc_->start(ipaProxyWorkerPath, args, fds);
> --
> Regards,
>
> Laurent Pinchart
>
Jacopo Mondi Nov. 30, 2021, 7:56 a.m. UTC | #2
Hi Laurent

On Tue, Nov 30, 2021 at 05:38:09AM +0200, Laurent Pinchart wrote:
> The file descriptor created for the remote side of the socket is passed
> to the forked process, but never closed. Fix the leak.
>
> The fix can be tested by running the unixsocket_ipc unit test under
> valgrind with `valgrind --track-fds=yes ./test/ipc/unixsocket_ipc`.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> ---
>  src/libcamera/ipc_pipe_unixsocket.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp
> index 65277500ff42..3ef907090131 100644
> --- a/src/libcamera/ipc_pipe_unixsocket.cpp
> +++ b/src/libcamera/ipc_pipe_unixsocket.cpp
> @@ -38,7 +38,7 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,
>  	}
>  	socket_->readyRead.connect(this, &IPCPipeUnixSocket::readyRead);
>  	args.push_back(std::to_string(fd.get()));
> -	fds.push_back(fd.release());
> +	fds.push_back(fd.get());
>
>  	proc_ = std::make_unique<Process>();
>  	int ret = proc_->start(ipaProxyWorkerPath, args, fds);
> --
> Regards,
>
> Laurent Pinchart
>

Patch
diff mbox series

diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp
index 65277500ff42..3ef907090131 100644
--- a/src/libcamera/ipc_pipe_unixsocket.cpp
+++ b/src/libcamera/ipc_pipe_unixsocket.cpp
@@ -38,7 +38,7 @@  IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,
 	}
 	socket_->readyRead.connect(this, &IPCPipeUnixSocket::readyRead);
 	args.push_back(std::to_string(fd.get()));
-	fds.push_back(fd.release());
+	fds.push_back(fd.get());
 
 	proc_ = std::make_unique<Process>();
 	int ret = proc_->start(ipaProxyWorkerPath, args, fds);