[PATCH v2] rust: net: netlink: validate attribute length before casting to `c_int`

[PATCH v2] rust: net: netlink: validate attribute length before casting to `c_int`

[Sagar Taunk]

`put()` trusted an unchecked `as` cast from `usize` to `c_int`.
When the length exceeds `i32::MAX` that cast wraps around to a
negative value.

This ultimately resulted in a kernel panic when the reinterpreted
value via `__nla_reserve()` and `skb_put()` became enormous.

Validate payload and header both fit together in a `u16`, rejecting
any payload that wouldn't leave room for `NLA_HDRLEN`.

Signed-off-by: Sagar Taunk <sagartaunk@proton.me>
---
Changes Since V1: 
Tried to make the diff smaller as pointed out by Alexandre Courbot.
Moreover, as pointed out by Sashiko,`nlattr` is stored in a 16-bit 
field which houses both the header and the payload, so make the check 
verify that there is enough space left for the header to fit with the 
payload. 

 rust/kernel/net/netlink.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/rust/kernel/net/netlink.rs b/rust/kernel/net/netlink.rs
index a2f4bd171dcf..667667346f96 100644
--- a/rust/kernel/net/netlink.rs
+++ b/rust/kernel/net/netlink.rs
@@ -90,9 +90,14 @@ fn put<T>(&mut self, attrtype: c_int, value: &T) -> Result
     where
         T: ?Sized + IntoBytes + Immutable,
     {
+        let max_payload_len = u16::MAX as usize - size_of::<bindings::nlattr>();
+
         let skb = self.skb.skb.as_ptr();
         let len = size_of_val(value);
         let ptr = core::ptr::from_ref(value).cast::<c_void>();
+        if len > max_payload_len {
+            return Err(EMSGSIZE);
+        }
         // SAFETY: `skb` is valid by `NetlinkSkBuff` type invariants, and the provided value is
         // readable and initialized for its `size_of` bytes.
         to_result(unsafe { bindings::nla_put(skb, attrtype, len as c_int, ptr) })
-- 
2.55.0

Re: [PATCH v2] rust: net: netlink: validate attribute length before casting to `c_int`

[Alice Ryhl]

On Tue, Sep 15, 2026 at 04:40:10PM +0000, Sagar Taunk wrote:
> `put()` trusted an unchecked `as` cast from `usize` to `c_int`.
> When the length exceeds `i32::MAX` that cast wraps around to a
> negative value.
> 
> This ultimately resulted in a kernel panic when the reinterpreted
> value via `__nla_reserve()` and `skb_put()` became enormous.
> 
> Validate payload and header both fit together in a `u16`, rejecting
> any payload that wouldn't leave room for `NLA_HDRLEN`.
> 
> Signed-off-by: Sagar Taunk <sagartaunk@proton.me>

This should include a Fixes: tag if it leads to a kernel panic.

Can you also update the subject to [PATCH net vX] according to the
guidelines in: Documentation/process/maintainer-netdev.rst

The patch itself LGTM.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Alice

Re: [PATCH v2] rust: net: netlink: validate attribute length before casting to `c_int`

[Sagar Taunk]

Thanks for the review. Also,I wanted to ask what do I put in the `Fixes` tag? 
Like the commit which introduces it or explain the problem there?

Thanks, 
Sagar Taunk


On Thursday, September 17th, 2026 at 3:05 PM, Alice Ryhl <aliceryhl@google.com> wrote:

> On Tue, Sep 15, 2026 at 04:40:10PM +0000, Sagar Taunk wrote:
> > `put()` trusted an unchecked `as` cast from `usize` to `c_int`.
> > When the length exceeds `i32::MAX` that cast wraps around to a
> > negative value.
> >
> > This ultimately resulted in a kernel panic when the reinterpreted
> > value via `__nla_reserve()` and `skb_put()` became enormous.
> >
> > Validate payload and header both fit together in a `u16`, rejecting
> > any payload that wouldn't leave room for `NLA_HDRLEN`.
> >
> > Signed-off-by: Sagar Taunk <sagartaunk@proton.me>
> 
> This should include a Fixes: tag if it leads to a kernel panic.
> 
> Can you also update the subject to [PATCH net vX] according to the
> guidelines in: Documentation/process/maintainer-netdev.rst
> 
> The patch itself LGTM.
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> 
> Alice
>

Re: [PATCH v2] rust: net: netlink: validate attribute length before casting to `c_int`

[Alice Ryhl]

On Thu, Sep 17, 2026 at 2:45 PM Sagar Taunk <sagartaunk@proton.me> wrote:
>
> Thanks for the review. Also,I wanted to ask what do I put in the `Fixes` tag?
> Like the commit which introduces it or explain the problem there?

You need to put the commit you are fixing. Please see
Documentation/process/submitting-patches.rst file for details on how
to write a Fixes: tag correctly.

Alice

Re: [PATCH v2] rust: net: netlink: validate attribute length before casting to `c_int`

[Alexandre Courbot]

On Tue Sep 15, 2026 at 5:40 PM BST, Sagar Taunk wrote:
> `put()` trusted an unchecked `as` cast from `usize` to `c_int`.
> When the length exceeds `i32::MAX` that cast wraps around to a
> negative value.
>
> This ultimately resulted in a kernel panic when the reinterpreted
> value via `__nla_reserve()` and `skb_put()` became enormous.
>
> Validate payload and header both fit together in a `u16`, rejecting
> any payload that wouldn't leave room for `NLA_HDRLEN`.
>
> Signed-off-by: Sagar Taunk <sagartaunk@proton.me>
> ---
> Changes Since V1: 
> Tried to make the diff smaller as pointed out by Alexandre Courbot.
> Moreover, as pointed out by Sashiko,`nlattr` is stored in a 16-bit 
> field which houses both the header and the payload, so make the check 
> verify that there is enough space left for the header to fit with the 
> payload. 
>
>  rust/kernel/net/netlink.rs | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/rust/kernel/net/netlink.rs b/rust/kernel/net/netlink.rs
> index a2f4bd171dcf..667667346f96 100644
> --- a/rust/kernel/net/netlink.rs
> +++ b/rust/kernel/net/netlink.rs
> @@ -90,9 +90,14 @@ fn put<T>(&mut self, attrtype: c_int, value: &T) -> Result
>      where
>          T: ?Sized + IntoBytes + Immutable,
>      {
> +        let max_payload_len = u16::MAX as usize - size_of::<bindings::nlattr>();

This can be a const. Also we discourage the use of `as` which can
silently truncate data. Please use `u16_as_usize` from the `num::casts`
module for the conversion.

A reader of the code will also not have the commit message context and
might wonder why you are using `u16::MAX`, so I think a comment
justifying that choice (i.e. your point about `nlattr` is also
necessary).

> +
>          let skb = self.skb.skb.as_ptr();
>          let len = size_of_val(value);
>          let ptr = core::ptr::from_ref(value).cast::<c_void>();
> +        if len > max_payload_len {
> +            return Err(EMSGSIZE);
> +        }

This looks like a better fix indeed.