Discussion:
Valgrind: r16287 - in /trunk: NEWS coregrind/m_syswrap/syswrap-linux.c include/vki/vki-linux.h memcheck/tests/arm64-linux/scalar.c memcheck/tests/arm64-linux/scalar.stderr.exp memcheck/tests/x86-linux/scalar.c memcheck/tests/x86-linux/scalar.stderr.exp
(too old to reply)
s***@valgrind.org
2017-03-27 05:06:51 UTC
Permalink
Raw Message
Author: iraisr
Date: Mon Mar 27 06:06:32 2017
New Revision: 16287

Log:
fcntl syscall wrapper was missing flock structure check on Linux.
Fixes BZ#377930.

Modified:
trunk/NEWS
trunk/coregrind/m_syswrap/syswrap-linux.c
trunk/include/vki/vki-linux.h
trunk/memcheck/tests/arm64-linux/scalar.c
trunk/memcheck/tests/arm64-linux/scalar.stderr.exp
trunk/memcheck/tests/x86-linux/scalar.c
trunk/memcheck/tests/x86-linux/scalar.stderr.exp

Modified: trunk/NEWS
==============================================================================
--- trunk/NEWS (original)
+++ trunk/NEWS Mon Mar 27 06:06:32 2017
@@ -151,6 +151,7 @@
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)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Modified: trunk/coregrind/m_syswrap/syswrap-linux.c
==============================================================================
--- trunk/coregrind/m_syswrap/syswrap-linux.c (original)
+++ trunk/coregrind/m_syswrap/syswrap-linux.c Mon Mar 27 06:06:32 2017
@@ -5898,19 +5898,45 @@
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 @@
}
} 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
}
}


Modified: trunk/include/vki/vki-linux.h
==============================================================================
--- trunk/include/vki/vki-linux.h (original)
+++ trunk/include/vki/vki-linux.h Mon Mar 27 06:06:32 2017
@@ -1416,6 +1416,22 @@
#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
//----------------------------------------------------------------------

Modified: trunk/memcheck/tests/arm64-linux/scalar.c
==============================================================================
--- trunk/memcheck/tests/arm64-linux/scalar.c (original)
+++ trunk/memcheck/tests/arm64-linux/scalar.c Mon Mar 27 06:06:32 2017
@@ -279,7 +279,7 @@
// 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

Modified: trunk/memcheck/tests/arm64-linux/scalar.stderr.exp
==============================================================================
--- trunk/memcheck/tests/arm64-linux/scalar.stderr.exp (original)
+++ trunk/memcheck/tests/arm64-linux/scalar.stderr.exp Mon Mar 27 06:06:32 2017
@@ -271,12 +271,37 @@
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 @@
...
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 @@
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)

Modified: trunk/memcheck/tests/x86-linux/scalar.c
==============================================================================
--- trunk/memcheck/tests/x86-linux/scalar.c (original)
+++ trunk/memcheck/tests/x86-linux/scalar.c Mon Mar 27 06:06:32 2017
@@ -279,7 +279,7 @@
// 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

Modified: trunk/memcheck/tests/x86-linux/scalar.stderr.exp
==============================================================================
--- trunk/memcheck/tests/x86-linux/scalar.stderr.exp (original)
+++ trunk/memcheck/tests/x86-linux/scalar.stderr.exp Mon Mar 27 06:06:32 2017
@@ -598,21 +598,46 @@
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)

Loading...