rust/kernel/net/netlink.rs | 5 +++++ 1 file changed, 5 insertions(+)
`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
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.
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
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 >
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
© 2016 - 2026 Red Hat, Inc.