Re: [RFC PATCH] rust: hwmon: add basic hwmon abstractions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Use-After-Free via `name` sysfs attribute.
- [Medium] Bypassing the mandatory `pin_init!` safety framework for pinned initialization.
- [Low] Missing `#[inline]` annotations on small abstraction helpers.
- [Low] Misspelled author name in module metadata.
--

commit c36781b9829b25d8c4099a3b3ad9cfa1c367592f
Author: Donjuanplatinum <donplat@barrensea.org>

rust: hwmon: add basic hwmon abstractions

Add a minimal Rust abstraction for the Hardware Monitoring subsystem.
This supports registering a single temperature sensor with the temp1_input
attribute.

> diff --git a/rust/kernel/hwmon.rs b/rust/kernel/hwmon.rs
> new file mode 100644
> index 0000000000000..5697a1a0d6c29
> --- /dev/null
> +++ b/rust/kernel/hwmon.rs

[ ... ]

> +impl TryFrom<u32> for SensorType {
> +    type Error = Error;
> +
> +    fn try_from(value: u32) -> Result<Self> {

This isn't a bug, but should this small abstraction helper be annotated with
#[inline] to match the Rust subsystem guidelines?

[ ... ]

> +impl TryFrom<u32> for TempAttr {
> +    type Error = Error;
> +
> +    fn try_from(value: u32) -> Result<Self> {

This isn't a bug, but should this helper also be annotated with #[inline]?

[ ... ]

> +impl<T: Driver> HwmonInner<T> {
> +    /// Returns a placeholder with all pointer fields set to null. The returned structure is
> +    /// safe to drop — no dynamic resources are held before registration completes.
> +    fn new_placeholder(driver: T) -> Self {

This isn't a bug, but could this abstraction function also be annotated
with #[inline]?

[ ... ]

> +impl<T: Driver> Registration<T> {
> +    /// Registers a new hwmon device.
> +    ///
> +    /// The device is registered as a child of parent. name must not contain characters
> +    /// rejected by hwmon_is_bad_char (hyphens, spaces, asterisks).
> +    pub fn new(parent: &Device, name: &CStr, data: T) -> Result<Self> {
> +        let mut boxed = KBox::new(HwmonInner::new_placeholder(data), GFP_KERNEL)?;

Is it possible to use the try_pin_init! framework here instead of manually
allocating a KBox and later sealing it with Pin::new_unchecked? 

The subsystem documentation states that try_pin_init! should be used to
initialize structs that require pinning.

[ ... ]

> +        // SAFETY: chip_info and all nested pointers target memory within the same
> +        // allocation, which remains valid until hwmon_device_unregister in Drop.
> +        // parent.as_raw() and name.as_char_ptr() are valid. The name is copied by
> +        // the kernel (via dev_set_name -> kvasprintf_const), so no lifetime issue.
> +        let hwmon_dev = from_err_ptr(unsafe {
> +            bindings::hwmon_device_register_with_info(
> +                parent.as_raw(),
> +                name.as_char_ptr(),

The safety comment states the name is copied by the kernel via dev_set_name,
but does the hwmon core use this pointer directly for the name sysfs
attribute rather than just the kobject directory name?

If hwmon_device_register_with_info() stores the raw pointer directly in
hwdev->name without copying it, and this function accepts a temporary &CStr
that can be dropped immediately after registration, will reading
/sys/class/hwmon/hwmonX/name from userspace dereference a freed pointer?

Could this API require a &'static CStr, or securely allocate and copy the
string to prevent a use-after-free?

> +                drvdata,
> +                &raw const (*inner_ptr).chip_info,
> +                core::ptr::null_mut(),
> +            )
> +        })?;

[ ... ]

> diff --git a/samples/rust/rust_driver_hwmon.rs b/samples/rust/rust_driver_hwmon.rs
> new file mode 100644
> index 0000000000000..3362de9246e09
> --- /dev/null
> +++ b/samples/rust/rust_driver_hwmon.rs

[ ... ]

> +module! {
> +    type: SampleModule,
> +    name: "rust_hwmon_driver",
> +    authors: ["DonjuanPlatium"],

This isn't a bug, but there is a typo in the author name (DonjuanPlatium
instead of DonjuanPlatinum).

> +    description: "Rust hwmon device sample",
> +    license: "GPL",
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512155747.10136-1-donplat@barrensea.org?part=1