| 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: <2024120453-unfunded-oversight-5161@gregkh>
Date: Wed, 4 Dec 2024 19:25:07 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Lee Jones <lee@...nel.org>
Cc: linux-kernel@...r.kernel.org, ojeda@...nel.org, alex.gaynor@...il.com,
boqun.feng@...il.com, gary@...yguo.net, bjorn3_gh@...tonmail.com,
benno.lossin@...ton.me, a.hindborg@...nel.org, aliceryhl@...gle.com,
tmgross@...ch.edu, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH 2/2] sample: rust_misc_device: Demonstrate additional
get/set value functionality
On Wed, Dec 04, 2024 at 05:46:25PM +0000, Lee Jones wrote:
> Expand the complexity of the sample driver by providing the ability to
> get and set an integer. The value is protected by a mutex.
>
> Here is a simple userspace program that fully exercises the sample
> driver's capabilities.
>
> int main() {
> int value, new_value;
> int fd, ret;
>
> // Open the device file
> printf("Opening /dev/rust-misc-device for reading and writing\n");
> fd = open("/dev/rust-misc-device", O_RDWR);
> if (fd < 0) {
> perror("open");
> return errno;
> }
>
> // Make call into driver to say "hello"
> printf("Calling Hello\n");
> ret = ioctl(fd, RUST_MISC_DEV_HELLO, NULL);
> if (ret < 0) {
> perror("ioctl: Failed to call into Hello");
> close(fd);
> return errno;
> }
>
> // Get initial value
> printf("Fetching initial value\n");
> ret = ioctl(fd, RUST_MISC_DEV_GET_VALUE, &value);
> if (ret < 0) {
> perror("ioctl: Failed to fetch the initial value");
> close(fd);
> return errno;
> }
>
> value++;
>
> // Set value to something different
> printf("Submitting new value (%d)\n", value);
> ret = ioctl(fd, RUST_MISC_DEV_SET_VALUE, &value);
> if (ret < 0) {
> perror("ioctl: Failed to submit new value");
> close(fd);
> return errno;
> }
>
> // Ensure new value was applied
> printf("Fetching new value\n");
> ret = ioctl(fd, RUST_MISC_DEV_GET_VALUE, &new_value);
> if (ret < 0) {
> perror("ioctl: Failed to fetch the new value");
> close(fd);
> return errno;
> }
>
> if (value != new_value) {
> printf("Failed: Committed and retrieved values are different (%d - %d)\n", value, new_value);
> close(fd);
> return -1;
> }
>
> // Call the unsuccessful ioctl
> printf("Attempting to call in to an non-existent IOCTL\n");
> ret = ioctl(fd, RUST_MISC_DEV_FAIL, NULL);
> if (ret < 0) {
> perror("ioctl: Succeeded to fail - this was expected");
> } else {
> printf("ioctl: Failed to fail\n");
> close(fd);
> return -1;
> }
>
> // Close the device file
> printf("Closing /dev/rust-misc-device\n");
> close(fd);
>
> printf("Success\n");
> return 0;
> }
>
> Signed-off-by: Lee Jones <lee@...nel.org>
> ---
> samples/rust/rust_misc_device.rs | 82 ++++++++++++++++++++++++--------
> 1 file changed, 62 insertions(+), 20 deletions(-)
>
> diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
> index 5f1b69569ef7..9c041497d881 100644
> --- a/samples/rust/rust_misc_device.rs
> +++ b/samples/rust/rust_misc_device.rs
> @@ -2,13 +2,20 @@
>
> //! Rust misc device sample.
>
> +use core::pin::Pin;
> +
> use kernel::{
> c_str,
> - ioctl::_IO,
> + ioctl::{_IO, _IOC_SIZE, _IOR, _IOW},
> miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
> + new_mutex,
> prelude::*,
> + sync::Mutex,
> + uaccess::{UserSlice, UserSliceReader, UserSliceWriter},
> };
>
> +const RUST_MISC_DEV_GET_VALUE: u32 = _IOR::<i32>('R' as u32, 7);
> +const RUST_MISC_DEV_SET_VALUE: u32 = _IOW::<i32>('R' as u32, 8);
Shouldn't this be 'W'?
> const RUST_MISC_DEV_HELLO: u32 = _IO('R' as u32, 9);
>
> module! {
> @@ -40,45 +47,80 @@ fn init(_module: &'static ThisModule) -> Result<Self> {
> }
> }
>
> -struct RustMiscDevice;
> +struct Inner {
> + value: i32,
> +}
>
> -impl RustMiscDevice {
> - fn new() -> Self {
> - Self
> - }
> +#[pin_data(PinnedDrop)]
> +struct RustMiscDevice {
> + #[pin]
> + inner: Mutex<Inner>,
> }
>
> #[vtable]
> impl MiscDevice for RustMiscDevice {
> - type Ptr = KBox<Self>;
> + type Ptr = Pin<KBox<Self>>;
>
> - fn open() -> Result<KBox<Self>> {
> + fn open() -> Result<Pin<KBox<Self>>> {
> pr_info!("Opening Rust Misc Device Sample\n");
>
> - Ok(KBox::new(RustMiscDevice::new(), GFP_KERNEL)?)
> + KBox::try_pin_init(
> + try_pin_init! {
> + RustMiscDevice { inner <- new_mutex!( Inner{ value: 0_i32 } )}
> + },
> + GFP_KERNEL,
> + )
> }
>
> - fn ioctl(
> - _device: <Self::Ptr as kernel::types::ForeignOwnable>::Borrowed<'_>,
> - cmd: u32,
> - _arg: usize,
> - ) -> Result<isize> {
> + fn ioctl(device: Pin<&RustMiscDevice>, cmd: u32, arg: usize) -> Result<isize> {
> pr_info!("IOCTLing Rust Misc Device Sample\n");
>
> - match cmd {
> - RUST_MISC_DEV_HELLO => pr_info!("Hello from the Rust Misc Device\n"),
> + let size = _IOC_SIZE(cmd);
> +
> + let _ = match cmd {
> + RUST_MISC_DEV_GET_VALUE => device.get_value(UserSlice::new(arg, size).writer())?,
> + RUST_MISC_DEV_SET_VALUE => device.set_value(UserSlice::new(arg, size).reader())?,
> + RUST_MISC_DEV_HELLO => device.hello()?,
> _ => {
> - pr_err!("IOCTL not recognised: {}\n", cmd);
> + pr_err!("-> IOCTL not recognised: {}\n", cmd);
> return Err(EINVAL);
Nit, wrong return value for an invalid ioctl, I missed that in patch 1,
sorry about that.
> }
> - }
> + };
>
> Ok(0)
> }
> }
>
> -impl Drop for RustMiscDevice {
> - fn drop(&mut self) {
> +#[pinned_drop]
> +impl PinnedDrop for RustMiscDevice {
> + fn drop(self: Pin<&mut Self>) {
> pr_info!("Exiting the Rust Misc Device Sample\n");
> }
> }
> +
> +impl RustMiscDevice {
> + fn set_value(&self, mut reader: UserSliceReader) -> Result<isize> {
> + let new_value = reader.read::<i32>()?;
> + let mut guard = self.inner.lock();
> +
> + pr_info!("-> Copying data from userspace (value: {})\n", new_value);
> +
> + guard.value = new_value;
> + Ok(0)
> + }
> +
> + fn get_value(&self, mut writer: UserSliceWriter) -> Result<isize> {
> + let guard = self.inner.lock();
> +
> + pr_info!("-> Copying data to userspace (value: {})\n", &guard.value);
> +
> + writer.write::<i32>(&guard.value)?;
What happens if it fails, shouldn't your pr_info() happen after this?
thanks,
greg k-h
> + Ok(0)
> + }
> +
> + fn hello(&self) -> Result<isize> {
> + pr_info!("-> Hello from the Rust Misc Device\n");
> +
> + Ok(0)
> + }
> +}
> --
> 2.47.0.338.g60cca15819-goog
>
Powered by blists - more mailing lists