From: Ivo Raisr Date: Mon, 27 Mar 2017 05:06:32 +0000 (+0000) Subject: fcntl syscall wrapper was missing flock structure check on Linux. X-Git-Tag: svn/VALGRIND_3_13_0~144 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=76e451c60df4734cb07df7a01fc732dac5d60df8;p=thirdparty%2Fvalgrind.git fcntl syscall wrapper was missing flock structure check on Linux. Fixes BZ#377930. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@16287 --- diff --git a/NEWS b/NEWS index 56c7053c78..013e84a409 100644 --- a/NEWS +++ b/NEWS @@ -151,6 +151,7 @@ where XXXXXX is the bug number as listed below. and FUTEX_WAKE_BITSET, check only 4 args for FUTEX_WAKE_BITSET, and 2 args for FUTEX_TRYLOCK_PI 377717 Fix massive space leak when reading compressed debuginfo sections +377930 fcntl syscall wrapper is missing flock structure check Release 3.12.0 (20 October 2016) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c index c79b98c7ce..a42bf14850 100644 --- a/coregrind/m_syswrap/syswrap-linux.c +++ b/coregrind/m_syswrap/syswrap-linux.c @@ -5898,19 +5898,45 @@ PRE(sys_fcntl) case VKI_F_GETLK: case VKI_F_SETLK: case VKI_F_SETLKW: + case VKI_F_OFD_GETLK: + case VKI_F_OFD_SETLK: + case VKI_F_OFD_SETLKW: + PRINT("sys_fcntl[ARG3=='lock'] ( %lu, %lu, %#lx )", ARG1, ARG2, ARG3); + PRE_REG_READ3(long, "fcntl", + unsigned int, fd, unsigned int, cmd, + struct vki_flock *, lock); + { + struct vki_flock *lock = (struct vki_flock *) ARG3; + PRE_FIELD_READ("fcntl(lock->l_type)", lock->l_type); + PRE_FIELD_READ("fcntl(lock->l_whence)", lock->l_whence); + PRE_FIELD_READ("fcntl(lock->l_start)", lock->l_start); + PRE_FIELD_READ("fcntl(lock->l_len)", lock->l_len); + if (ARG2 == VKI_F_GETLK || ARG2 == VKI_F_OFD_GETLK) { + PRE_FIELD_WRITE("fcntl(lock->l_pid)", lock->l_pid); + } + } + break; + # if defined(VGP_x86_linux) || defined(VGP_mips64_linux) case VKI_F_GETLK64: case VKI_F_SETLK64: case VKI_F_SETLKW64: -# endif - case VKI_F_OFD_GETLK: - case VKI_F_OFD_SETLK: - case VKI_F_OFD_SETLKW: PRINT("sys_fcntl[ARG3=='lock'] ( %lu, %lu, %#lx )", ARG1, ARG2, ARG3); PRE_REG_READ3(long, "fcntl", unsigned int, fd, unsigned int, cmd, struct flock64 *, lock); + { + struct vki_flock64 *lock = (struct vki_flock64 *) ARG3; + PRE_FIELD_READ("fcntl(lock->l_type)", lock->l_type); + PRE_FIELD_READ("fcntl(lock->l_whence)", lock->l_whence); + PRE_FIELD_READ("fcntl(lock->l_start)", lock->l_start); + PRE_FIELD_READ("fcntl(lock->l_len)", lock->l_len); + if (ARG2 == VKI_F_GETLK64) { + PRE_FIELD_WRITE("fcntl(lock->l_pid)", lock->l_pid); + } + } break; +# endif case VKI_F_SETOWN_EX: PRINT("sys_fcntl[F_SETOWN_EX] ( %lu, %lu, %lu )", ARG1, ARG2, ARG3); @@ -5965,6 +5991,14 @@ POST(sys_fcntl) } } else if (ARG2 == VKI_F_GETOWN_EX) { POST_MEM_WRITE(ARG3, sizeof(struct vki_f_owner_ex)); + } else if (ARG2 == VKI_F_GETLK || ARG2 == VKI_F_OFD_GETLK) { + struct vki_flock *lock = (struct vki_flock *) ARG3; + POST_FIELD_WRITE(lock->l_pid); +# if defined(VGP_x86_linux) || defined(VGP_mips64_linux) + } else if (ARG2 == VKI_F_GETLK64) { + struct vki_flock64 *lock = (struct vki_flock64 *) ARG3; + PRE_FIELD_WRITE("fcntl(lock->l_pid)", lock->l_pid); +# endif } } diff --git a/include/vki/vki-linux.h b/include/vki/vki-linux.h index fd3d7584a0..082d5f6173 100644 --- a/include/vki/vki-linux.h +++ b/include/vki/vki-linux.h @@ -1416,6 +1416,22 @@ struct vki_dirent64 { #define VKI_F_SETPIPE_SZ (VKI_F_LINUX_SPECIFIC_BASE + 7) #define VKI_F_GETPIPE_SZ (VKI_F_LINUX_SPECIFIC_BASE + 8) +struct vki_flock { + short l_type; + short l_whence; + __vki_kernel_off_t l_start; + __vki_kernel_off_t l_len; + __vki_kernel_pid_t l_pid; +}; + +struct vki_flock64 { + short l_type; + short l_whence; + __vki_kernel_loff_t l_start; + __vki_kernel_loff_t l_len; + __vki_kernel_pid_t l_pid; +}; + //---------------------------------------------------------------------- // From linux-2.6.8.1/include/linux/sysctl.h //---------------------------------------------------------------------- diff --git a/memcheck/tests/arm64-linux/scalar.c b/memcheck/tests/arm64-linux/scalar.c index cd8cb2af99..fd49db6c7f 100644 --- a/memcheck/tests/arm64-linux/scalar.c +++ b/memcheck/tests/arm64-linux/scalar.c @@ -279,7 +279,7 @@ int main(void) // For F_GETLK the 3rd arg is 'lock'. On x86, this fails w/EBADF. But // on amd64 in 32-bit mode it fails w/EFAULT. We don't check the 1st two // args for the reason given above. - GO(__NR_fcntl, "(GETLK) 1s 0m"); + GO(__NR_fcntl, "(GETLK) 1s 5m"); SY(__NR_fcntl, -1, F_GETLK, x0); FAIL; //FAILx(EBADF); // __NR_mpx arm64 doesn't implement mpx diff --git a/memcheck/tests/arm64-linux/scalar.stderr.exp b/memcheck/tests/arm64-linux/scalar.stderr.exp index fdfdb371a2..8f9d8b6cab 100644 --- a/memcheck/tests/arm64-linux/scalar.stderr.exp +++ b/memcheck/tests/arm64-linux/scalar.stderr.exp @@ -271,12 +271,37 @@ Syscall param fcntl(arg) contains uninitialised byte(s) by 0x........: main (scalar.c:277) ----------------------------------------------------- - 25: __NR_fcntl (GETLK) 1s 0m + 25: __NR_fcntl (GETLK) 1s 5m ----------------------------------------------------- Syscall param fcntl(lock) contains uninitialised byte(s) ... by 0x........: main (scalar.c:283) +Syscall param fcntl(lock->l_type) points to unaddressable byte(s) + ... + by 0x........: main (scalar.c:283) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + +Syscall param fcntl(lock->l_whence) points to unaddressable byte(s) + ... + by 0x........: main (scalar.c:283) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + +Syscall param fcntl(lock->l_start) points to unaddressable byte(s) + ... + by 0x........: main (scalar.c:283) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + +Syscall param fcntl(lock->l_len) points to unaddressable byte(s) + ... + by 0x........: main (scalar.c:283) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + +Syscall param fcntl(lock->l_pid) points to unaddressable byte(s) + ... + by 0x........: main (scalar.c:283) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + ----------------------------------------------------- 154: __NR_setpgid 2s 0m ----------------------------------------------------- @@ -555,6 +580,9 @@ Syscall param setpriority(which) contains uninitialised byte(s) ... by 0x........: main (scalar.c:458) + +More than 100 errors detected. Subsequent errors +will still be recorded, but in less detail than before. Syscall param setpriority(who) contains uninitialised byte(s) ... by 0x........: main (scalar.c:458) @@ -579,9 +607,6 @@ Syscall param statfs(path) points to unaddressable byte(s) by 0x........: main (scalar.c:466) Address 0x........ is not stack'd, malloc'd or (recently) free'd - -More than 100 errors detected. Subsequent errors -will still be recorded, but in less detail than before. Syscall param statfs(buf) points to unaddressable byte(s) ... by 0x........: main (scalar.c:466) diff --git a/memcheck/tests/x86-linux/scalar.c b/memcheck/tests/x86-linux/scalar.c index 6c6089196d..47e4d65db6 100644 --- a/memcheck/tests/x86-linux/scalar.c +++ b/memcheck/tests/x86-linux/scalar.c @@ -279,7 +279,7 @@ int main(void) // For F_GETLK the 3rd arg is 'lock'. On x86, this fails w/EBADF. But // on amd64 in 32-bit mode it fails w/EFAULT. We don't check the 1st two // args for the reason given above. - GO(__NR_fcntl, "(GETLK) 1s 0m"); + GO(__NR_fcntl, "(GETLK) 1s 5m"); SY(__NR_fcntl, -1, F_GETLK, x0); FAIL; //FAILx(EBADF); // __NR_mpx 56 diff --git a/memcheck/tests/x86-linux/scalar.stderr.exp b/memcheck/tests/x86-linux/scalar.stderr.exp index ebee0640ac..2d687fa035 100644 --- a/memcheck/tests/x86-linux/scalar.stderr.exp +++ b/memcheck/tests/x86-linux/scalar.stderr.exp @@ -598,21 +598,46 @@ Syscall param fcntl(arg) contains uninitialised byte(s) by 0x........: main (scalar.c:277) ----------------------------------------------------- - 55: __NR_fcntl (GETLK) 1s 0m + 55: __NR_fcntl (GETLK) 1s 5m ----------------------------------------------------- Syscall param fcntl(lock) contains uninitialised byte(s) ... by 0x........: main (scalar.c:283) + +More than 100 errors detected. Subsequent errors +will still be recorded, but in less detail than before. +Syscall param fcntl(lock->l_type) points to unaddressable byte(s) + ... + by 0x........: main (scalar.c:283) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + +Syscall param fcntl(lock->l_whence) points to unaddressable byte(s) + ... + by 0x........: main (scalar.c:283) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + +Syscall param fcntl(lock->l_start) points to unaddressable byte(s) + ... + by 0x........: main (scalar.c:283) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + +Syscall param fcntl(lock->l_len) points to unaddressable byte(s) + ... + by 0x........: main (scalar.c:283) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + +Syscall param fcntl(lock->l_pid) points to unaddressable byte(s) + ... + by 0x........: main (scalar.c:283) + Address 0x........ is not stack'd, malloc'd or (recently) free'd + ----------------------------------------------------- 56: __NR_mpx ni ----------------------------------------------------- ----------------------------------------------------- 57: __NR_setpgid 2s 0m ----------------------------------------------------- - -More than 100 errors detected. Subsequent errors -will still be recorded, but in less detail than before. Syscall param setpgid(pid) contains uninitialised byte(s) ... by 0x........: main (scalar.c:291)