https://bugs.kde.org/show_bug.cgi?id=506806

--- Comment #2 from Mark Wielaard <[email protected]> ---
> Subject: [PATCH] Fix execveat() with AT_FDCWD and relative path               
>                                                                               
> 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.                            
>                                                                               
> This functionality is test-covered by the execveat01 LTP testcase.            
>                                                                               
> https://bugs.kde.org/show_bug.cgi?id=506806                                   

Aha, bad logic. Good find.

> diff --git a/coregrind/m_syswrap/syswrap-linux.c 
> b/coregrind/m_syswrap/syswrap-linux.c                                         
>                                                                               
>                                                
> index 552fceee8..9cd0303fa 100644                                             
>                                                                               
>                                                                               
>                   
> --- a/coregrind/m_syswrap/syswrap-linux.c                                     
>                                                                               
>                                                                               
>                   
> +++ b/coregrind/m_syswrap/syswrap-linux.c                                     
>                                                                               
>                                                                               
>                   
> @@ -13804,6 +13804,7 @@ PRE(sys_execveat)                                     
>                                                                               
>                                                                               
>                   
>     return;                                                                   
>                                                                               
>                                                                               
>                   
>  #endif                                                                       
>                                                                               
>                                                                               
>                   
>                                                                               
>                                                                               
>                                                                               
>                   
> +   int arg_1 = (int) ARG1;                                                   
>                                                                               
>                                                                               
>                   
>     const HChar *path = (const HChar*) ARG2;                                  
>                                                                               
>                                                                               
>                   
>     Addr arg_2    = ARG3;                                                     
>                                                                               
>                                                                               
>                   
>     Addr arg_3    = ARG4;                                                     
>                                                                               
>                                                                               
>                   

Please use Int and (Int)

> @@ -13817,8 +13818,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 (!ML_(fd_allowed)(arg_1, "execveat", tid, False) && arg_1 != 
> VKI_AT_FDCWD) {                                                               
>                                                                               
>                     
>                 SET_STATUS_Failure( VKI_EBADF );                              
>                                                                               
>                                                                               
>                   
>                 return;                                                       
>                                                                               
>                                                                               
>                   
>             }                                                                 
>                                                                               
>                                                                               
>                   

nitpick. swapping the checks might be a tiny optimization, since the arg_1 !=
VKI_AT_FDCWD is fast and cheap, but ML_(fd_allowed) might (with fd tracking) do
a lot more work.

> -           else if (ARG1 == VKI_AT_FDCWD) {                                  
>                                                                               
>                                                                               
>                   
> +           else if (arg_1 == VKI_AT_FDCWD) {                                 
>                                                                               
>                                                                               
>                   
>                 check_at_symlink = True;                                      
>                                                                               
>                                                                               
>                   
>             } else                                                            
>                                                                               
>                                                                               
>                   

This is existing code, but I think previously it never triggered since it was
missing the (Int) cast.
It seems bogus though. Why would it trigger a check_at_symlink check?
It looks like if execveat is called with AT_FDCWD and the (relative) path to
the exe is a symlink it would always fail whether or not AT_SYMLINK_NOFOLLOW is
given as flag.

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to