Explicitly specify types to arguments of 'libc::syscall' #74
Conversation
src/linux_android.rs
Outdated
| libc::syscall(libc::SYS_getrandom, | ||
| buf.as_mut_ptr() as *mut libc::c_void, | ||
| buf.len() as libc::size_t, | ||
| 0 as libc::c_uint) as libc::ssize_t |
There was a problem hiding this comment.
Ah, you looked up the proper types, nice. :)
I am going to close my PR then.
I think it would be a good idea though to add a local helper method that takes arguments of the right types and just forwards them to the syscall. Then the types do not have to be duplicated.
|
Thank you for the fix! I would also prefer to have a helper function (something like |
|
It turns out that |
|
Note the CI failures. |
|
Oh, well. I'll switch back to the raw syscall. |
The 'libc::syscall' function uses varargs - as a result, its arguments are completely untyped. THe user must ensure that it is called with the proper types for the targeted syscall - otherwise, the calling convention might cause arguments to be put into the wrong registers. This commit explicitly casts the arguments to 'libc::syscall' to the proper type for the 'getrandom' syscall. This ensures that the correct types for the target platform will always be used, instead of relying on the types used happening to match those required by the target platform.
Aaron1011
force-pushed
the
fix/sys-getrandom
branch
from
dccfc46
to
8599283
Compare
|
Maybe it's worth to write the helper like this? fn getrandom_syscall(buf: &mut [u8], block: bool) -> libc::ssize_t {
let flags = if block { 0 } else { libc::GRND_NONBLOCK };
unsafe {
libc::syscall(
libc::SYS_getrandom,
buf.as_mut_ptr() as *mut libc::c_void,
buf.len() as libc::size_t,
flags as libc::c_uint,
) as libc::ssize_t
}
}
// and call it in `is_getrandom_available` as
getrandom_syscall(&mut [], false)It may remove the need for rust-lang/miri#884. |
|
@newpavlov: That code is valid, though - other crates might try to do the same thing. As long as the Linux kernel accepts it, so should Miri. |
|
Agreed, that PR will land in Miri pending just a nit. |
|
I have fixed formatting and replaced |
|
@newpavlov thanks! What is your release schedule? If rust-lang/rust#63261 lands before a new release happens, we might see failures in https://github.com/RalfJung/miri-test-libstd. |
|
I will do release in an hour or two. |
|
I think this PR is the cause of this failure? |
|
No, this PR just turned that failure from an ICE to an "invalid NULL ptr usage". getrandom 0.1.7 was broken and caused Miri to ICE. getrandom 0.1.8 is fixed, but Miri had a bug making it not work. (So, there used to be two bugs, this PR here just fixes one of them.) That remaining bug (and the failure you are seeing) is fixed by rust-lang/miri#884, which is being landed in the Miri rustup component by rust-lang/rust#63320. |
test-cargo-miri: cargo update With both rust-random/getrandom#74 and #884 having landed, this should work now.
) The 'libc::syscall' function uses varargs - as a result, its arguments are completely untyped. THe user must ensure that it is called with the proper types for the targeted syscall - otherwise, the calling convention might cause arguments to be put into the wrong registers. This commit explicitly casts the arguments to 'libc::syscall' to the proper type for the 'getrandom' syscall. This ensures that the correct types for the target platform will always be used, instead of relying on the types used happening to match those required by the target platform. This particular commit is a backport of 6716ad0, with the addition of `sys_fill_exact` from master (originally committed in 65660e0) to make the backport more obviously correct. # Conflicts: # src/linux_android.rs
) The 'libc::syscall' function uses varargs - as a result, its arguments are completely untyped. THe user must ensure that it is called with the proper types for the targeted syscall - otherwise, the calling convention might cause arguments to be put into the wrong registers. This commit explicitly casts the arguments to 'libc::syscall' to the proper type for the 'getrandom' syscall. This ensures that the correct types for the target platform will always be used, instead of relying on the types used happening to match those required by the target platform. This particular commit is a backport of 6716ad0, with the addition of `sys_fill_exact` from master (originally committed in 65660e0) to make the backport more obviously correct.
The 'libc::syscall' function uses varargs - as a result, its arguments
are completely untyped. THe user must ensure that it is called with the
proper types for the targeted syscall - otherwise, the calling
convention might cause arguments to be put into the wrong registers.
This commit explicitly casts the arguments to 'libc::syscall' to the
proper type for the 'getrandom' syscall. This ensures that the correct
types for the target platform will always be used, instead of relying on
the types used happening to match those required by the target platform.