[PATCH] linux-user: Use qemu_set_cloexec() to mark pidfd as FD_CLOEXEC

[PATCH] linux-user: Use qemu_set_cloexec() to mark pidfd as FD_CLOEXEC

[Peter Maydell]

In the linux-user do_fork() function we try to set the FD_CLOEXEC
flag on a pidfd like this:

    fcntl(pid_fd, F_SETFD, fcntl(pid_fd, F_GETFL) | FD_CLOEXEC);

This has two problems:
 (1) it doesn't check errors, which Coverity complains about
 (2) we use F_GETFL when we mean F_GETFD

Deal with both of these problems by using qemu_set_cloexec() instead.
That function will assert() if the fcntls fail, which is fine (we are
inside fork_start()/fork_end() so we know nothing can mess around
with our file descriptors here, and we just got this one from
pidfd_open()).

(As we are touching the if() statement here, we correct the
indentation.)

Coverity: CID 1508111
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index c600d5ccc0e..b7ec9a4f363 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6743,10 +6743,9 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
                 int pid_child = ret;
                 pid_fd = pidfd_open(pid_child, 0);
                 if (pid_fd >= 0) {
-                        fcntl(pid_fd, F_SETFD, fcntl(pid_fd, F_GETFL)
-                                               | FD_CLOEXEC);
+                    qemu_set_cloexec(pid_fd);
                 } else {
-                        pid_fd = 0;
+                    pid_fd = 0;
                 }
 #endif
                 put_user_u32(pid_fd, parent_tidptr);
-- 
2.43.0

Re: [PATCH] linux-user: Use qemu_set_cloexec() to mark pidfd as FD_CLOEXEC

[Daniel P. Berrangé]

On Fri, Jul 11, 2025 at 03:12:17PM +0100, Peter Maydell wrote:
> In the linux-user do_fork() function we try to set the FD_CLOEXEC
> flag on a pidfd like this:
> 
>     fcntl(pid_fd, F_SETFD, fcntl(pid_fd, F_GETFL) | FD_CLOEXEC);
> 
> This has two problems:
>  (1) it doesn't check errors, which Coverity complains about
>  (2) we use F_GETFL when we mean F_GETFD
> 
> Deal with both of these problems by using qemu_set_cloexec() instead.
> That function will assert() if the fcntls fail, which is fine (we are
> inside fork_start()/fork_end() so we know nothing can mess around
> with our file descriptors here, and we just got this one from
> pidfd_open()).
> 
> (As we are touching the if() statement here, we correct the
> indentation.)
> 
> Coverity: CID 1508111
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/syscall.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] linux-user: Use qemu_set_cloexec() to mark pidfd as FD_CLOEXEC

[Richard Henderson]

On 7/11/25 08:12, Peter Maydell wrote:
> In the linux-user do_fork() function we try to set the FD_CLOEXEC
> flag on a pidfd like this:
> 
>      fcntl(pid_fd, F_SETFD, fcntl(pid_fd, F_GETFL) | FD_CLOEXEC);
> 
> This has two problems:
>   (1) it doesn't check errors, which Coverity complains about
>   (2) we use F_GETFL when we mean F_GETFD
> 
> Deal with both of these problems by using qemu_set_cloexec() instead.
> That function will assert() if the fcntls fail, which is fine (we are
> inside fork_start()/fork_end() so we know nothing can mess around
> with our file descriptors here, and we just got this one from
> pidfd_open()).
> 
> (As we are touching the if() statement here, we correct the
> indentation.)
> 
> Coverity: CID 1508111
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   linux-user/syscall.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH] linux-user: Use qemu_set_cloexec() to mark pidfd as FD_CLOEXEC

[Richard Henderson]

On 7/11/25 08:12, Peter Maydell wrote:
> In the linux-user do_fork() function we try to set the FD_CLOEXEC
> flag on a pidfd like this:
> 
>      fcntl(pid_fd, F_SETFD, fcntl(pid_fd, F_GETFL) | FD_CLOEXEC);
> 
> This has two problems:
>   (1) it doesn't check errors, which Coverity complains about
>   (2) we use F_GETFL when we mean F_GETFD
> 
> Deal with both of these problems by using qemu_set_cloexec() instead.
> That function will assert() if the fcntls fail, which is fine (we are
> inside fork_start()/fork_end() so we know nothing can mess around
> with our file descriptors here, and we just got this one from
> pidfd_open()).
> 
> (As we are touching the if() statement here, we correct the
> indentation.)
> 
> Coverity: CID 1508111
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   linux-user/syscall.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)

Queued, thanks.

r~