Re: [PATCH 1/2] samples: rust: Provide example using the new Rust MiscDevice abstraction

[Greg KH]

On Thu, Dec 05, 2024 at 08:41:01AM +0000, Lee Jones wrote:
> On Wed, 04 Dec 2024, Greg KH wrote:
> 
> > On Wed, Dec 04, 2024 at 05:46:24PM +0000, Lee Jones wrote:
> > > This sample driver demonstrates the following basic operations:
> > > 
> > > * Register a Misc Device
> > > * Create /dev/rust-misc-device
> > > * Open the aforementioned character device
> > > * Operate on the character device via a simple ioctl()
> > > * Close the character device
> > > 
> > > Signed-off-by: Lee Jones <lee@xxxxxxxxxx>
> > > ---
> > >  samples/rust/Kconfig             | 10 ++++
> > >  samples/rust/Makefile            |  1 +
> > >  samples/rust/rust_misc_device.rs | 84 ++++++++++++++++++++++++++++++++
> > >  3 files changed, 95 insertions(+)
> > >  create mode 100644 samples/rust/rust_misc_device.rs
> > > 
> > > diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> > > index b0f74a81c8f9..df384e679901 100644
> > > --- a/samples/rust/Kconfig
> > > +++ b/samples/rust/Kconfig
> > > @@ -20,6 +20,16 @@ config SAMPLE_RUST_MINIMAL
> > >  
> > >  	  If unsure, say N.
> > >  
> > > +config SAMPLE_RUST_MISC_DEVICE
> > > +	tristate "Misc device"
> > > +	help
> > > +	  This option builds the Rust misc device.
> > > +
> > > +	  To compile this as a module, choose M here:
> > > +	  the module will be called rust_misc_device.
> > > +
> > > +	  If unsure, say N.
> > > +
> > >  config SAMPLE_RUST_PRINT
> > >  	tristate "Printing macros"
> > >  	help
> > > diff --git a/samples/rust/Makefile b/samples/rust/Makefile
> > > index c1a5c1655395..ad4b97a98580 100644
> > > --- a/samples/rust/Makefile
> > > +++ b/samples/rust/Makefile
> > > @@ -2,6 +2,7 @@
> > >  ccflags-y += -I$(src)				# needed for trace events
> > >  
> > >  obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
> > > +obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
> > >  obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
> > >  
> > >  rust_print-y := rust_print_main.o rust_print_events.o
> > > diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
> > > new file mode 100644
> > > index 000000000000..5f1b69569ef7
> > > --- /dev/null
> > > +++ b/samples/rust/rust_misc_device.rs
> > > @@ -0,0 +1,84 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > 
> > Nit, you forgot a copyright line here :)
> 
> I can add one, but none of the other drivers in this directory has one.

I think the copyright owner of this file will appreciate that.  In fact,
I think it might be required by them :)

> > Anyway, other than the copyright, this looks good to me.
> > 
> > Although we should get the "validate the data" rust patch set in here
> > soon, so we don't have to go and fix up all users of the miscdev rust
> > api at once.  Maybe I'll dig that series up over the holiday break if
> > someone doesn't beat me to it.
> 
> What needs doing?  Do you have a link?

https://lore.kernel.org/r/20240925205244.873020-1-benno.lossin@xxxxxxxxx

But in thinking about it more, this isn't going to work well with misc
devices as the data is coming from userspace, which already goes through
the user slice code.  Unless userslice should be marking the data as
untrusted?  I think that needs to happen as well.

thanks,

greg k-h