Check mmap fd is valid, if used, and fail early with EBADF if not

mmap should fail with EBADF if the given fd is bad (or used by valgrind
itself) when used (flags does not contain MAP_ANONYMOUS).

Check both with ML_(fd_allowed) (which might only warn) and fcntl
(VKI_F_GETFD) to see if the file descriptor is valid. Fail early so
the address space manager and the actual mmap call don't do
unnecessary work (and might fail with a different error code).

This fixes the LTP mmap08 testcase.

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

diff --git a/NEWS b/NEWS
index 796d9716e5c102e3a0d760092d35f577b61d8992..868d4218fe43c508cf83ce59d63df9ee62b5fb86 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -56,6 +56,7 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 506499  Unhandled syscall 592 (exterrctl - FreeBSD
 506795  Better report which clone flags are problematic
 506930  valgrind allows SIGKILL being reset to SIG_DFL
+506970  mmap needs an EBADF fd_allowed check
 
 To see details of a given bug, visit
   https://bugs.kde.org/show_bug.cgi?id=XXXXXX

diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c
index 50415a2faa518eaf1ae610a84d23ceab78390462..7ad280980777629250446abfaff176dae6d61404 100644 (file)
--- a/coregrind/m_syswrap/syswrap-generic.c
+++ b/coregrind/m_syswrap/syswrap-generic.c
@@ -2653,6 +2653,17 @@ ML_(generic_PRE_sys_mmap) ( ThreadId tid,
    VG_(core_panic)("can't use ML_(generic_PRE_sys_mmap) on Darwin");
 #  endif
 
+   /* fd (arg4) is only used when flags (arg4) does not contain
+      MAP_ANONYMOUS. ML_(fd_allowed) might just warn (with --track-fds)
+      and not fail, unless it is a Valgrind owned file descriptor.
+      So also check with fcntl (F_GETFD) to know if it really is a bad
+      fd. Fail early in that case with EBADF.  */
+   if (!(arg4 & VKI_MAP_ANONYMOUS)
+       && (!ML_(fd_allowed)(arg5, "mmap", tid, False)
+           || VG_(fcntl) (arg5, VKI_F_GETFD, 0) < 0)) {
+      return VG_(mk_SysRes_Error)( VKI_EBADF );
+   }
+
    if (arg2 == 0) {
       /* SuSV3 says: If len is zero, mmap() shall fail and no mapping
          shall be established. */