https://bugs.kde.org/show_bug.cgi?id=507853
--- Comment #2 from Mark Wielaard <[email protected]> --- Comment on attachment 183880 --> https://bugs.kde.org/attachment.cgi?id=183880 proposed patch > Do more fine-grained checks within sys_faccessat and sys_faccessat2 > syscall wrappers. Allow passing special value of VKI_AT_FDCWD as a > file descriptor. Check for valid flags. I don't think we need to check for valid flags (see below). +507853 faccessat and faccessat2 should handle AT_FDCWD, AT_EMPTY_PATH and absolute paths OK, thanks. > - PRINT("sys_faccessat ( %ld, %#" FMT_REGWORD "x(%s), %ld )", > - SARG1, ARG2, (HChar*)(Addr)ARG2, SARG3); > + Int arg_1 = (Int) ARG1; > + const HChar *path = (const HChar*) ARG2; > + Int arg_3 = (Int) ARG3; > + PRINT("sys_faccessat ( %d, %#" FMT_REGWORD "x(%s), %d )", > + arg_1, ARG2, path, arg_3); OK. > + if (path[0] != '/') You want a if (ML_(safe_to_deref) (path, 1)) before that or valgrind might crash if path is a bogus pointer. > + if ( arg_1 != VKI_AT_FDCWD && !ML_(fd_allowed)(SARG1, "faccessat", > tid, False) ) > + SET_STATUS_Failure( VKI_EBADF ); I think this works fine, but for consistency you might want to change that SARG1 with arg_1. > PRE_MEM_RASCIIZ( "faccessat2(pathname)", ARG2 ); > - if ( !ML_(fd_allowed)(SARG1, "faccessat2", tid, False) ) > - SET_STATUS_Failure( VKI_EBADF ); > + if (path[0] != '/') Same as above, needs an safe_to_deref check first. > + if ( arg_1 != VKI_AT_FDCWD && !ML_(fd_allowed)(SARG1, "faccessat2", > tid, False) ) > + SET_STATUS_Failure( VKI_EBADF ); arg_1 for consistency? > + if (arg_4 & ~(VKI_AT_EACCESS | VKI_AT_EMPTY_PATH | > VKI_AT_SYMLINK_NOFOLLOW)) > + SET_STATUS_Failure( VKI_EINVAL ); I am not sure we need to do this, we can just let the kernel detect any bad flags. Probably my fault for mentioning AT_EMPTY_PATH in the bug subject. I hadn't actually reviewed if we needed anything special for that case. It seems no. Technically that means we also don't need the VKI_AT_ constant updates. But they seem useful anyway. -- You are receiving this mail because: You are watching all bug changes.
