Bug 424018 - Valgrind crashes with --trace-syscalls=yes if syscalls use PRINT %s with a non-dereferenceable string
Summary: Valgrind crashes with --trace-syscalls=yes if syscalls use PRINT %s with a no...
Status: REPORTED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-07-09 07:54 UTC by Paul Floyd
Modified: 2020-07-09 07:54 UTC (History)
0 users

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Floyd 2020-07-09 07:54:26 UTC
SUMMARY

As a quick example, in the memcheck/tests/x86-linux directory if I run

../../../vg-in-place --trace-syscalls=yes ./scalar < scalar.c

then I get

-----------------------------------------------------                                                                       
 11:         __NR_execve 3s 1m                                                                                              
-----------------------------------------------------                                                                       
SYSCALL[13743,1](4) ... [async] --> Success(0x8b)                                                                           
SYSCALL[13743,1](11) --13743-- VALGRIND INTERNAL ERROR: Valgrind received a signal 11 (SIGSEGV) - exiting                   
--13743-- si_code=1;  Faulting address: 0x1;  sp: 0x8288bb28                                                                

valgrind: the 'impossible' happened:
   Killed by fatal signal           

host stacktrace:
==13743==    at 0x5801E368: myvprintf_str (m_debuglog.c:623)
==13743==    by 0x5801EBAD: vgPlain_debugLog_vprintf (m_debuglog.c:1027)
==13743==    by 0x58028850: vprintf_WRK (m_libcprint.c:647)             
==13743==    by 0x58028C11: vgPlain_printf (m_libcprint.c:671)          
==13743==    by 0x58070414: vgSysWrap_generic_sys_execve_before (syswrap-generic.c:3152)
==13743==    by 0x58014F49: vgPlain_client_syscall (syswrap-main.c:1914)                
==13743==    by 0x580123F8: handle_syscall (scheduler.c:1208)                           
==13743==    by 0x58013A19: vgPlain_scheduler (scheduler.c:1526)                        
==13743==    by 0x58076D03: run_a_thread_NORETURN (syswrap-linux.c:101)

The code in scalar.c is

   // __NR_execve 11
   GO(__NR_execve, "3s 1m");
   SY(__NR_execve, x0 + 1, x0 + 1, x0); FAIL;

The execve signature is

int execve(const char *filename, char *const argv[],
                  char *const envp[]);


As the stacktrace says, this is handled in syswrap-generic.c


   PRINT("sys_execve ( %#" FMT_REGWORD "x(%s), %#" FMT_REGWORD "x, %#"
         FMT_REGWORD "x )", ARG1, (HChar*)(Addr)ARG1, ARG2, ARG3);

The problem is with the "(%s)" dereferencing (HChar*)(Addr)ARG1, which is 1.

I suggest adding a new function such as

const HChar* ML_(sanitize_string)(Addr ptr)
{
   if (ML_(safe_to_deref)((const void *)ptr, 1)) {
      return (const HChar*)pr;
   } else {
      return "invalid string";
   }
}

and changing the PRINT to

   PRINT("sys_execve ( %#" FMT_REGWORD "x(%s), %#" FMT_REGWORD "x, %#"
         FMT_REGWORD "x )", ARG1, ML_(sanitize_sring)((Addr)ARG1), ARG2, ARG3);

I'm not providing a patch as there are many other similar examples.

It might also be possible to change VG_(printf) to handle non-dereferenceable pointers. That would require fewer code changes at the expense of always checking pointers.