Re: [PATCH] rust: net: netlink: validate attribute length before casting to `c_int`
From: "Alexandre Courbot" <acourbot@nvidia.com>
Date: 2026-09-14 01:10:51
Also in:
lkml, rust-for-linux
On Sun Sep 13, 2026 at 11:17 AM JST, Sagar Taunk wrote:
quoted hunk ↗ jump to hunk
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 <redacted> --- 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.