[Valgrind-developers] [valgrind] PRE(sys_prlimit64): Check ARG3 and ARG4 ML_(safe_to_deref) up front

[Mark W. <ma...@so...>]

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=f51744ed2d1f07c814b72853ca946da3c94de0f1

commit f51744ed2d1f07c814b72853ca946da3c94de0f1
Author: Mark Wielaard <ma...@kl...>
Date:   Tue May 20 12:09:13 2025 +0200

    PRE(sys_prlimit64): Check ARG3 and ARG4 ML_(safe_to_deref) up front
    
    The previous commit 859d267a456c "PR504341: Prevent LTP setrlimit05
    syscall test from crashing valgrind" changed the checking logic of the
    PRE handler changing the if-else control flow. Do the ARG3 and ARG4
    ML_(safe_to_deref) checking up front and return EFAULT early so the
    later checking logic doesn't need to change.
    
    https://bugs.kde.org/show_bug.cgi?id=504341
    
    Suggested-by: Matthias <zz...@ge...>

Diff:
---
 coregrind/m_syswrap/syswrap-linux.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index 470635f565..fa2ab9e9fe 100644
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -2295,19 +2295,27 @@ PRE(sys_prlimit64)
                  vki_pid_t, pid, unsigned int, resource,
                  const struct rlimit64 *, new_rlim,
                  struct rlimit64 *, old_rlim);
-   if (ARG3)
+   if (ARG3) {
       PRE_MEM_READ( "rlimit64(new_rlim)", ARG3, sizeof(struct vki_rlimit64) );
-   if (ARG4)
+      if (!ML_(safe_to_deref)((void*)(Addr)ARG3, sizeof(struct vki_rlimit64))) {
+         SET_STATUS_Failure(VKI_EFAULT);
+         return;
+      }
+   }
+   if (ARG4) {
       PRE_MEM_WRITE( "rlimit64(old_rlim)", ARG4, sizeof(struct vki_rlimit64) );
-
-   if (ARG3) {
-      if (ML_(safe_to_deref)( (void*)(Addr)ARG3, sizeof(struct vki_rlimit64) )) {
-         if (((struct vki_rlimit64 *)(Addr)ARG3)->rlim_cur
-              > ((struct vki_rlimit64 *)(Addr)ARG3)->rlim_max) {
-            SET_STATUS_Failure( VKI_EINVAL );
-         }
+      if (!ML_(safe_to_deref)((void*)(Addr)ARG4, sizeof(struct vki_rlimit64))) {
+         SET_STATUS_Failure(VKI_EFAULT);
+         return;
       }
-   } else if (ARG1 == 0 || ARG1 == VG_(getpid)()) {
+   }
+
+   if (ARG3 &&
+       ((struct vki_rlimit64 *)(Addr)ARG3)->rlim_cur
+        > ((struct vki_rlimit64 *)(Addr)ARG3)->rlim_max) {
+      SET_STATUS_Failure( VKI_EINVAL );
+   }
+   else if (ARG1 == 0 || ARG1 == VG_(getpid)()) {
       switch (ARG2) {
       case VKI_RLIMIT_NOFILE:
          SET_STATUS_Success( 0 );