Re: [PATCH v5 6/6] rust: use strict provenance APIs
From: Tamir Duberstein
Date: Wed Mar 19 2025 - 14:26:20 EST
On Wed, Mar 19, 2025 at 10:42 AM Benno Lossin <benno.lossin@xxxxxxxxx> wrote:
>
> On Wed Mar 19, 2025 at 3:14 PM CET, Tamir Duberstein wrote:
> > On Wed, Mar 19, 2025 at 8:21 AM Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
> >> On Wed, Mar 19, 2025 at 12:23:44AM +0000, Benno Lossin wrote:
> >> > On Tue Mar 18, 2025 at 1:29 PM CET, Alice Ryhl wrote:
> >> > > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
> >> > >> @@ -264,7 +266,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> {
> >> > >> let res = unsafe {
> >> > >> bindings::_copy_from_user(
> >> > >> out.as_mut_ptr().cast::<c_void>(),
> >> > >> - self.ptr as *const c_void,
> >> > >> + crate::with_exposed_provenance(self.ptr),
> >> > >> len,
> >> > >> )
> >> > >> };
> >> > >
> >> > > That's especially true for cases like this. These are userspace pointers
> >> > > that are never dereferenced. It's not useful to care about provenance
> >> > > here.
> >> >
> >> > I agree for this case, but I think we shouldn't be using raw pointers
> >> > for this to begin with. I'd think that a newtype wrapping `usize` is a
> >> > much better fit. It can then also back the `IoRaw` type. AFAIU user
> >> > space pointers don't have provenance, right? (if they do, then we should
> >> > use this API :)
> >>
> >> We're doing that to the fullest extent possible already. We only convert
> >> them to pointers when calling C FFI functions that take user pointers as
> >> a raw pointer.
> >>
> >> Alice
> >
> > Personally, I agree with Benno that `as` conversions are a misfeature
> > in the language.
> >
> > I think this patch and the ensuing discussion is making perfect the
> > enemy of good, so I'd prefer to drop it and revisit when the
> > ergonomics have improved.
>
> I don't think that we need to rush on the rest of the patch series.
> Boqun's suggestion is very good and I'm not sure which ergonomics need
> to be improved here.
The improved ergonomics arrive in Rust 1.79. See Boqun's reply that
explains we need to keep all the stubs until then.
Regarding landing the rest of the series - you said it yourself: "it's
only going to get more painful in the long run to change this". The
nature of lints is that the longer you don't enable them, the likelier
you are to have a higher hill to climb later.