[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aDbpsB3ayj6tFfbI@google.com>
Date: Wed, 28 May 2025 10:47:12 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Al Viro <viro@...iv.linux.org.uk>
Cc: Miguel Ojeda <ojeda@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Arnd Bergmann <arnd@...db.de>, Andrew Morton <akpm@...ux-foundation.org>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
"Björn Roy Baron" <bjorn3_gh@...tonmail.com>, Benno Lossin <benno.lossin@...ton.me>,
Andreas Hindborg <a.hindborg@...nel.org>, Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>, rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] uaccess: rust: use newtype for user pointers
On Tue, May 27, 2025 at 11:12:11PM +0100, Al Viro wrote:
> On Tue, May 27, 2025 at 01:53:12PM +0000, Alice Ryhl wrote:
> > In C code we use sparse with the __user annotation to detect cases where
> > a user pointer is mixed up with other things. To replicate that, we
> > introduce a new struct UserPtr that serves the same purpose using the
> > newtype pattern.
> >
> > The UserPtr type is not marked with #[derive(Debug)], which means that
> > it's not possible to print values of this type. This avoids ASLR
> > leakage.
> >
> > The type is added to the prelude as it is a fairly fundamental type
> > similar to c_int. The wrapping_add() method is renamed to
> > wrapping_byte_add() for consistency with the method name found on raw
> > pointers.
>
> That's considerably weaker than __user, though - with
> struct foo {struct bar x; struct baz y[2]; };
> struct foo __user *p;
> void f(struct bar __user *);
> sparse does figure out that f(&p->y[1]) is a type error - &p->y[1] is
> struct baz __user * and f() expects struct bar __user *.
>
> It's not just mixing userland pointers with other things - it's not mixing
> userland pointers to different types, etc.
>
> In practice I've seen quite a few brainos caught by that...
We don't currently have any way to perform that kind of pointer-math on
user pointers, nor do we have any users of it. I imagine that this type
checking is only useful if you can actually perform pointer math in the
first place?
Right now, actual reading/writing to userspace is done via the
UserSliceReader and UserSliceWriter types rather than directly on
UserPtr, and UserPtr is just the constructor for those other types.
There is already some means of type checking on UserSliceReader and
UserSliceWriter, so I'm inclined to say that this is good enough.
Of course, if we ever add a way to do pointer math on user pointers,
then they should have a pointee type and hence also this check.
Alice
Powered by blists - more mailing lists