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

[Alexandre Courbot]

On Sun Sep 13, 2026 at 11:17 AM JST, Sagar Taunk wrote:
> As pointed out by Sashiko, `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.
>
> `nla_put()`'s `skb_tailroom()` check treats the length as signed, so
> the wrapped negative value slips past it. Further down, `__nla_reserve()`
> and `skb_put` reinterpret the same value as unsigned, turning it into an
> enormous length and triggering a kernel panic via `skb_over_panic()`.
>
> So, validate the cast with `c_int::try_from()` instead, and return
> `EMSGSIZE` when the length doesn't fit.
>
> Signed-off-by: Sagar Taunk <sagartaunk@xxxxxxxxx>
> ---
>  rust/kernel/net/netlink.rs | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/rust/kernel/net/netlink.rs b/rust/kernel/net/netlink.rs
> index 3c2b142a7402..f929f63b32c1 100644
> --- a/rust/kernel/net/netlink.rs
> +++ b/rust/kernel/net/netlink.rs
> @@ -88,11 +88,19 @@ fn put<T>(&mut self, attrtype: c_int, value: &T) -> Result
>          T: ?Sized + IntoBytes + Immutable,
>      {
>          let skb = self.skb.skb.as_ptr();
> -        let len = size_of_val(value);
> -        let ptr = core::ptr::from_ref(value).cast::<c_void>();
> -        // 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) })
> +        let bytes = value.as_bytes();
> +        // `nla_put()` takes attrlen as a plain `c_int`. If `bytes.len()`
> +        // doesn't fit, an `as` cast would wrap around a negative value.
> +        // Which then, would feed a huge unsigned length to `__nla_reserve()`
> +        // and `skb_put()` causing it to panic via `skb_over_panic()`. So,
> +        // the following check will reject it instead.
> +        let len = c_int::try_from(bytes.len()).map_err(|_| EMSGSIZE)?;

I understand that you wanted to reuse `bytes` here, but the original use
of `size_of_val` is clearer and keeping it would also reduce the diff,
so let's call `c_int::try_from` on that.

Also the comment is unneeded. The change of type is justified by the
fact that `nla_put` requires a `c_int`. Any further explanation adds
confusion, and the mentioned `__nla_reserve` doesn't even appear in the
chunk. This reads like what an LLM would produce, i.e. tediously.

> +        let ptr = bytes.as_ptr().cast::<c_void>();

Same here, the original line was fine so no need to change it. So it
looks like `bytes` is not needed at all.

As a general rule, and to make reviewing easier, it is a good idea to
keep the diffs are small as possible. Here the commit message says this
patch validates a value, so it should limit itself to that.