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

[Christian Brauner <brauner@kernel.org>]

On Wed, Oct 02, 2024 at 03:45:08PM GMT, 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.

This presupposes that we will have Rust drivers - not C drivers - that
define structs like it's 1990. You yourself and me included try to
enforce that structs are correctly aligned and padded. So I see this as
a non-argument. We wouldn't let this slide in new system calls so I
don't see why we would in new ioctls.