Bug 506813

What do glibc / the kernel do to not resolve symlinks with  AT_SYMLINK_NOFOLLOW? For https://bugs.kde.org/show_bug.cgi?id=505673 I’ll be making VG_(realpth) and VG_(lstat) available on all platforms. Possibly that will be of some use but it always resolves symlinks.

The reason we have to handle AT_SYMLINK_NOFOLLOW explicitly is because we try to translate the execveat call into a execve call.
For execveat glibc does nothing special for AT_SYMLINK_NOFOLLOW because it is a simple wrapper of the kernel syscall.
In other cases it seems glibc also doesn't do something special just passes AT_SYMLINK_NOFOLLOW to syscalls that take a flag.

Having VG_(lstat) would certainly be helpful. But I believe the check used in our linux sys_execveat PRE wrapper is actually OK.
VG_(stat)(path, &statbuf); if (VKI_S_ISLNK(statbuf.mode)) SET_STATUS_Failure( VKI_ELOOP );
It is just that it isn't done consistently. It is only done on relative paths, not on absolute paths.

Created attachment 183659 [details]
proposed patch

> +       } else {
> +           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;
> +           }

Yes, this is the correct check to do.

But after reading this code twice now (see also bug #506806) I believe the if/else logic here is somewhat wonky.
The exact same code is there slightly upward, but then for relative paths (path[0] != '/').

Inside the relative path block there is some funky logic that probably shouldn't be there.
And the AT_SYMLINK_NOFOLLOW logic should probably be only afterwards (the same for absolute and relative paths).

I think the check for the relative path check can be reduced to:

         if (arg_1 != VKI_AT_FDCWD
             && !ML_(fd_allowed)(arg_1, "execveat", tid, False) && arg_1) {
            SET_STATUS_Failure( VKI_EBADF );
            return;
         }
         /* If pathname is empty and AT_EMPTY_PATH is 
            set then dirfd describes the whole path. */
         if (path[0] == '\0') {
            if (ARG5 & VKI_AT_EMPTY_PATH) {
               if (VG_(resolve_filename)(arg_1, &buf)) {
                  path = buf;
                  check_pathptr = False;
               }
            }
         } 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;
         }

And then you do the VKI_AT_SYMLINK_NOFOLLOW and ~(VKI_AT_SYMLINK_NOFOLLOW | VKI_AT_EMPTY_PATH) just for both relative and absolute (the relative path will have been made absolute at this point).

Hope this makes sense.

Created attachment 183710 [details]
proposed patch

Yes, it does make sense!  Thank you for the review.  Since the two bugs are closely related, I've prepared a patch addressing them both.  It seems to pass both regtest and ltpchecks with execveat tests.  Please review.

Makes sense to combine these fixes. Even though it does more checks the code is not simpler. Thanks. Pushed as

commit a2c294b3b19ef4af391014e7b3cbc5ec75b2bd68
Author: Martin Cermak <mcermak@redhat.com>
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