Re: [PATCH] rust: auxiliary: validate DeviceId name length

["Alexandre Courbot" <acourbot@nvidia.com>]

On Wed Sep 9, 2026 at 4:19 PM JST, Greg Kroah-Hartman wrote:
> On Tue, Sep 08, 2026 at 11:32:46PM -0400, Georgios Androutsopoulos wrote:
>> `DeviceId::new()` copies `modname` and `name` into the fixed 40-byte
>> `auxiliary_device_id::name` array without checking that they fit. An
>> oversized name is caught by the array bounds check, but the error
>> reports an out-of-bounds index in the copy loop rather than the
>> constraint the caller violated.
>> 
>> Check the invariant explicitly instead, so the failure states the length
>> limit rather than an array index.
>> 
>> In a constant context exceeding the limit leads to a build error; at
>> runtime it panics, so add a `# Panics` section for it.
>> 
>> Fixes: ce735e73dd59 ("rust: auxiliary: add auxiliary device / driver abstractions")
>> Signed-off-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>
>> ---
>>  rust/kernel/auxiliary.rs | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>> 
>> diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
>> index 60dfbec8f330..1f3ba86d6d96 100644
>> --- a/rust/kernel/auxiliary.rs
>> +++ b/rust/kernel/auxiliary.rs
>> @@ -137,10 +137,20 @@ macro_rules! module_auxiliary_driver {
>>  
>>  impl DeviceId {
>>      /// Create a new [`DeviceId`] from name.
>> +    ///
>> +    /// # Panics
>> +    ///
>> +    /// Panics if the combined module and device name, including the
>> +    /// separator and trailing NUL, exceeds `AUXILIARY_NAME_SIZE` bytes.
>>      pub const fn new(modname: &'static CStr, name: &'static CStr) -> Self {
>>          let name = name.to_bytes_with_nul();
>>          let modname = modname.to_bytes_with_nul();
>>  
>> +        assert!(
>> +            modname.len().saturating_add(name.len()) <= bindings::AUXILIARY_NAME_SIZE as usize,
>> +            "auxiliary device ID is too long"
>> +        );
>
> We really shouldn't panic, we should error out and fail the creation
> instead.
>
> But what is placing the constraint of the name size here?  The C api
> just takes a pointer, it doesn't care about the size, why does the rust
> binding care?

Note that this is not a runtime panic, this method is only ever called
in const context by the `auxiliary_device_table!` macro, so a panic here
translates to a build error. The current code also panicks if the name
is larger than the target array, but it did so when the array was
accessed out-of-bounds, with a more obscure error message. What this
patch does is provide a better error message for a condition that was
already checked.

So while the patch is arguably an improvement, I would suggest to drop
its `Fixes:` tag as it doesn't really fixes a condition that wasn't
already checked. Also the commit message's "at runtime it panics" gives
the wrong idea of when this code is evaluated.