The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH net] team: validate the length of U32 and S32 option data
@ 2026-11-11  8:59 Wenbo Sun
  2026-11-12  1:22 ` Hangbin Liu
  0 siblings, 1 reply; 3+ messages in thread
From: Wenbo Sun @ 2026-11-11  8:59 UTC (permalink / raw)
  To: Jiri Pirko, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: netdev, linux-kernel, sunwenbo, 1342116488

In team_nl_options_set_doit(), TEAM_ATTR_OPTION_DATA attribute policy is
NLA_BINARY in team_attr_option_nl_policy because the option data format
depends on the option type.

When setting an option of type TEAM_OPTION_TYPE_U32 or
TEAM_OPTION_TYPE_S32, ctx.data.u32_val or ctx.data.s32_val is retrieved via
nla_get_u32(attr_data) or nla_get_s32(attr_data) without validating that
the attribute payload is at least sizeof(u32) or sizeof(s32). If userspace
supplies truncated option data (e.g. 0 to 3 bytes), nla_get_u32() or
nla_get_s32() will perform an out-of-bounds read past the attribute data
buffer.

This issue was identified during a code audit of Netlink option handlers
in the team driver. The bug was confirmed and reproduced on a UML kernel:
passing truncated option data for notify_peers_count (U32) or priority
(S32) succeeded on an unpatched kernel instead of failing validation.

Add explicit length checks for TEAM_OPTION_TYPE_U32 and
TEAM_OPTION_TYPE_S32 in team_nl_options_set_doit() to reject truncated
attributes with -EINVAL, similar to commit 5bef3ac184b5 ("team: better
TEAM_OPTION_TYPE_STRING validation").

Tested:
- In UML, verified that sending truncated U32 and S32 option data (0 to 3
  bytes) is rejected with -EINVAL on a patched kernel, while valid U32 and
  S32 option settings succeed.
- Built with W=1 on UML, x86_64 allmodconfig, and x86_64 allyesconfig with
  zero new warnings or errors.

Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
Signed-off-by: Wenbo Sun <sunwenbo@cmss.chinamobile.com>
---
 drivers/net/team/team_core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index 957cc7885d2f..617f13e1620c 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -2833,6 +2833,10 @@ int team_nl_options_set_doit(struct sk_buff *skb, struct genl_info *info)
 			ctx.info = opt_inst_info;
 			switch (opt_type) {
 			case TEAM_OPTION_TYPE_U32:
+				if (nla_len(attr_data) < sizeof(u32)) {
+					err = -EINVAL;
+					goto team_put;
+				}
 				ctx.data.u32_val = nla_get_u32(attr_data);
 				break;
 			case TEAM_OPTION_TYPE_STRING:
@@ -2852,6 +2856,10 @@ int team_nl_options_set_doit(struct sk_buff *skb, struct genl_info *info)
 				ctx.data.bool_val = attr_data ? true : false;
 				break;
 			case TEAM_OPTION_TYPE_S32:
+				if (nla_len(attr_data) < sizeof(s32)) {
+					err = -EINVAL;
+					goto team_put;
+				}
 				ctx.data.s32_val = nla_get_s32(attr_data);
 				break;
 			default:
-- 
2.33.0




^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] team: validate the length of U32 and S32 option data
  2026-11-11  8:59 [PATCH net] team: validate the length of U32 and S32 option data Wenbo Sun
@ 2026-11-12  1:22 ` Hangbin Liu
  2026-11-12  6:58   ` Wenbo Sun
  0 siblings, 1 reply; 3+ messages in thread
From: Hangbin Liu @ 2026-11-12  1:22 UTC (permalink / raw)
  To: Wenbo Sun
  Cc: Jiri Pirko, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, netdev, linux-kernel, 1342116488

Hi Wenbo,
On Wed, Nov 11, 2026 at 04:59:24PM +0800, Wenbo Sun wrote:
> In team_nl_options_set_doit(), TEAM_ATTR_OPTION_DATA attribute policy is
> NLA_BINARY in team_attr_option_nl_policy because the option data format
> depends on the option type.
> 
> When setting an option of type TEAM_OPTION_TYPE_U32 or
> TEAM_OPTION_TYPE_S32, ctx.data.u32_val or ctx.data.s32_val is retrieved via
> nla_get_u32(attr_data) or nla_get_s32(attr_data) without validating that
> the attribute payload is at least sizeof(u32) or sizeof(s32). If userspace
> supplies truncated option data (e.g. 0 to 3 bytes), nla_get_u32() or
> nla_get_s32() will perform an out-of-bounds read past the attribute data
> buffer.
> 
> This issue was identified during a code audit of Netlink option handlers
> in the team driver. The bug was confirmed and reproduced on a UML kernel:
> passing truncated option data for notify_peers_count (U32) or priority
> (S32) succeeded on an unpatched kernel instead of failing validation.
> 
> Add explicit length checks for TEAM_OPTION_TYPE_U32 and
> TEAM_OPTION_TYPE_S32 in team_nl_options_set_doit() to reject truncated
> attributes with -EINVAL, similar to commit 5bef3ac184b5 ("team: better
> TEAM_OPTION_TYPE_STRING validation").
> 
> Tested:
> - In UML, verified that sending truncated U32 and S32 option data (0 to 3
>   bytes) is rejected with -EINVAL on a patched kernel, while valid U32 and
>   S32 option settings succeed.
> - Built with W=1 on UML, x86_64 allmodconfig, and x86_64 allyesconfig with
>   zero new warnings or errors.
> 
> Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
> Signed-off-by: Wenbo Sun <sunwenbo@cmss.chinamobile.com>
> ---
>  drivers/net/team/team_core.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
> index 957cc7885d2f..617f13e1620c 100644
> --- a/drivers/net/team/team_core.c
> +++ b/drivers/net/team/team_core.c
> @@ -2833,6 +2833,10 @@ int team_nl_options_set_doit(struct sk_buff *skb, struct genl_info *info)
>  			ctx.info = opt_inst_info;
>  			switch (opt_type) {
>  			case TEAM_OPTION_TYPE_U32:
> +				if (nla_len(attr_data) < sizeof(u32)) {
> +					err = -EINVAL;
> +					goto team_put;
> +				}
>  				ctx.data.u32_val = nla_get_u32(attr_data);
>  				break;
>  			case TEAM_OPTION_TYPE_STRING:
> @@ -2852,6 +2856,10 @@ int team_nl_options_set_doit(struct sk_buff *skb, struct genl_info *info)
>  				ctx.data.bool_val = attr_data ? true : false;
>  				break;
>  			case TEAM_OPTION_TYPE_S32:
> +				if (nla_len(attr_data) < sizeof(s32)) {
> +					err = -EINVAL;
> +					goto team_put;
> +				}
>  				ctx.data.s32_val = nla_get_s32(attr_data);
>  				break;
>  			default:

What if the data size larger than s32/u32? Should we check it with

  if (nla_len(attr_data) != sizeof(s32))

?

Thanks
Hangbin

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] team: validate the length of U32 and S32 option data
  2026-11-12  1:22 ` Hangbin Liu
@ 2026-11-12  6:58   ` Wenbo Sun
  0 siblings, 0 replies; 3+ messages in thread
From: Wenbo Sun @ 2026-11-12  6:58 UTC (permalink / raw)
  To: Hangbin Liu
  Cc: Jiri Pirko, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, netdev, linux-kernel, sunwenbo,
	1342116488

Hi Hangbin,

Thanks for the review and good catch.

On Thu, Nov 12, 2026 at 01:22:11AM +0000, Hangbin Liu wrote:
> What if the data size larger than s32/u32? Should we check it with
>
>   if (nla_len(attr_data) != sizeof(s32))
>
> ?

Agreed -- checking exact length (!= sizeof(u32) for TEAM_OPTION_TYPE_U32
and != sizeof(s32) for TEAM_OPTION_TYPE_S32) is definitely cleaner and
safer.

While '<' prevents the out-of-bounds read, standard NLA_U32 and NLA_S32
attributes require an exact 4-byte payload (as tracked in nla_attr_len[]
in lib/nlattr.c). Since TEAM_ATTR_OPTION_DATA uses NLA_BINARY in
team_attr_option_nl_policy and TEAM_CMD_OPTIONS_SET skips strict
validation, checking only '<' would silently ignore trailing bytes on
oversized attributes (or read the upper 32 bits if an 8-byte integer were
sent on big-endian). Valid callers using nla_put_u32()/nla_put_s32()
always emit exact 4-byte attributes.

I will update both branches to use:

  if (nla_len(attr_data) != sizeof(u32))

and

  if (nla_len(attr_data) != sizeof(s32))

respectively, extend the test to cover oversized payloads as well, and
send a [PATCH net v2] after the 24-hour resend window.

Thanks,
Wenbo



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-12  6:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-11  8:59 [PATCH net] team: validate the length of U32 and S32 option data Wenbo Sun
2026-11-12  1:22 ` Hangbin Liu
2026-11-12  6:58   ` Wenbo Sun

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox