Skip to content

Pass pointers, not slices, to libc::ioctl #2181

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

maurer
Copy link
Contributor

@maurer maurer commented Nov 7, 2023

Slices are not FFI-safe.
&[u8] and &mut [u8] are wide pointers, which means that at the moment, we're getting lucky because they're passed via the "ScalarPair" ABI, and this means that passing a &[u8] results in passing two arguments, *const u8 and usize for pointer and length. This passes an extra argument to ioctl, which happens to work because the extra vararg is skipped. We're getting lucky right now, and we should explicitly pass the pointer we meant to pass instead.

This is normally checked on extern "C" functions, but ioctl in particular uses varargs which prevents the compiler check from reporting.

What does this PR do

Passes the buffer pointer to libc::ioctl rather than trying to pass slices across FFI.

Checklist:

  • I have read CONTRIBUTING.md
  • I have written necessary tests and rustdoc comments (Comment updated, no new tests)
  • A change log has been added if this PR modifies nix's API (N/A)

Slices are not FFI-safe.
&[u8] and &mut [u8] are *wide pointers*, which means that at the moment,
we're getting lucky because they're passed via the "ScalarPair" ABI, and
this means that passing a `&[u8]` results in passing two arguments,
`*const u8` and `usize` for pointer and length. This passes an extra
argument to ioctl, which happens to work because the extra vararg is
skipped. We're getting lucky right now, and we should explicitly pass
the pointer we meant to pass instead.

This is normally checked on `extern "C"` functions, but `ioctl` in
particular uses varargs which prevents the compiler check from
reporting.
@SteveLauC
Copy link
Member

Good catch! Thanks, and congratulations on your first contribution to nix!

@SteveLauC SteveLauC added this pull request to the merge queue Nov 8, 2023
Merged via the queue into nix-rust:master with commit 0ff4621 Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants