linux-kernel - Re: [PATCH v2 2/2] rust: miscdevice: add base miscdevice abstraction

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2024100223-unwitting-girdle-92a5@gregkh>
Date: Wed, 2 Oct 2024 18:04:38 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Arnd Bergmann <arnd@...db.de>
Cc: Christian Brauner <brauner@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Miguel Ojeda <ojeda@...nel.org>,
	Alexander Viro <viro@...iv.linux.org.uk>, Jan Kara <jack@...e.cz>,
	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>, rust-for-linux@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] rust: miscdevice: add base miscdevice abstraction

On Wed, Oct 02, 2024 at 03:45:08PM +0000, Arnd Bergmann wrote:
> On Wed, Oct 2, 2024, at 14:23, Christian Brauner wrote:
> 
> > and then copy the stuff via copy_struct_from_user() or copy back out to
> > user via other means.
> >
> > This way you can safely extend ioctl()s in a backward and forward
> > compatible manner and if we can enforce this for new drivers then I
> > think that's what we should do.
> 
> I don't see much value in building generic code for ioctl around
> this specific variant of extensibility. Extending ioctl commands
> by having a larger structure that results in a new cmd code
> constant is fine, but there is little difference between doing
> this with the same or a different 'nr' value. Most drivers just
> always use a new nr here, and I see no reason to discourage that.
> 
> There is actually a small risk in your example where it can
> break if you have the same size between native and compat
> variants of the same command, like
> 
> struct old {
>     long a;
> };
> 
> struct new {
>     long a;
>     int b;
> };
> 
> Here, the 64-bit 'old' has the same size as the 32-bit 'new',
> so if we try to handle them in a shared native/compat ioctl
> function, this needs an extra in_conmpat_syscall() check that
> adds complexity and is easy to forget.

Agreed, "extending" ioctls is considered a bad thing and it's just
easier to create a new one.  Or use some flags and reserved fields, if
you remember to add them in the beginning...

Anyway, this is all great, but for now, I'll take this series in my tree
and we can add onto it from there.  I'll dig up some sample code that
uses this too, so that we make sure it works properly.  Give me a few
days to catch up before it lands in my trees...

thanks,

greg k-h