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: <fc5cee69-e37a-4053-a425-f9191e58011a@app.fastmail.com>
Date: Wed, 02 Oct 2024 15:31:59 +0000
From: "Arnd Bergmann" <arnd@...db.de>
To: "Alice Ryhl" <aliceryhl@...gle.com>
Cc: "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
 "Miguel Ojeda" <ojeda@...nel.org>,
 "Alexander Viro" <viro@...iv.linux.org.uk>,
 "Christian Brauner" <brauner@...nel.org>, "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 2, 2024, at 14:23, Alice Ryhl wrote:
> On Wed, Oct 2, 2024 at 3:59 PM Arnd Bergmann <arnd@...db.de> wrote:
>>
>> On Wed, Oct 2, 2024, at 13:31, Alice Ryhl wrote:
>> > On Wed, Oct 2, 2024 at 3:25 PM Arnd Bergmann <arnd@...db.de> wrote:
>> >>
>> You can also see the effects of the compat handlers there,
>> e.g. VIDIOC_QUERYBUF has three possible sizes associated
>> with it, depending on sizeof(long) and sizeof(time_t).
>>
>> There is a small optimization for buffers up to 128 bytes
>> to avoid the dynamic allocation, and this is likely a good
>> idea elsewhere as well.
>
> Oh, my. That seems like a rather sophisticated ioctl handler.
>
> Do we want all new ioctl handlers to work along those lines?

It was intentionally an example to demonstrate the worst
case one might hit, and I would hope that most drivers end
up not having to worry about them. 

To clarify: the file I mentioned is itself a piece of
infrastructure that is used to make the actual drivers
simpler, in this case by having drivers just fill out
a 'struct v4l2_ioctl_ops' with the command specific callbacks.

This works because video_ioctl2() has a clearly defined set
of commands that is shared by a large number of drivers.
For a generic bit of infrastructure, we obviously wouldn't
do anything that knows about specific commands, but the way
the get_user/put_user part works based on the size can be
quite similar.

There is similar piece of infrastructure in
drivers/gpu/drm/drm_ioctl.c, which is a bit simpler.

>> It seems like it should be possible to validate the size of
>> the argument against _IOC_SIZE(cmd) at compile time, but this
>> is not currently done, right?
>
> No, right now that validation happens at runtime. The ioctl handler
> tries to use the UserSliceReader to read a struct, which fails if the
> struct is too large.

Ok.

> I wonder if we could go for something more comprehensive than the
> super simple thing I just put together. I'm sure we can validate more
> things at compile time.



     Arnd