Bug 493430

Also when https://bugs.kde.org/show_bug.cgi?id=493433 has been added (--modify-fds) all returned file descriptors should go through POST_newFd_RES or equivalent.

Created attachment 182946 [details]
patch

commit fa5cdc7fcd24f09adcfb7d44ee87b96b3350994d (HEAD -> fcntl)
Author: Alexandra Hájková <ahajkova@redhat.com>
Date:   Fri Jul 4 07:35:13 2025 -0400

    Implement fcntl F_CREATED_QUERY
    
    Define VKI_F_CREATED_QUERY in vki-linux.h.
    Recognize it in PRE(sys_fcntl).
    This fixes ltp tests failures. When running:
    make ltpchecks TESTS="fcntl40 fcntl40_64
    the tests would fail with:
    fcntl40: unempty log2.filtered:
    ==1809471== Warning: unimplemented fcntl command: 1028
    
    https://bugs.kde.org/show_bug.cgi?id=506076

(In reply to Alexandra Hajkova from comment #2)
> Created attachment 182946 [details]
> patch
> 
> commit fa5cdc7fcd24f09adcfb7d44ee87b96b3350994d (HEAD -> fcntl)
> Author: Alexandra Hájková <ahajkova@redhat.com>
> Date:   Fri Jul 4 07:35:13 2025 -0400
> 
>     Implement fcntl F_CREATED_QUERY
>     
>     Define VKI_F_CREATED_QUERY in vki-linux.h.
>     Recognize it in PRE(sys_fcntl).
>     This fixes ltp tests failures. When running:
>     make ltpchecks TESTS="fcntl40 fcntl40_64
>     the tests would fail with:
>     fcntl40: unempty log2.filtered:
>     ==1809471== Warning: unimplemented fcntl command: 1028
>     
>     https://bugs.kde.org/show_bug.cgi?id=506076

Looks good. Please push.

(In reply to Mark Wielaard from comment #3)
> (In reply to Alexandra Hajkova from comment #2)
> > Created attachment 182946 [details]
> > patch
> > 
> > commit fa5cdc7fcd24f09adcfb7d44ee87b96b3350994d (HEAD -> fcntl)
> > Author: Alexandra Hájková <ahajkova@redhat.com>
> > Date:   Fri Jul 4 07:35:13 2025 -0400
> > 
> >     Implement fcntl F_CREATED_QUERY
> >     
> >     Define VKI_F_CREATED_QUERY in vki-linux.h.
> >     Recognize it in PRE(sys_fcntl).
> >     This fixes ltp tests failures. When running:
> >     make ltpchecks TESTS="fcntl40 fcntl40_64
> >     the tests would fail with:
> >     fcntl40: unempty log2.filtered:
> >     ==1809471== Warning: unimplemented fcntl command: 1028
> >     
> >     https://bugs.kde.org/show_bug.cgi?id=506076
> 
> Looks good. Please push.

Still looks good, but this is the wrong bug.
This bug is still open/unfixed. Please push and update bug #506076 instead.

One example of where we didn't check (all) fds was in dup2.

commit 0dbd164e1767dc29a6e0ea8d2c86b02d6913043b
Author: Mark Wielaard <mark@klomp.org>
Date:   Sat Jul 5 00:51:36 2025 +0200

    Check dup2 oldfd before allowing the syscall
    
    The dup201 LTP test fails with TFAIL: dup2(1024, 5) succeeded
    
    That is because 1024 here is the soft file limit (so one higher than
    the max number of fds). Valgrind raises the soft limit a little
    internally to have a few private fds for itself. So this dup2 call
    succeeds (and possibly dups and internal valgrind fd into the
    newfd). We should check the oldfd before allowing the dup2 syscall,
    like we already check the newfd.

diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c
index f8d73e1973d3..50deb1e7641f 100644
--- a/coregrind/m_syswrap/syswrap-generic.c
+++ b/coregrind/m_syswrap/syswrap-generic.c
@@ -3758,6 +3758,8 @@ PRE(sys_dup2)
 {
    PRINT("sys_dup2 ( %" FMT_REGWORD "u, %" FMT_REGWORD "u )", ARG1, ARG2);
    PRE_REG_READ2(long, "dup2", unsigned int, oldfd, unsigned int, newfd);
+   if (!ML_(fd_allowed)(ARG1, "dup2", tid, False))
+      SET_STATUS_Failure( VKI_EBADF );
    if (!ML_(fd_allowed)(ARG2, "dup2", tid, True))
       SET_STATUS_Failure( VKI_EBADF );
 }

Created attachment 183634 [details]
patch

Make sure all syscalls that use or return file descriptors

 checks the file descriptor is valid. Also make sure
 --modify-fds=high option would affect all such sycalls.

Thanks, tweaked the commit message a little and added a NEWS bug entry.

commit bc66a6e865d952ac51ffb0e63c127ce7cd977b98
Author: Alexandra Hájková <ahajkova@redhat.com>
Date:   Tue Jul 29 09:49:26 2025 -0400

    Add fd_allowed and POST_newFd_RES to all syscalls that use or return fds
    
    This makes sure all file descriptors that take a file descriptor check
    that the file descriptor is valid. Also makes sure that the
    --modify-fds=high option affects all sycalls that return a file
    descriptor.
    
    https://bugs.kde.org/show_bug.cgi?id=493430

I’ll take a look at other platforms when I get back from holidays.