Bug 309554

*** Bug 349952 has been marked as a duplicate of this bug. ***

*** Bug 369027 has been marked as a duplicate of this bug. ***

*** Bug 368916 has been marked as a duplicate of this bug. ***

Created attachment 183988 [details]
proposed patch

Created attachment 184624 [details]
updated patch

In this needed?

+   if (!ML_(safe_to_deref)((void*)(Addr)ARG1, ARG2))
+      PRE_MEM_READ("sys_remap_file_pages(addr)", ARG1, ARG2);

My understanding of the manpage is that this syscall just rearranges the order of the mapping between pages in a file and pages in memory. I don't think that it reads or writes any memory.

(In reply to Paul Floyd from comment #6)
> In this needed?
> 
> +   if (!ML_(safe_to_deref)((void*)(Addr)ARG1, ARG2))
> +      PRE_MEM_READ("sys_remap_file_pages(addr)", ARG1, ARG2);
> 
> My understanding of the manpage is that this syscall just rearranges the
> order of the mapping between pages in a file and pages in memory. I don't
> think that it reads or writes any memory.

The PRE_MEM_READ("sys_remap_file_pages(addr)", ARG1, ARG2) should always be done unconditionally.
We want to warn if any addr in the array provided to the kernel contains undefined bits

We only need the ML_(safe_to_deref) check after that if we are going to use those addresses ourselves.
But I am not fully clear on whether we want to track any of this through the address manager and/or call
notify_core_and_tool_of_mmap and notify_core_and_tool_of_mprotect.
But it says "The prot argument must be specified as 0" so the protection doesn't change.
And for flags it says "all  flags other than MAP_NONBLOCK are ignored".

If I understand things correctly all remap_file_pages does is move the file mapping around inside an
already existing mmap for a file. Then we don't really care I guess. All we are interested in is the
original mmap of the fd?

If so, then I think the patch is ok, except for the if (!ML_(safe_to_deref)((void*)(Addr)ARG1, ARG2)).
Just drop that and do the PRE_MEM_READ("sys_remap_file_pages(addr)", ARG1, ARG2); unconditionally.

Created attachment 184670 [details]
updated patch

I've dropped that bogus condition.   This patch regtests and ltp-tests fine. But that, of course, doesn't mean that it's correct.  It only tries to address the existing comments (and thanks a ton for those!).

(In reply to mcermak from comment #8)
> Created attachment 184670 [details]
> updated patch
> 
> I've dropped that bogus condition.   This patch regtests and ltp-tests fine.
> But that, of course, doesn't mean that it's correct.  It only tries to
> address the existing comments (and thanks a ton for those!).

I think we should go with this variant. We probably don't need any "hooks" into the valgrind memspace manager.
If the ltp tests work OK with this then it should be good to go.

commit 818e7661ecac5e0fb60f19f28ad64cd3bff6cdd9
Author: Martin Cermak <mcermak@redhat.com>
Date:   Wed Sep 3 16:02:26 2025 +0200

    Wrap the remap_file_pages syscall
    
    The remap_file_pages() system call is used to create a nonlinear
    mapping, that is, a mapping in which the pages of the file are
    mapped into a nonsequential order in memory.  It is deprecated
    but in some cases it may still be used.  LTP remap_file_pages01
    and remap_file_pages02 test-cover it.
    
    Declare a remap_file_pages wrapper in priv_syswrap-linux.h
    and hook it for {amd64,arm,arm64,mips64,ppc32,ppc64,riscv64,s390x\
    ,x86}- linux using LINX_ with PRE handler in syswrap-linux.c
    
    https://bugs.kde.org/show_bug.cgi?id=309554