Bug 506970 - mmap needs an EBADF fd_allowed check
Summary: mmap needs an EBADF fd_allowed check
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (other bugs)
Version First Reported In: 3.25.0
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks: 506971
  Show dependency treegraph
 
Reported: 2025-07-12 21:22 UTC by Mark Wielaard
Modified: 2025-07-17 10:40 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Wielaard 2025-07-12 21:22:34 UTC
The LTP mmap08 testcase fails with:
mmap08.c:28: TFAIL: mmap(NULL, page_sz, PROT_WRITE, MAP_FILE | MAP_SHARED, fd, 0) expected EBADF: EINVAL (22)
The test closes fd before calling mmap and expects EBADF not EINVAL.

There should probably be a fd_allowed check on arg5 in ML_(generic_PRE_sys_mmap)
Comment 1 Mark Wielaard 2025-07-16 12:04:20 UTC
Proposed patch https://code.wildebeest.org/git/user/mjw/valgrind/commit/?h=mmap-fd-check

diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c
index 50415a2fa..2ba3ca9df 100644
--- a/coregrind/m_syswrap/syswrap-generic.c
+++ b/coregrind/m_syswrap/syswrap-generic.c
@@ -2653,6 +2653,12 @@ ML_(generic_PRE_sys_mmap) ( ThreadId tid,
    VG_(core_panic)("can't use ML_(generic_PRE_sys_mmap) on Darwin");
 #  endif
 
+   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. */

Seems to do as expected, if you know that arg4 is flags, arg5 is fd, that if flags contains MAP_ANONYMOUS fd is ignored and that ML_(fd_allowed) might just warn (with --track-fds), so checking with fcntl (F_GETFD) is needed to know if it really is a bad fd.

Will add a comment with the above.
Comment 2 Mark Wielaard 2025-07-17 10:40:26 UTC
commit bd1e857cd493f4d1e64c3f5ae1061650644c666b
Author: Mark Wielaard <mark@klomp.org>
Date:   Wed Jul 16 02:45:39 2025 +0200

    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