Bug 450437 - Warn for execve syscall with argv or argv[0] being NULL.
Summary: Warn for execve syscall with argv or argv[0] being NULL.
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: 3.18.1
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Mark Wielaard
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-02-17 10:29 UTC by Mark Wielaard
Modified: 2022-04-07 10:28 UTC (History)
2 users (show)

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


Attachments
Warn for execve syscall with argv or argv[0] being NULL (11.78 KB, text/plain)
2022-02-17 10:29 UTC, Mark Wielaard
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Wielaard 2022-02-17 10:29:55 UTC
Created attachment 146855 [details]
Warn for execve syscall with argv or argv[0] being NULL

For execve valgrind would silently fail when argv was NULL or
unadressable. Make sure that this produces a warning under memcheck.

The linux kernel accepts argv[0] being NULL, but most other kernels
don't since posix says it should be non-NULL and it causes argc to
be zero which is unexpected and might cause security issues.

This adjusts some testcases so they don't rely on execve succeeding
when argv is NULL and expect warnings about argv or argv[0] being
NULL or unaddressable.

See also https://lwn.net/Articles/882799/
Comment 1 Paul Floyd 2022-03-29 13:40:00 UTC
FreeBSD doesn't allow a zero argc (which is what you get with a NULL argv). This was only enforced relatively recently

https://reviews.freebsd.org/R10:773fa8cd136a5775241c3e3a70f1997633ebeedf

FreeBSD kernel testcase for this

https://reviews.freebsd.org/D34045
Comment 2 Mark Wielaard 2022-04-02 20:06:57 UTC
Paul, would you be able to test the attached patch on a freebsd system and see if all tests (none/tests/execve, memcheck/tests/execve1 and memchck/tests/execve2) pass?
Comment 3 Paul Floyd 2022-04-03 09:48:51 UTC
I'll test later today.
Comment 4 Paul Floyd 2022-04-03 14:36:24 UTC
Minor thing, missing EXTRA_DISTs

memcheck/tests/linux/Makefile.am:1: error: sys-execveat.stderr.exp.orig is missing in EXTRA_DIST
memcheck/tests/arm64-linux/Makefile.am:1: error: scalar.stderr.exp.orig is missing in EXTRA_DIST
memcheck/tests/x86-linux/Makefile.am:1: error: scalar.stderr.exp.orig is missing in EXTRA_DIST
memcheck/tests/Makefile.am:1: error: execve1.stderr.exp.orig is missing in EXTRA_DIST
memcheck/tests/Makefile.am:1: error: execve2.stderr.exp.orig is missing in EXTRA_DIST

scalar execve test has an extra message for argv that looks better, so will need updating

There is a similar diff in the fexecve test
Comment 5 Paul Floyd 2022-04-03 14:38:45 UTC
Was too quick with the EXTRA_DISTs, they are the patch cruft not the real files

There will probably be at least one more .exp (x86 scalar)
Comment 6 Mark Wielaard 2022-04-03 19:22:25 UTC
So do I assume correctly that the generic none/tests/execve, memcheck/tests/execve1 and memchck/tests/execve2 tests PASS with this patch on freebsd, but that the freebsd specific memcheck/tests/freebsd/scalar.vgtest fails and needs updates to  memcheck/tests/freebsd/scalar.stderr.exp and /memcheck/tests/freebsd/scalar.stderr.exp-x86?

Also how many/which of the freebsd specific fexecve tests fail?
memcheck/tests/freebsd/fexecve.vgtest
none/tests/freebsd/fexecve_hw1.vgtest
none/tests/freebsd/fexecve_txt.vgtest
none/tests/freebsd/fexecve_script2.vgtest
none/tests/freebsd/fexecve_hw2.vgtest
none/tests/freebsd/fexecve_script1.vgtest

Are these failures that look "reasonable" and need updates to the .exp files for the new warnings or are they bugs?
Comment 7 Paul Floyd 2022-04-03 19:31:44 UTC
Just scalar and fexecve, the other fexecve tests were OK. 

The diffs look reasonable (just extra errors for argv/argv[0] like on Linux)
Comment 8 Mark Wielaard 2022-04-06 21:07:31 UTC
commit 8eb547054a051a00742b1b9e1b381015fafeacb9
Author: Mark Wielaard <mark@klomp.org>
Date:   Wed Feb 16 22:56:31 2022 +0100

    Warn for execve syscall with argv or argv[0] being NULL.
    
    For execve valgrind would silently fail when argv was NULL or
    unadressable. Make sure that this produces a warning under memcheck.
    
    The linux kernel accepts argv[0] being NULL, but most other kernels
    don't since posix says it should be non-NULL and it causes argc to
    be zero which is unexpected and might cause security issues.
    
    This adjusts some testcases so they don't rely on execve succeeding
    when argv is NULL and expect warnings about argv or argv[0] being
    NULL or unaddressable.
    
    https://bugs.kde.org/show_bug.cgi?id=450437

Paul, could you push the needed .exp file changes for freebsd?
Comment 9 Paul Floyd 2022-04-07 06:53:03 UTC
Done
Comment 10 Mark Wielaard 2022-04-07 10:28:46 UTC
Closing now that the freebsd exp files have also been updated:

commit 19584a93d15e4a0f027ccd478b0e70caaa594d05
Author: Paul Floyd <pjfloyd@wanadoo.fr>
Date:   Thu Apr 7 07:56:27 2022 +0200

    Update FreeBSD [f]execve expecteds

    As requested by Mark, for https://bugs.kde.org/show_bug.cgi?id=450437