[Valgrind-developers] [valgrind] Fix execveat() with AT_FDCWD and relative path, add more checks

[Mark W. <ma...@so...>]

https://sourceware.org/cgit/valgrind/commit/?id=a2c294b3b19ef4af391014e7b3cbc5ec75b2bd68

commit a2c294b3b19ef4af391014e7b3cbc5ec75b2bd68
Author: Martin Cermak <mc...@re...>
Date:   Fri Aug 1 15:35:04 2025 +0200

    Fix execveat() with AT_FDCWD and relative path, add more checks
    
    This update does address two closely related problems:
    
    1) In case execveat() is called with a special file descriptor value
    of AT_FDCWD (-100), it should accept this special value, and
    interpret the provided pathname as relative to the current working
    directory of the calling process (like execve(2)) instead of
    failing with EBADF, as it does without this patch.
    
    Covered by LTP testcase execveat01.
    
    https://bugs.kde.org/show_bug.cgi?id=506806
    
    2) Add checks preventing execveat() of symlinked programs in case
    AT_SYMLINK_NOFOLLOW was specified.
    
    Add checks preventing execveat() from passing in case invalid
    flag was specified.
    
    Covered by LTP testcase execveat02.
    
    https://bugs.kde.org/show_bug.cgi?id=506813

Diff:
---
 NEWS                                |  2 ++
 coregrind/m_syswrap/syswrap-linux.c | 45 ++++++++++++++++++-------------------
 2 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/NEWS b/NEWS
index 32bea93531..cf879c8d20 100644
--- a/NEWS
+++ b/NEWS
@@ -58,6 +58,8 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 506076  unimplemented fcntl command: 1028 (F_CREATED_QUERY)
 506499  Unhandled syscall 592 (exterrctl - FreeBSD
 506795  Better report which clone flags are problematic
+506806  Fix execveat() with AT_FDCWD and relative path
+506813  The execveat wrapper needs to do more checking
 506910  openat2 with RESOLVE_NO_MAGICLINKS succeeds on /proc/self/exe
 506928  Wrap (deprecated) linux specific ustat syscall
 506929  Wrap (deprecated) linux sysfs syscall
diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index 66e5ca62e7..c81d941a74 100644
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -13893,12 +13893,12 @@ PRE(sys_execveat)
    return;
 #endif
 
+   Int arg_1 = (Int) ARG1;
    const HChar *path = (const HChar*) ARG2;
    Addr arg_2    = ARG3;
    Addr arg_3    = ARG4;
    const HChar   *buf;
    HChar         *abs_path = NULL;
-   Bool check_at_symlink = False;
    Bool check_pathptr = True;
 
    if (ML_(safe_to_deref) (path, 1)) {
@@ -13906,8 +13906,12 @@ PRE(sys_execveat)
         * and just pass the pathname, try to determine
         * the absolute path otherwise. */
        if (path[0] != '/') {
-           /* Check dirfd is a valid fd. */
-           if (!ML_(fd_allowed)(ARG1, "execveat", tid, False)) {
+           /* Check dirfd is a valid fd.
+            * BUT: allow special value of AT_FDCWD (-101) per the execveat(2) man page:
+            *      If pathname is relative and dirfd is the special value AT_FDCWD,
+            *      then pathname is interpreted relative to the current working directory
+            *      of the calling process  (like execve(2)). */
+           if (arg_1 != VKI_AT_FDCWD && !ML_(fd_allowed)(arg_1, "execveat", tid, False)) {
                SET_STATUS_Failure( VKI_EBADF );
                return;
            }
@@ -13915,38 +13919,33 @@ PRE(sys_execveat)
               set then dirfd describes the whole path. */
            if (path[0] == '\0') {
                if (ARG5 & VKI_AT_EMPTY_PATH) {
-                   if (VG_(resolve_filename)(ARG1, &buf)) {
+                   if (VG_(resolve_filename)(arg_1, &buf)) {
                        path = buf;
                        check_pathptr = False;
                    }
                }
-           }
-           else if (ARG1 == VKI_AT_FDCWD) {
-               check_at_symlink = True;
-           } else
-               if (ARG5 & VKI_AT_SYMLINK_NOFOLLOW)
-                   check_at_symlink = True;
-               else if (VG_(resolve_filename)(ARG1, &buf)) {
+           } else if (VG_(resolve_filename)(arg_1, &buf)) {
 		   abs_path = VG_(malloc)("execveat",
                                           (VG_(strlen)(buf) + 1
                                           + VG_(strlen)(path) + 1));
                    VG_(sprintf)(abs_path, "%s/%s", buf, path);
                    path = abs_path;
                    check_pathptr = False;
-               }
-               else
-                   path = NULL;
-           if (check_at_symlink) {
-               struct vg_stat statbuf;
-               SysRes statres;
-
-               statres = VG_(stat)(path, &statbuf);
-               if (sr_isError(statres) || VKI_S_ISLNK(statbuf.mode)) {
-                   SET_STATUS_Failure( VKI_ELOOP );
-                   return;
-               }
            }
        }
+       if (ARG5 & VKI_AT_SYMLINK_NOFOLLOW) {
+          struct vg_stat statbuf;
+          SysRes statres;
+          statres = VG_(stat)(path, &statbuf);
+          if (sr_isError(statres) || VKI_S_ISLNK(statbuf.mode)) {
+             SET_STATUS_Failure( VKI_ELOOP );
+             return;
+          }
+       }
+       if(ARG5 & ~(VKI_AT_SYMLINK_NOFOLLOW | VKI_AT_EMPTY_PATH)) {
+          SET_STATUS_Failure( VKI_EINVAL );
+          return;
+       }
    } else {
        SET_STATUS_Failure(VKI_EFAULT);
        return;