| 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
| ||
|
Message-Id: <CB975A56-A8D5-4615-8755-04D7B0BBBCA5@collabora.com>
Date: Tue, 25 Feb 2025 06:37:33 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: Danilo Krummrich <dakr@...nel.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>,
Alice Ryhl <aliceryhl@...gle.com>,
Trevor Gross <tmgross@...ch.edu>,
Joel Becker <jlbec@...lplan.org>,
Christoph Hellwig <hch@....de>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Will Deacon <will@...nel.org>,
Waiman Long <longman@...hat.com>,
Fiona Behrens <me@...enk.dev>,
Charalampos Mitrodimas <charmitro@...teo.net>,
rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 3/4] rust: configfs: add a sample demonstrating
configfs usage
Hi Andreas,
> On 24 Feb 2025, at 10:21, Andreas Hindborg <a.hindborg@...nel.org> wrote:
>
> Add a sample to the samples folder, demonstrating the intended use of the
> rust configfs API.
nit: this is not the first time I see Rust not capitalized in this series.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@...nel.org>
> ---
> samples/rust/Kconfig | 11 +++
> samples/rust/Makefile | 1 +
> samples/rust/rust_configfs.rs | 179 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 191 insertions(+)
>
> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> index 918dbead2c0b..2f97bf9a7b4c 100644
> --- a/samples/rust/Kconfig
> +++ b/samples/rust/Kconfig
> @@ -10,6 +10,17 @@ menuconfig SAMPLES_RUST
>
> if SAMPLES_RUST
>
> +config SAMPLE_RUST_CONFIGFS
> + tristate "Configfs sample"
> + depends on CONFIGFS_FS
> + help
> + This option builds the Rust configfs sample.
> +
> + To compile this as a module, choose M here:
> + the module will be called rust_configfs.
> +
> + If unsure, say N.
> +
> config SAMPLE_RUST_MINIMAL
> tristate "Minimal"
> help
> diff --git a/samples/rust/Makefile b/samples/rust/Makefile
> index 5a8ab0df0567..72122f010caf 100644
> --- a/samples/rust/Makefile
> +++ b/samples/rust/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE) += rust_misc_device.o
> obj-$(CONFIG_SAMPLE_RUST_PRINT) += rust_print.o
> obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI) += rust_driver_pci.o
> obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM) += rust_driver_platform.o
> +obj-$(CONFIG_SAMPLE_RUST_CONFIGFS) += rust_configfs.o
>
> rust_print-y := rust_print_main.o rust_print_events.o
>
> diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
> new file mode 100644
> index 000000000000..36a2c848a979
> --- /dev/null
> +++ b/samples/rust/rust_configfs.rs
> @@ -0,0 +1,179 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Rust configfs sample.
> +
> +use kernel::alloc::flags;
> +use kernel::c_str;
> +use kernel::configfs;
> +use kernel::configfs_attrs;
> +use kernel::new_mutex;
> +use kernel::page::PAGE_SIZE;
> +use kernel::prelude::*;
> +use kernel::sync::Mutex;
> +
> +module! {
> + type: RustConfigfs,
> + name: "rust_configfs",
> + author: "Rust for Linux Contributors",
> + description: "Rust configfs sample",
> + license: "GPL",
> +}
> +
> +#[pin_data]
> +struct RustConfigfs {
> + #[pin]
> + config: configfs::Subsystem<Configuration>,
> +}
> +
> +#[pin_data]
> +struct Configuration {
> + message: &'static CStr,
> + #[pin]
> + bar: Mutex<(KBox<[u8; PAGE_SIZE]>, usize)>,
> +}
> +
> +impl Configuration {
> + fn new() -> impl PinInit<Self, Error> {
> + try_pin_init!(Self {
> + message: c_str!("Hello World\n"),
> + bar <- new_mutex!((KBox::new([0; PAGE_SIZE], flags::GFP_KERNEL)?, 0)),
> + })
> + }
> +}
> +
> +impl kernel::InPlaceModule for RustConfigfs {
> + fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
> + pr_info!("Rust configfs sample (init)\n");
> +
> + let item_type = configfs_attrs! {
> + container: configfs::Subsystem<Configuration>,
> + data: Configuration,
> + child: Child,
> + attributes: [
> + message: 0,
> + bar: 1,
> + ],
> + };
As I said in the previous patch, this macro is a bit complex. Is there anything more you can do with it?
If so, it better be in this driver, because I hardly think anybody will go through the source code itself
to figure out. My 2c.
> +
> + try_pin_init!(Self {
> + config <- configfs::Subsystem::new(
> + c_str!("rust_configfs"), item_type, Configuration::new()
> + ),
> + })
> + }
> +}
> +
> +#[vtable]
> +impl configfs::GroupOperations for Configuration {
> + type Child = Child;
> +
> + fn make_group(&self, name: &CStr) -> Result<impl PinInit<configfs::Group<Child>, Error>> {
> + let tpe = configfs_attrs! {
> + container: configfs::Group<Child>,
> + data: Child,
> + child: GrandChild,
> + attributes: [
> + baz: 0,
> + ],
> + };
> +
> + Ok(configfs::Group::new(name.try_into()?, tpe, Child::new()))
> + }
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<0> for Configuration {
> + type Data = Configuration;
> +
> + fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> + pr_info!("Show message\n");
> + let data = container.message;
> + page[0..data.len()].copy_from_slice(data);
> + Ok(data.len())
> + }
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<1> for Configuration {
> + type Data = Configuration;
> +
> + fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> + pr_info!("Show bar\n");
> + let guard = container.bar.lock();
> + let data = guard.0.as_slice();
> + let len = guard.1;
> + page[0..len].copy_from_slice(&data[0..len]);
> + Ok(len)
> + }
> +
> + fn store(container: &Configuration, page: &[u8]) -> Result {
> + pr_info!("Store bar\n");
> + let mut guard = container.bar.lock();
> + guard.0[0..page.len()].copy_from_slice(page);
> + guard.1 = page.len();
> + Ok(())
> + }
> +}
> +
> +#[pin_data]
> +struct Child {}
nit: you don’t need the braces here
> +
> +impl Child {
> + fn new() -> impl PinInit<Self, Error> {
> + try_pin_init!(Self {})
> + }
> +}
> +
> +#[vtable]
> +impl configfs::GroupOperations for Child {
> + type Child = GrandChild;
> +
> + fn make_group(&self, name: &CStr) -> Result<impl PinInit<configfs::Group<GrandChild>, Error>> {
> + let tpe = configfs_attrs! {
> + container: configfs::Group<GrandChild>,
> + data: GrandChild,
> + attributes: [
> + gc: 0,
> + ],
> + };
> +
> + Ok(configfs::Group::new(
> + name.try_into()?,
> + tpe,
> + GrandChild::new(),
> + ))
> + }
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<0> for Child {
> + type Data = Child;
> +
> + fn show(_container: &Child, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> + pr_info!("Show baz\n");
> + let data = c"Hello Baz\n".to_bytes();
> + page[0..data.len()].copy_from_slice(data);
> + Ok(data.len())
> + }
> +}
> +
> +#[pin_data]
> +struct GrandChild {}
…nor here
> +
> +impl GrandChild {
> + fn new() -> impl PinInit<Self, Error> {
> + try_pin_init!(Self {})
> + }
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<0> for GrandChild {
> + type Data = GrandChild;
> +
> + fn show(_container: &GrandChild, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> + pr_info!("Show baz\n");
As I said in the cover letter, perhaps this one slip through :)
> + let data = c"Hello GC\n".to_bytes();
> + page[0..data.len()].copy_from_slice(data);
> + Ok(data.len())
> + }
> +}
>
> --
> 2.47.0
>
>
I’m OK with this patch. It works, and it does what it’s supposed to do, i.e.: showcase the API.
With the “Show baz” nit fixed:
Reviewed-by: Daniel Almeida <daniel.almeida@...labora.com>
Powered by blists - more mailing lists