Bug 433641 - Syscall param fstatat(file_name) points to unaddressable byte(s) at __fxstatat / statx_generic / statx / std::sys::unix::fs::try_statx
Summary: Syscall param fstatat(file_name) points to unaddressable byte(s) at __fxstata...
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-02-26 08:34 UTC by mh
Modified: 2021-02-28 21:37 UTC (History)
1 user (show)

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


Attachments
Patch (1.41 KB, patch)
2021-02-26 08:34 UTC, mh
Details

Note You need to log in before you can comment on or make changes to this bug.
Description mh 2021-02-26 08:34:21 UTC
Created attachment 136182 [details]
Patch

SUMMARY

https://sourceware.org/git/?p=valgrind.git;a=commit;h=2a7d3ae768f9e5b29acd5cb743c3fb13640a391c
was a fix for an older occurrence of the same problem.

The problem is that rust calls statx with two pointers set to NULL to test whether the system call is supported.

It actually does it in two steps, which is why the previous fix didn't end up working completely.

The first step is to check whether the libc has the statx function. If it does, that's used.

If not, we hit the second step, which is to use syscall(SYS_statx, ...). That's actually what was fixed in the commit mentioned above.

Back when that commit was made, the error had been found against a glibc that didn't have the statx function. This new error happens when running against a glibc that does support statx, but running on a kernel that doesn't. In this case, glibc it falls back to fstatat:
https://sourceware.org/git/?p=glibc.git;a=blob;f=io/statx_generic.c;h=797d08571aff3f3754ed23b48df1d6fb3e95b984;hb=035c012e32c11e84d64905efaf55e74f704d3668#l60


STEPS TO REPRODUCE
1. Install rust
2. cargo new foo
3. cd foo
4. cat >src/main.rs <<EOF
fn main() -> std::io::Result<()> {
    std::fs::metadata("/some/file/path.txt")?;
    Ok(())
}
EOF
5. cargo build
6. valgrind target/debug/foo

OBSERVED RESULT


```
==3074== Syscall param fstatat(file_name) points to unaddressable byte(s)
==3074==    at 0x4980E49: __fxstatat (fxstatat.c:43)
==3074==    by 0x4980C21: statx_generic (statx_generic.c:53)
==3074==    by 0x4980C21: statx (statx.c:39)
==3074==    by 0x129400: statx (library/std/src/sys/unix/weak.rs:134)
==3074==    by 0x129400: std::sys::unix::fs::try_statx (library/std/src/sys/unix/fs.rs:123)
==3074==    by 0x1288D9: std::sys::unix::fs::stat (library/std/src/sys/unix/fs.rs:1105)
==3074==    by 0x10D43C: std::fs::metadata (fs.rs:1567)
==3074==    by 0x10D6F2: foo::main (src/main.rs:2)
==3074==    by 0x10D1AA: core::ops::function::FnOnce::call_once (function.rs:227)
==3074==    by 0x10DB1D: std::sys_common::backtrace::__rust_begin_short_backtrace (backtrace.rs:125)
==3074==    by 0x10DBE0: std::rt::lang_start::{{closure}} (rt.rs:66)
==3074==    by 0x127696: call_once<(),Fn<()>> (function.rs:259)
==3074==    by 0x127696: do_call<&Fn<()>,i32> (library/std/src/panicking.rs:379)
==3074==    by 0x127696: try<i32,&Fn<()>> (library/std/src/panicking.rs:343)
==3074==    by 0x127696: catch_unwind<&Fn<()>,i32> (library/std/src/panic.rs:396)
==3074==    by 0x127696: std::rt::lang_start_internal (library/std/src/rt.rs:51)
==3074==    by 0x10DBB6: std::rt::lang_start (rt.rs:65)
==3074==    by 0x10D829: main (in /builds/worker/foo/target/debug/foo)
==3074==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
```

EXPECTED RESULT

No error

SOFTWARE/OS VERSIONS
Linux kernel 4.4
glibc 2.28.
Comment 1 Mark Wielaard 2021-02-28 21:37:53 UTC
This is somewhat unfortunate since it removes another valid check for bogus addresses passed to the kernel. Maybe it is time for rust to come with a default suppression file?

But I can see why you want this when running Rust code under valgrind. Pushed as:

commit 5d45e212a66fe83f593693adf452d4ea78dcf1d3
Author: Mike Hommey <mh@glandium.org>
Date:   Fri Feb 26 17:09:52 2021 +0900

    sys_newfstatat: don't complain if |file_name| is NULL.
    
    This is a followup to 2a7d3ae76, in the case rust code runs against a
    glibc that supports statx but a kernel that doesn't, in which case glibc
    falls back to fstatat.
    
    https://bugs.kde.org/show_bug.cgi?id=433641