linux-kernel - [PATCH v3 0/1] rust: device: rename "Device::from

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-next>] [day] [month] [year] [list]

Message-ID: <20240930144328.51098-1-trintaeoitogc@gmail.com>
Date: Mon, 30 Sep 2024 11:43:26 -0300
From: Guilherme Giacomo Simoes <trintaeoitogc@...il.com>
To: gregkh@...uxfoundation.org,
	rafael@...nel.org,
	ojeda@...nel.org,
	alex.gaynor@...il.com,
	boqun.feng@...il.com,
	gary@...yguo.net,
	bjorn3_gh@...tonmail.com,
	benno.lossin@...ton.me,
	aliceryhl@...gle.com,
	mcgrof@...nel.org,
	russ.weight@...ux.dev,
	dakr@...hat.com,
	a.hindborg@...nel.org
Cc: Guilherme Giacomo Simoes <trintaeoitogc@...il.com>,
	rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH v3 0/1] rust: device: rename "Device::from_raw()" 

Why did I make this change?
This function "Device::from_raw()" increments the refcount by this
command "bindings::get_device(prt)". This can be confused because the
function Arc::from_raw() from the standard library, doesn't increment
the refcount.

This discussion is in
https://rust-for-linux.zulipchat.com/#narrow/stream/291566-Library/topic/Inconsistency.20of.20.60from_raw.60.2E

The options can be:
1) Rename the function for don't make confusing with the
Arc::from_raw().
2) Remove this function and use the unsafe { Device::as_ref(ptr)
}.into() when I need the get pointer for the device.

Proposed solution
I like the first option. Because, how was will commented by Boqun Feng ,
when the people write the "unsafe { Device::as_ref(ptr) }.into()" again,
again and again... inevitably anybody will create a help function for
this.

Then I think that we should rename this function for
Device::get_from_raw() or maybe Device::get_device() and I like more of
the second option because, this will be equal the get_device() function
that already exists in .c code.


How do I test this:
I create this simple file in sample/rust/device.rs
""""""""
use kernel::device::Device;
use kernel::prelude::*;
use kernel::types::ARef;

module! {
    type: DeviceTest,
    name: "device_test",
    author: "Test device",
    description: "A simple module for test device",
    license: "GPL",
}

struct DeviceTest;

impl kernel::Module for DeviceTest {
    fn init(_module: &'static ThisModule) -> Result<Self> {
        pr_info!("initial device test");
        let device = create_and_get_device();
        pr_info!("device created");

        Ok(DeviceTest)
    }
}

impl Drop for DeviceTest {
    fn drop(&mut self) {
        pr_info!("bye bye driver test");
    }
}

fn create_and_get_device() -> ARef<Device> {
    let device = unsafe { Device::get_device(core::ptr::null_mut()) };
    device
}
""""""""

I set this in Kconfig
diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index b0f74a81c8f9..7779969e7dd6 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -37,4 +37,9 @@ config SAMPLE_RUST_HOSTPROGS

          If unsure, say N.

+config SAMPLE_DEVICE_TEST
+       tristate "Device test"
+       help
+               This option is for device test
+
 endif # SAMPLES_RUST


and in Makefile
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 03086dabbea4..85a8b30100e7 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -2,5 +2,6 @@

 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)              += rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)                        += rust_print.o
+obj-$(CONFIG_SAMPLE_DEVICE_TEST)                       += device.o

 subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS)         += hostprogs


Then I enable this in menu config... compile the kernel e run this in a
qemu:
qemu-system-x86_64 -kernel bzImage -initrd initramfs.img -m 2G -machine
q35 -device ich9-ahci,id=sata -drive
id=disk,file=rootfs.img,if=none,format=raw -device
ide-hd,drive=disk,bus=sata.0 -append "root=/dev/sda console=ttyS0"
-nographic -monitor telnet:127.0.0.1:5555,server,nowai

the expected print is showing
[    2.786174] device_test: initial device test
[    2.786541] device_test: device created


Guilherme Giacomo Simoes (1):
  rust: device: rename "Device::from_raw()"

 rust/kernel/device.rs   | 2 +-
 rust/kernel/firmware.rs | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 

Changes from v2:
 - Refactored commit message style