[RFC,v3,5/9] libcamera: process: Ensure that file descriptors are nonnegative
diff mbox series

Message ID 20250325180821.1456154-6-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • libcamera: process: Remove `ProcessManager` singleton
Related show

Commit Message

Barnabás Pőcze March 25, 2025, 6:08 p.m. UTC
Return `-EINVAL` from `Process::start()` if any of the file
descriptors are negative as those most likely signal some
kind of issue such as missed error checking.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/libcamera/process.cpp | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Kieran Bingham March 25, 2025, 8:51 p.m. UTC | #1
Quoting Barnabás Pőcze (2025-03-25 18:08:17)
> Return `-EINVAL` from `Process::start()` if any of the file
> descriptors are negative as those most likely signal some
> kind of issue such as missed error checking.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/libcamera/process.cpp | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> index 716523acc..62e63255b 100644
> --- a/src/libcamera/process.cpp
> +++ b/src/libcamera/process.cpp
> @@ -243,6 +243,11 @@ int Process::start(const std::string &path,
>         if (pid_ > 0)
>                 return -EBUSY;
>  
> +       for (int fd : fds) {
> +               if (fd < 0)
> +                       return -EINVAL;
> +       }
> +
>         int childPid = fork();
>         if (childPid == -1) {
>                 ret = -errno;
> @@ -282,6 +287,8 @@ void Process::closeAllFdsExcept(const std::vector<int> &fds)
>         std::vector<int> v(fds);
>         sort(v.begin(), v.end());
>  
> +       ASSERT(v.empty() || v.front() >= 0);
> +
>         DIR *dir = opendir("/proc/self/fd");
>         if (!dir)
>                 return;
> -- 
> 2.49.0
>
Laurent Pinchart March 26, 2025, 1:08 p.m. UTC | #2
Hi Barnabás,

Thank you for the patch.

On Tue, Mar 25, 2025 at 07:08:17PM +0100, Barnabás Pőcze wrote:
> Return `-EINVAL` from `Process::start()` if any of the file
> descriptors are negative as those most likely signal some
> kind of issue such as missed error checking.

Feel free to go up to the 72 columns limit :-)

> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/libcamera/process.cpp | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> index 716523acc..62e63255b 100644
> --- a/src/libcamera/process.cpp
> +++ b/src/libcamera/process.cpp
> @@ -243,6 +243,11 @@ int Process::start(const std::string &path,
>  	if (pid_ > 0)
>  		return -EBUSY;
>  
> +	for (int fd : fds) {
> +		if (fd < 0)
> +			return -EINVAL;
> +	}
> +
>  	int childPid = fork();
>  	if (childPid == -1) {
>  		ret = -errno;
> @@ -282,6 +287,8 @@ void Process::closeAllFdsExcept(const std::vector<int> &fds)
>  	std::vector<int> v(fds);
>  	sort(v.begin(), v.end());
>  
> +	ASSERT(v.empty() || v.front() >= 0);
> +
>  	DIR *dir = opendir("/proc/self/fd");
>  	if (!dir)
>  		return;

Patch
diff mbox series

diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
index 716523acc..62e63255b 100644
--- a/src/libcamera/process.cpp
+++ b/src/libcamera/process.cpp
@@ -243,6 +243,11 @@  int Process::start(const std::string &path,
 	if (pid_ > 0)
 		return -EBUSY;
 
+	for (int fd : fds) {
+		if (fd < 0)
+			return -EINVAL;
+	}
+
 	int childPid = fork();
 	if (childPid == -1) {
 		ret = -errno;
@@ -282,6 +287,8 @@  void Process::closeAllFdsExcept(const std::vector<int> &fds)
 	std::vector<int> v(fds);
 	sort(v.begin(), v.end());
 
+	ASSERT(v.empty() || v.front() >= 0);
+
 	DIR *dir = opendir("/proc/self/fd");
 	if (!dir)
 		return;