Bug 505228

Created attachment 182042 [details]
proposed patch

Tested on rhel10 aarch64, ppc64le, s390, and x86_64.  Example:

el10 ppc64le # strace -qq -emseal --raw=all --signal='!all' -f -o /tmp/xxx ./auxprogs/auxchecks/ltp-full-20250530/testcases/kernel/syscalls/mseal/mseal01 >&/dev/null; echo "-------------"; cat /tmp/xxx
-------------
504824 syscall_0x193(0, 0x7fffc55503d0, 0, 0x20, 0x30, 0x10077f00) = -1 ENOSYS (Function not implemented)
504825 mseal(0x7fff8df30000, 0x10000, 0) = 0
504826 mseal(0x7fff8df30000, 0x10000, 0) = 0
504827 mseal(0x7fff8df30000, 0x10000, 0) = 0
504828 mseal(0x7fff8df30000, 0x10000, 0) = 0
504829 mseal(0x7fff8df30000, 0x10000, 0) = 0
504830 mseal(0x7fff8df30000, 0x10000, 0) = 0
el10 ppc64le # ./vg-in-place --tool=none --trace-syscalls=yes ./auxprogs/auxchecks/ltp-full-20250530/testcases/kernel/syscalls/mseal/mseal01 |& grep ^SYSCALL.*sys_mseal
SYSCALL[504838,1](462) sys_mseal ( 0x4330000, 65536, 0x0, )[sync] --> Success(0x0) 
SYSCALL[504839,1](462) sys_mseal ( 0x4330000, 65536, 0x0, )[sync] --> Success(0x0) 
SYSCALL[504840,1](462) sys_mseal ( 0x4330000, 65536, 0x0, )[sync] --> Success(0x0) 
SYSCALL[504841,1](462) sys_mseal ( 0x4330000, 65536, 0x0, )[sync] --> Success(0x0) 
SYSCALL[504842,1](462) sys_mseal ( 0x4330000, 65536, 0x0, )[sync] --> Success(0x0) 
SYSCALL[504843,1](462) sys_mseal ( 0x4330000, 65536, 0x0, )[sync] --> Success(0x0) 
el10 ppc64le #

Looks good in general, if the program uses mseal on their own address blocks that should in general be harmless for valgrind since we don't want to unmmap or mprotect normal pages after the program has. But the current check is wrong imho:

+PRE(sys_mseal)
+{
+    /* int mseal(void *addr, size_t len, unsigned long flags) */
+    PRINT("sys_mseal ( %#" FMT_REGWORD "x, %" FMT_REGWORD "u, %#" FMT_REGWORD "x, )", ARG1, ARG2, ARG3);
+    PRE_REG_READ3(int, "mseal", void *, addr,  vki_size_t, len, int, flags);
+    if (ARG1 != 0) {
+        PRE_MEM_READ( "mseal (addr)", ARG1, ARG2 ); 
+    }
+}

First, is 0/NULL really special?
Second, PRE_MEM_READ checks if all memory given to the kernel is addressable and defined.
But I think it doesn't really have to be defined. It does however have to be addresses the program itself manages, not valgrind memory.
So instead I think the check should be something like:

   if (!ML_(valid_client_addr)(ARG1, ARG2, tid, "mseal"))
      SET_STATUS_Failure(VKI_ENOMEM);

So the syscall fails early if it isn't program memory.

Created attachment 182329 [details]
proposed patch

My thinko was that it is an address that needs to be dereferenced and written to, just like with the PRE(sys_time) example in README_MISSING_SYSCALL_OR_IOCTL which obviously isn't the case.  In this case 0 isn't a special value of ARG1 because mseal is supposed to be able to apply to any part of the program virtual address space, incl it's very beginning.  Please correct me if I'm getting it wrong.

Thanks for your review, Mark.  Updated patch attached.

Looks good. Thanks. Pushed as:

commit 9775bc496e4b6f80dec993e5d147356ebbe29fe3
Author: Martin Cermak <mcermak@redhat.com>
Date:   Tue Jun 17 13:51:48 2025 +0200

    Wrap linux specific mseal syscall
    
    mseal takes address, size and flags.  Flags are reserved for
    future use. Modern CPUs support memory permissions such as RW and
    NX bits.  The mseal syscall takes address and size parameters to
    additionally protect memory mapping against modifications.
    
    FTR: https://docs.kernel.org/userspace-api/mseal.html
    
    Declare a sys_mseal wrapper in priv_syswrap-linux.h and hook it
    for {amd64,arm,arm64,mips64,nanomips,ppc32,ppc64,riscv64,s390x,x86}-linux
    using LINX_ with PRE handler in syswrap-linux.c
    
    https://bugs.kde.org/show_bug.cgi?id=505228