linux-kernel - Re: [PATCH v4 2/4] samples: rust: Provide example using the new Rust 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: <2073da49-37c6-4add-9450-47842d05cd79@app.fastmail.com>
Date: Fri, 06 Dec 2024 11:29:00 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: "Alice Ryhl" <aliceryhl@...gle.com>
Cc: "Lee Jones" <lee@...nel.org>, linux-kernel@...r.kernel.org,
 "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
 "Miguel Ojeda" <ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>,
 "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
Subject: Re: [PATCH v4 2/4] samples: rust: Provide example using the new Rust
 MiscDevice abstraction

On Fri, Dec 6, 2024, at 11:09, Alice Ryhl wrote:
> On Fri, Dec 6, 2024 at 11:05 AM Arnd Bergmann <arnd@...db.de> wrote:
>>
>> On Fri, Dec 6, 2024, at 10:05, Lee Jones wrote:
>> > This sample driver demonstrates the following basic operations:
>> >
>> > * Register a Misc Device
>> > * Create /dev/rust-misc-device
>> > * Provide open call-back for the aforementioned character device
>> > * Operate on the character device via a simple ioctl()
>> > * Provide close call-back for the character device
>> >
>> > Signed-off-by: Lee Jones <lee@...nel.org>
>>
>> Could you include a compat_ioctl() callback in the example?
>> I think it would be good to include it as a reminder for
>> authors of actual drivers that every driver implementing
>> ioctl should also implement compat_ioctl. In C drivers, this
>> can usually be done by pointing .compat_ioctl() to the
>> generic compat_ptr_ioctl() function, which assumes that 'arg'
>> is a pointer disguised as an 'unsigned long'.
>
> The current Rust logic for building the fops table will use
> compat_ptr_ioctl() automatically if you specify ioctl() but don't
> specify compat_ioctl(), so this already uses compat_ptr_ioctl(). But
> maybe that's not what we want?

Ok, got it. It's usually the right thing to do, but it's easy
to get wrong if there is at least one ioctl command that actually
needs an integer argument instead of a pointer.

Almost all command definitions are for either no argument or
a pointer argument, and compat_ptr_ioctl() works fine there, by
doing a conversion from a 32-bit pointer to a 64-bit pointer
by zero-extending the upper 33 (on s390) or 32 bits (everywhere
else). Integer values need to either a 32-bit sign-extension
or a 32-bit zero-extension depending on how the argument is
interpreted on 32-bit architectures.

I wonder if we should change the prototype of the ioctl
callback to always pass a __user pointer and just not allow
the few commands that pass an integer in rust drivers, and
worry about it only when it's absolutely needed.

     Arnd