linux: try to use libc getrandom to allow interposition #78785

cuviper · 2020-11-05T20:56:38Z

We'll try to use a weak getrandom symbol first, because that allows
things like LD_PRELOAD interposition. For example, perf measurements
might want to disable randomness to get reproducible results. If the
weak symbol is not found, we fall back to a raw SYS_getrandom call.

rust-highfive · 2020-11-05T20:56:41Z

r? @m-ou-se

(rust_highfive has picked a reviewer for you, use r? to override)

cuviper · 2020-11-05T20:58:01Z

cc @eddyb -- this may remove the need for the proc_macro NonRandomState hack in #78781.

m-ou-se · 2020-11-05T21:22:47Z

It might be a bit confusing if non-linux/android platforms have only syscall!{} that is already weak, but linux/android get both syscall!{} and weak_syscall!{} where the first one is not weak.

Considering syscall!{} is only used for one other case on linux (statx), maybe it's good to just make syscall!{} weak in all cases, instead of adding a macro. Then that function can also be overridden with LD_PRELOAD.

cuviper · 2020-11-05T21:50:11Z

maybe it's good to just make syscall!{} weak in all cases, instead of adding a macro.

I thought about that. I was also looking at a few bare calls to libc::syscall that perhaps should use the macro, and SYS_futex gave me pause because there is no libc wrapper for that. But maybe it's still reasonable to do as you say, and even convert other calls (I see copy_file_range), leaving futex alone.

m-ou-se · 2020-11-05T21:57:14Z

leaving futex alone.

Yeah, SYS_futex is definitely the weird one here. I think copy_file_range is the only other syscall() call. Converting that one too sounds good.

cuviper · 2020-11-05T22:37:07Z

Done!

eddyb · 2020-11-06T03:19:51Z

cc @eddyb -- this may remove the need for the proc_macro NonRandomState hack in #78781.

Hmm. We have avoided using plain HashMap/HashSet inside rustc itself, so this could go either way I suppose.

cuviper · 2020-11-06T19:48:32Z

@eddyb I guess it's debatable whether proc_macro needs that kind of hash safety when the rest of the compiler doesn't care. You labeled it a hack though, so I thought you'd want a way to avoid that. Either way, this change to getrandom lets you influence randomness in external crates too -- serde_derive etc.

the8472 · 2020-11-06T21:23:34Z

library/std/src/sys/unix/weak.rs

+                    concat_idents!(SYS_, $name),
+                    $($arg_name as c_long),*
+                ) as $ret
+            }


Would this work for sendfile? The sendfile64 libc wrapper is available on 32bit and 64bit. But on 64bit only the raw sendfile syscall is available since it's already 64bit-wide.

The macro is assuming that the libc function name matches SYS_name exactly. We would need something new to deal with different names among libc and 32/64-bit syscalls.

It's also meant for cases where the function is relatively new, not yet in the minimum libc we intend to support, so the syscall is a fallback. I don't think that's the case for sendfile64, because it has been around since glibc 2.3.

bors · 2020-11-14T15:00:12Z

☔ The latest upstream changes (presumably #75272) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

We'll try to use a weak `getrandom` symbol first, because that allows things like `LD_PRELOAD` interposition. For example, perf measurements might want to disable randomness to get reproducible results. If the weak symbol is not found, we fall back to a raw `SYS_getrandom` call.

cuviper · 2020-11-16T19:43:03Z

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

m-ou-se · 2020-11-17T19:28:35Z

@bors r+

bors · 2020-11-17T19:28:36Z

📌 Commit cd22381 has been approved by m-ou-se

Rollup of 11 pull requests Successful merges: - rust-lang#78361 (Updated the list of white-listed target features for x86) - rust-lang#78785 (linux: try to use libc getrandom to allow interposition) - rust-lang#78999 (stability: More precise location for deprecation lint on macros) - rust-lang#79039 (Tighten the bounds on atomic Ordering in std::sys::unix::weak::Weak) - rust-lang#79079 (Turn top-level comments into module docs in MIR visitor) - rust-lang#79114 (add trailing_zeros and leading_zeros to non zero types) - rust-lang#79131 (Enable AVX512 *epi64 variants by updating stdarch) - rust-lang#79133 (bootstrap: use the same version number for rustc and cargo) - rust-lang#79145 (Fix handling of panic calls) - rust-lang#79151 (Fix typo in `std::io::Write` docs) - rust-lang#79158 (type is too big -> values of the type are too big) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

RalfJung · 2020-11-19T08:24:45Z

library/std/src/sys/unix/rand.rs

+            fn getrandom(
+                buffer: *mut libc::c_void,
+                length: libc::size_t,
+                flags: libc::c_uint


Even though this says c_uint, in Miri we are seeing the flags argument being passed as a ptr-sized value when dlsym returns NULL. Before this change it was always of size 4. Did something go wrong with the type logic?

RalfJung · 2020-11-19T08:44:17Z

library/std/src/sys/unix/weak.rs

+            } else {
+                syscall(
+                    concat_idents!(SYS_, $name),
+                    $($arg_name as c_long),*


This is probably where all arguments are made ptr-sized. It is not clear to me what that is done (we do have proper type information after all!), nor why it should even be allowed. Isn't it UB when the caller and callee types do not match?

This is to make sure they're all integral/pointer values that fit in a machine word. As long as that's the case, the exact type doesn't matter. The ABI of a variadic function like syscall will just put each argument into their own register, extended to a usize/c_long.

Is this a guarantee made by C? For all ABIs? And what happens with the value that is passed, logically, to the syscall? It should get truncated to the smaller of "type used" and "type expected", I guess?

I propose we do something like #79196.

rust-highfive assigned m-ou-se Nov 5, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 5, 2020

m-ou-se added O-linux Operating system: Linux T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 5, 2020

cuviper force-pushed the weak-getrandom branch from 4a755ed to de12492 Compare November 5, 2020 22:20

the8472 reviewed Nov 6, 2020

View reviewed changes

cuviper added 3 commits November 16, 2020 11:26

Try weak symbols for all linux syscall! wrappers

a035626

Use syscall! for copy_file_range too

cd22381

cuviper force-pushed the weak-getrandom branch from de12492 to cd22381 Compare November 16, 2020 19:42

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 17, 2020

m-ou-se mentioned this pull request Nov 18, 2020

Rollup of 11 pull requests #79167

Merged

bors merged commit 61134aa into rust-lang:master Nov 18, 2020

rustbot added this to the 1.50.0 milestone Nov 18, 2020

m-ou-se mentioned this pull request Nov 18, 2020

miri no longer builds after rust-lang/rust#79167 #79176

Closed

m-ou-se mentioned this pull request Nov 18, 2020

Make weak syscalls in std work. rust-lang/miri#1629

Closed

RalfJung reviewed Nov 19, 2020

View reviewed changes

cuviper mentioned this pull request Nov 19, 2020

unix/weak: pass arguments to syscall at the given type #79196

Merged

eddyb mentioned this pull request Nov 20, 2020

Integrate measureme's hardware performance counter support. #78781

Merged

jyn514 mentioned this pull request Nov 24, 2020

Revert "linux: try to use libc getrandom to allow interposition" #79389

Closed

josephlr mentioned this pull request Apr 29, 2021

The getrandom crate m-ou-se/getrandom#1

Open

cuviper deleted the weak-getrandom branch September 21, 2021 16:44

RalfJung mentioned this pull request Apr 3, 2022

Don't cast thread name to an integer for prctl #95626

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

linux: try to use libc getrandom to allow interposition #78785

linux: try to use libc getrandom to allow interposition #78785

cuviper commented Nov 5, 2020

rust-highfive commented Nov 5, 2020

cuviper commented Nov 5, 2020

m-ou-se commented Nov 5, 2020 •

edited

Loading

cuviper commented Nov 5, 2020

m-ou-se commented Nov 5, 2020

cuviper commented Nov 5, 2020

eddyb commented Nov 6, 2020

cuviper commented Nov 6, 2020

the8472 Nov 6, 2020

cuviper Nov 6, 2020

bors commented Nov 14, 2020

cuviper commented Nov 16, 2020

m-ou-se commented Nov 17, 2020

bors commented Nov 17, 2020

RalfJung Nov 19, 2020 •

edited

Loading

RalfJung Nov 19, 2020 •

edited

Loading

m-ou-se Nov 19, 2020 •

edited

Loading

RalfJung Nov 19, 2020

RalfJung Nov 19, 2020

linux: try to use libc getrandom to allow interposition #78785

linux: try to use libc getrandom to allow interposition #78785

Conversation

cuviper commented Nov 5, 2020

rust-highfive commented Nov 5, 2020

cuviper commented Nov 5, 2020

m-ou-se commented Nov 5, 2020 • edited Loading

cuviper commented Nov 5, 2020

m-ou-se commented Nov 5, 2020

cuviper commented Nov 5, 2020

eddyb commented Nov 6, 2020

cuviper commented Nov 6, 2020

the8472 Nov 6, 2020

Choose a reason for hiding this comment

cuviper Nov 6, 2020

Choose a reason for hiding this comment

bors commented Nov 14, 2020

cuviper commented Nov 16, 2020

m-ou-se commented Nov 17, 2020

bors commented Nov 17, 2020

RalfJung Nov 19, 2020 • edited Loading

Choose a reason for hiding this comment

RalfJung Nov 19, 2020 • edited Loading

Choose a reason for hiding this comment

m-ou-se Nov 19, 2020 • edited Loading

Choose a reason for hiding this comment

RalfJung Nov 19, 2020

Choose a reason for hiding this comment

RalfJung Nov 19, 2020

Choose a reason for hiding this comment

m-ou-se commented Nov 5, 2020 •

edited

Loading

RalfJung Nov 19, 2020 •

edited

Loading

RalfJung Nov 19, 2020 •

edited

Loading

m-ou-se Nov 19, 2020 •

edited

Loading