Netdev List
 help / color / mirror / Atom feed
* [PATCH net] net/mlx5: Fix slab-out-of-bounds when handling team device events
@ 2026-09-22 13:51 Anirudh Virdi
  2026-09-22 17:50 ` Shay Drori
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Anirudh Virdi @ 2026-09-22 13:51 UTC (permalink / raw)
  To: netdev
  Cc: saeedm, leon, tariqt, mbloch, andrew+netdev, davem, edumazet,
	kuba, pabeni, shayd, ohartoov, maorg, linux-rdma, Anirudh Virdi

The mlx5_handle_changeupper_event() and mlx5_handle_changeinfodata_event()
functions assume the upper device is a bonding device and call bonding-
specific
APIs like bond_slave_get_rcu() and bond_is_slave_inactive().

When a device is enslaved to a team master instead of bonding, the code
attempts to access the team_port structure as if it were a bond slave
structure. Since team_port is smaller than slave, this causes KASAN to
detect an out-of-bounds memory access.

Fix this by checking explicitly for bonding devices using
netif_is_bond_master() instead of the generic netif_is_lag_master() which
includes both bonding and team devices. This ensures mlx5 only processes
bonding events, not team events, preventing the invalid memory access.

Tested with Mellanox ConnectX-5 on Linux 7.2.0-rc6:
- Bonding: PASS (no regressions)
- Team device: PASS (no KASAN errors)

Fixes: 54493a08e21f ("net/mlx5: Lag, record inactive state of bond device")
Signed-off-by: Anirudh Virdi <avirdi@redhat.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
index 28d16fdc3f06..022a69cdebaf 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
@@ -1897,7 +1897,7 @@ static int mlx5_handle_changeupper_event(struct mlx5_lag *ldev,
 	int changed = 0;
 	int i, idx = -1;
 
-	if (!netif_is_lag_master(upper))
+	if (!netif_is_bond_master(upper))
 		return 0;
 
 	if (info->linking)
@@ -2004,7 +2004,7 @@ static int mlx5_handle_changeinfodata_event(struct mlx5_lag *ldev,
 	bool has_inactive = 0;
 	int idx;
 
-	if (!netif_is_lag_master(ndev))
+	if (!netif_is_bond_master(ndev))
 		return 0;
 
 	rcu_read_lock();
-- 
2.55.0


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

* Re: [PATCH net] net/mlx5: Fix slab-out-of-bounds when handling team device events
  2026-09-22 13:51 [PATCH net] net/mlx5: Fix slab-out-of-bounds when handling team device events Anirudh Virdi
@ 2026-09-22 17:50 ` Shay Drori
  2026-09-22 18:37 ` Mark Bloch
  2026-09-24 13:54 ` netdev-bot+sashiko
  2 siblings, 0 replies; 5+ messages in thread
From: Shay Drori @ 2026-09-22 17:50 UTC (permalink / raw)
  To: Anirudh Virdi, netdev
  Cc: saeedm, leon, tariqt, mbloch, andrew+netdev, davem, edumazet,
	kuba, pabeni, ohartoov, maorg, linux-rdma



On 22/09/2026 16:51, Anirudh Virdi wrote:
> External email: Use caution opening links or attachments
> 
> 
> The mlx5_handle_changeupper_event() and mlx5_handle_changeinfodata_event()
> functions assume the upper device is a bonding device and call bonding-
> specific
> APIs like bond_slave_get_rcu() and bond_is_slave_inactive().
> 
> When a device is enslaved to a team master instead of bonding, the code
> attempts to access the team_port structure as if it were a bond slave
> structure. Since team_port is smaller than slave, this causes KASAN to
> detect an out-of-bounds memory access.
> 
> Fix this by checking explicitly for bonding devices using
> netif_is_bond_master() instead of the generic netif_is_lag_master() which
> includes both bonding and team devices. This ensures mlx5 only processes
> bonding events, not team events, preventing the invalid memory access.
> 
> Tested with Mellanox ConnectX-5 on Linux 7.2.0-rc6:
> - Bonding: PASS (no regressions)
> - Team device: PASS (no KASAN errors)
> 
> Fixes: 54493a08e21f ("net/mlx5: Lag, record inactive state of bond device")
> Signed-off-by: Anirudh Virdi <avirdi@redhat.com>

Reviewed-by: Shay Drory <shayd@nvidia.com>

> ---
>   drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> index 28d16fdc3f06..022a69cdebaf 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> @@ -1897,7 +1897,7 @@ static int mlx5_handle_changeupper_event(struct mlx5_lag *ldev,
>          int changed = 0;
>          int i, idx = -1;
> 
> -       if (!netif_is_lag_master(upper))
> +       if (!netif_is_bond_master(upper))
>                  return 0;
> 
>          if (info->linking)
> @@ -2004,7 +2004,7 @@ static int mlx5_handle_changeinfodata_event(struct mlx5_lag *ldev,
>          bool has_inactive = 0;
>          int idx;
> 
> -       if (!netif_is_lag_master(ndev))
> +       if (!netif_is_bond_master(ndev))
>                  return 0;
> 
>          rcu_read_lock();
> --
> 2.55.0
> 


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

* Re: [PATCH net] net/mlx5: Fix slab-out-of-bounds when handling team device events
  2026-09-22 13:51 [PATCH net] net/mlx5: Fix slab-out-of-bounds when handling team device events Anirudh Virdi
  2026-09-22 17:50 ` Shay Drori
@ 2026-09-22 18:37 ` Mark Bloch
  2026-09-24 13:54 ` netdev-bot+sashiko
  2 siblings, 0 replies; 5+ messages in thread
From: Mark Bloch @ 2026-09-22 18:37 UTC (permalink / raw)
  To: Anirudh Virdi, netdev
  Cc: saeedm, leon, tariqt, andrew+netdev, davem, edumazet, kuba,
	pabeni, shayd, ohartoov, maorg, linux-rdma



On 22/09/2026 16:51, Anirudh Virdi wrote:
> The mlx5_handle_changeupper_event() and mlx5_handle_changeinfodata_event()
> functions assume the upper device is a bonding device and call bonding-
> specific
> APIs like bond_slave_get_rcu() and bond_is_slave_inactive().
> 
> When a device is enslaved to a team master instead of bonding, the code
> attempts to access the team_port structure as if it were a bond slave
> structure. Since team_port is smaller than slave, this causes KASAN to
> detect an out-of-bounds memory access.
> 
> Fix this by checking explicitly for bonding devices using
> netif_is_bond_master() instead of the generic netif_is_lag_master() which
> includes both bonding and team devices. This ensures mlx5 only processes
> bonding events, not team events, preventing the invalid memory access.
> 
> Tested with Mellanox ConnectX-5 on Linux 7.2.0-rc6:
> - Bonding: PASS (no regressions)
> - Team device: PASS (no KASAN errors)
> 
> Fixes: 54493a08e21f ("net/mlx5: Lag, record inactive state of bond device")
> Signed-off-by: Anirudh Virdi <avirdi@redhat.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> index 28d16fdc3f06..022a69cdebaf 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> @@ -1897,7 +1897,7 @@ static int mlx5_handle_changeupper_event(struct mlx5_lag *ldev,
>  	int changed = 0;
>  	int i, idx = -1;
>  
> -	if (!netif_is_lag_master(upper))
> +	if (!netif_is_bond_master(upper))

I think blocking teams might not be what we want to do, what about

@@ -1923,9 +1923,11 @@ static int mlx5_handle_changeupper_event(struct mlx5_lag *ldev,
                      }
              }
              if (i < MLX5_MAX_PORTS) {
-                     slave = bond_slave_get_rcu(ndev_tmp);
-                     if (slave)
-                             has_inactive |= bond_is_slave_inactive(slave);
+                     if (netif_is_bond_master(upper)) {
+                             slave = bond_slave_get_rcu(ndev_tmp);
+                             if (slave)
+                                     has_inactive |= bond_is_slave_inactive(slave);
+                     }
                      bond_status |= (1 << idx);
              }

I think this works better.
>  		return 0;
>  
>  	if (info->linking)
> @@ -2004,7 +2004,7 @@ static int mlx5_handle_changeinfodata_event(struct mlx5_lag *ldev,
>  	bool has_inactive = 0;
>  	int idx;
>  
> -	if (!netif_is_lag_master(ndev))
> +	if (!netif_is_bond_master(ndev))

I'm okay with this as mlx5 doesn't handle state for teams ports anyway.

Mark


>  		return 0;
>  
>  	rcu_read_lock();


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

* Re: [PATCH net] net/mlx5: Fix slab-out-of-bounds when handling team device events
  2026-09-22 13:51 [PATCH net] net/mlx5: Fix slab-out-of-bounds when handling team device events Anirudh Virdi
  2026-09-22 17:50 ` Shay Drori
  2026-09-22 18:37 ` Mark Bloch
@ 2026-09-24 13:54 ` netdev-bot+sashiko
       [not found]   ` <CAJHXLyHH-RG_y04yT-SV+r3+_scac15iKrMtVfNG9Ti3wQFmrw@mail.gmail.com>
  2 siblings, 1 reply; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-09-24 13:54 UTC (permalink / raw)
  To: avirdi
  Cc: netdev, saeedm, leon, tariqt, mbloch, andrew+netdev, davem,
	edumazet, kuba, pabeni, shayd, ohartoov, maorg, linux-rdma

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Over-broad guard: replacing netif_is_lag_master() with
  netif_is_bond_master() at the very top of…
- [Low] Commit message inaccuracy: the message states that
  mlx5_handle_changeinfodata_event() suffers the same team_port-…

Pre-existing issues:
- [High] The identical bug class the patch claims to eliminate remains
  reachable in the mlx5e TC mirred offload path: get_fdb_out_dev()…

--

net/mlx5: Fix slab-out-of-bounds when handling team device events

This changes the two LAG notifier helpers in mlx5's lag.c to gate on
netif_is_bond_master() instead of netif_is_lag_master(), so that a team
master no longer reaches bond_slave_get_rcu()/bond_is_slave_inactive().
The reported symptom is a KASAN out-of-bounds read when a team_port is
interpreted as a struct slave.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> index dd14cdc378de0..2714d39ae7c24 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> @@ -1922,7 +1922,7 @@ static int mlx5_handle_changeupper_event(struct mlx5_lag *ldev,
>  	int changed = 0;
>  	int i, idx = -1;
>  
> -	if (!netif_is_lag_master(upper))
> +	if (!netif_is_bond_master(upper))
>  		return 0;

[Severity: High]
Does this early return do more than avoid the bonding-only calls?  The
only team-unsafe statements in mlx5_handle_changeupper_event() are further
down in the for_each_netdev_in_bond_rcu() loop:

	slave = bond_slave_get_rcu(ndev_tmp);
	if (slave)
		has_inactive |= bond_is_slave_inactive(slave);

Returning 0 at the top of the function also skips the computation of
tracker->tx_type, tracker->hash_type and tracker->is_bonded for any team
master.

Since mlx5_lag_netdev_event() only writes the tracker back and queues the
work when a handler returns non-zero:

	if (changed)
		mlx5_lag_update_tracker_speed(&tracker, ndev);

	ldev->tracker = tracker;

	if (changed)
		mlx5_queue_bond_work(ldev, 0);

and mlx5_do_bond() gates activation on the tracker:

	do_bond = tracker.is_bonded && mlx5_lag_check_prereq(ldev);

can is_bonded ever become true again for mlx5 ports aggregated by a team
master?  Team does advertise the TX types mlx5 accepts —
team_upper_dev_link() sets lag_upper_info.tx_type = team->mode->lag_tx_type,
with NETDEV_LAG_TX_TYPE_ACTIVEBACKUP in team_mode_activebackup and
NETDEV_LAG_TX_TYPE_HASH in team_mode_loadbalance — which are exactly the
two values the mode_supported test in this function allows.

Would gating only the bond_slave_get_rcu()/bond_is_slave_inactive() block
on netif_is_bond_master() remove the out-of-bounds read while keeping
team-based LAG tracking?

Related to this, does the driver's notifier state machine become
asymmetric?  mlx5_handle_changelowerstate_event() still admits team ports:

	if (!netif_is_lag_port(ndev))
		return 0;

and returns 1, so team NETDEV_CHANGELOWERSTATE events keep updating
tracker->netdev_state[] and queueing bond work while is_bonded can never be
set.  mlx5_lag_update_tracker_speed() also still resolves a team master as
the bond device:

	if (netif_is_lag_master(ndev))
		bond_dev = ndev;

and publishes its speed through mlx5_lag_query_bond_speed().

One more question on the changelog: with a Fixes: tag aimed at stable,
should the loss of hardware LAG offload for team-aggregated ports be
mentioned, since there is no extack or dmesg indication when the handler
bails out early?

>  
>  	if (info->linking)
> @@ -2029,7 +2029,7 @@ static int mlx5_handle_changeinfodata_event(struct mlx5_lag *ldev,
>  	bool has_inactive = 0;
>  	int idx;
>  
> -	if (!netif_is_lag_master(ndev))
> +	if (!netif_is_bond_master(ndev))
>  		return 0;

[Severity: Low]
The commit message says:

    The mlx5_handle_changeupper_event() and mlx5_handle_changeinfodata_event()
    functions assume the upper device is a bonding device and call bonding-
    specific
    APIs like bond_slave_get_rcu() and bond_is_slave_inactive().

Can mlx5_handle_changeinfodata_event() actually be entered with a team
master?  NETDEV_CHANGEINFODATA has a single emitter in the tree, in
bond_option_active_slave_set():

	call_netdevice_notifiers(NETDEV_CHANGEINFODATA, bond->dev);

drivers/net/team/ never emits it, so this hunk looks like hardening of an
unreachable path rather than a second fix.  Could the changelog be narrowed
to the mlx5_handle_changeupper_event() path, so stable backporters see the
real scope?

Also, the sentence above is wrapped mid-word ("bonding-\nspecific") in the
changelog.

>  
>  	rcu_read_lock();

[Severity: High]
This isn't a bug introduced by this patch, but the same pattern is still
present in the mlx5e TC mirred offload path, and it may be worth fixing in
the same series.  get_fdb_out_dev() in
drivers/net/ethernet/mellanox/mlx5/core/en/tc/act/mirred.c still uses the
generic check before a bonding-only accessor:

	} else if (netif_is_lag_master(out_dev)) {
		fdb_out_dev = bond_option_active_slave_get_rcu(netdev_priv(out_dev));

For a team master, netdev_priv() is a struct team, and
bond_option_active_slave_get_rcu() reads it as struct bonding:

include/net/bonding.h:bond_option_active_slave_get_rcu() {
	const struct slave *slave = rcu_dereference_rtnl(bond->curr_active_slave);

	return bond_uses_primary(bond) && slave ? slave->dev : NULL;
}

bond_uses_primary() reads BOND_MODE(bond) out of unrelated bytes of the team
private area, and bond->curr_active_slave aliases team->header_ops_cache
(struct team starts with pcpu_stats, it has no leading dev member).  If the
garbage mode value happens to be one of the primary modes, does this
dereference header_ops_cache as struct slave * and return slave->dev as a
bogus struct net_device *, which mlx5e_eswitch_rep() then dereferences?

The reachable caller is parse_mirred():

	out_dev = get_fdb_out_dev(uplink_dev, out_dev);
	if (!out_dev)
		return -ENODEV;

reached from tc_act_parse_mirred() when
netdev_port_same_parent_id(priv->netdev, out_dev) holds, which a team master
over mlx5 representors satisfies.  Should this site use
netif_is_bond_master() too, the way mpesw.c already does?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922135123.46205-1-avirdi%40redhat.com

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

* Re: [PATCH net] net/mlx5: Fix slab-out-of-bounds when handling team device events
       [not found]   ` <CAJHXLyHH-RG_y04yT-SV+r3+_scac15iKrMtVfNG9Ti3wQFmrw@mail.gmail.com>
@ 2026-09-25  6:45     ` Anirudh Virdi
  0 siblings, 0 replies; 5+ messages in thread
From: Anirudh Virdi @ 2026-09-25  6:45 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: netdev, saeedm, leon, tariqt, mbloch, andrew+netdev, davem,
	edumazet, kuba, pabeni, shayd, ohartoov, maorg, linux-rdma

Thank you for the detailed analysis, Sashiko!

The concerns you've raised have been addressed in v2 of this patch:

1. Over-broad guard: v2 now keeps netif_is_lag_master() at the top of
   mlx5_handle_changeupper_event() and only wraps the bond-specific API calls
   (bond_slave_get_rcu/bond_is_slave_inactive) with netif_is_bond_master().
   This allows team LAG tracking to continue working while preventing the
   out-of-bounds read.

2. Commit message inaccuracy: v2 correctly narrows the scope to only
   mlx5_handle_changeupper_event(), acknowledging that
   mlx5_handle_changeinfodata_event() is only reached from bonding code paths.

3. TC mirred issue: Noted as pre-existing. This can be addressed in a
   separate patch series after this fix merges.

v2 is available at:
https://lore.kernel.org/netdev/20260924064244.94045-1-avirdi@redhat.com/

Feedback appreciated!


On Fri, Sep 25, 2026 at 12:12 PM Anirudh Virdi <avirdi@redhat.com> wrote:
>
> Thank you for the detailed analysis, Sashiko!
>
> The concerns you've raised have been addressed in v2 of this patch:
>
> 1. Over-broad guard: v2 now keeps netif_is_lag_master() at the top of
>    mlx5_handle_changeupper_event() and only wraps the bond-specific API calls
>    (bond_slave_get_rcu/bond_is_slave_inactive) with netif_is_bond_master().
>    This allows team LAG tracking to continue working while preventing the
>    out-of-bounds read.
>
> 2. Commit message inaccuracy: v2 correctly narrows the scope to only
>    mlx5_handle_changeupper_event(), acknowledging that
>    mlx5_handle_changeinfodata_event() is only reached from bonding code paths.
>
> 3. TC mirred issue: Noted as pre-existing. This can be addressed in a
>    separate patch series after this fix merges.
>
> v2 is available at:
> https://lore.kernel.org/netdev/20260924064244.94045-1-avirdi@redhat.com/
>
> Feedback appreciated!
>
>
> On Thu, Sep 24, 2026 at 7:30 PM <netdev-bot+sashiko@kernel.org> wrote:
>>
>> Thank you for your contribution! Sashiko AI review found 2 potential
>> issue(s) to consider.
>>
>> Critical: 0 · High: 1 (1) · Medium: 0 · Low: 1
>>
>> If you believe none of the issues are legitimate, please reply to each
>> issue of Medium severity or higher and explain why the AI is wrong.
>> If any of them are legitimate and you plan to respin, please let patchwork
>> know by including "pw-bot: cr" as a separate line at the end of your reply
>> (one such reply per series is enough).
>>
>> - [High] Over-broad guard: replacing netif_is_lag_master() with
>>   netif_is_bond_master() at the very top of…
>> - [Low] Commit message inaccuracy: the message states that
>>   mlx5_handle_changeinfodata_event() suffers the same team_port-…
>>
>> Pre-existing issues:
>> - [High] The identical bug class the patch claims to eliminate remains
>>   reachable in the mlx5e TC mirred offload path: get_fdb_out_dev()…
>>
>> --
>>
>> net/mlx5: Fix slab-out-of-bounds when handling team device events
>>
>> This changes the two LAG notifier helpers in mlx5's lag.c to gate on
>> netif_is_bond_master() instead of netif_is_lag_master(), so that a team
>> master no longer reaches bond_slave_get_rcu()/bond_is_slave_inactive().
>> The reported symptom is a KASAN out-of-bounds read when a team_port is
>> interpreted as a struct slave.
>>
>> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
>> > index dd14cdc378de0..2714d39ae7c24 100644
>> > --- a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
>> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
>> > @@ -1922,7 +1922,7 @@ static int mlx5_handle_changeupper_event(struct mlx5_lag *ldev,
>> >       int changed = 0;
>> >       int i, idx = -1;
>> >
>> > -     if (!netif_is_lag_master(upper))
>> > +     if (!netif_is_bond_master(upper))
>> >               return 0;
>>
>> [Severity: High]
>> Does this early return do more than avoid the bonding-only calls?  The
>> only team-unsafe statements in mlx5_handle_changeupper_event() are further
>> down in the for_each_netdev_in_bond_rcu() loop:
>>
>>         slave = bond_slave_get_rcu(ndev_tmp);
>>         if (slave)
>>                 has_inactive |= bond_is_slave_inactive(slave);
>>
>> Returning 0 at the top of the function also skips the computation of
>> tracker->tx_type, tracker->hash_type and tracker->is_bonded for any team
>> master.
>>
>> Since mlx5_lag_netdev_event() only writes the tracker back and queues the
>> work when a handler returns non-zero:
>>
>>         if (changed)
>>                 mlx5_lag_update_tracker_speed(&tracker, ndev);
>>
>>         ldev->tracker = tracker;
>>
>>         if (changed)
>>                 mlx5_queue_bond_work(ldev, 0);
>>
>> and mlx5_do_bond() gates activation on the tracker:
>>
>>         do_bond = tracker.is_bonded && mlx5_lag_check_prereq(ldev);
>>
>> can is_bonded ever become true again for mlx5 ports aggregated by a team
>> master?  Team does advertise the TX types mlx5 accepts —
>> team_upper_dev_link() sets lag_upper_info.tx_type = team->mode->lag_tx_type,
>> with NETDEV_LAG_TX_TYPE_ACTIVEBACKUP in team_mode_activebackup and
>> NETDEV_LAG_TX_TYPE_HASH in team_mode_loadbalance — which are exactly the
>> two values the mode_supported test in this function allows.
>>
>> Would gating only the bond_slave_get_rcu()/bond_is_slave_inactive() block
>> on netif_is_bond_master() remove the out-of-bounds read while keeping
>> team-based LAG tracking?
>>
>> Related to this, does the driver's notifier state machine become
>> asymmetric?  mlx5_handle_changelowerstate_event() still admits team ports:
>>
>>         if (!netif_is_lag_port(ndev))
>>                 return 0;
>>
>> and returns 1, so team NETDEV_CHANGELOWERSTATE events keep updating
>> tracker->netdev_state[] and queueing bond work while is_bonded can never be
>> set.  mlx5_lag_update_tracker_speed() also still resolves a team master as
>> the bond device:
>>
>>         if (netif_is_lag_master(ndev))
>>                 bond_dev = ndev;
>>
>> and publishes its speed through mlx5_lag_query_bond_speed().
>>
>> One more question on the changelog: with a Fixes: tag aimed at stable,
>> should the loss of hardware LAG offload for team-aggregated ports be
>> mentioned, since there is no extack or dmesg indication when the handler
>> bails out early?
>>
>> >
>> >       if (info->linking)
>> > @@ -2029,7 +2029,7 @@ static int mlx5_handle_changeinfodata_event(struct mlx5_lag *ldev,
>> >       bool has_inactive = 0;
>> >       int idx;
>> >
>> > -     if (!netif_is_lag_master(ndev))
>> > +     if (!netif_is_bond_master(ndev))
>> >               return 0;
>>
>> [Severity: Low]
>> The commit message says:
>>
>>     The mlx5_handle_changeupper_event() and mlx5_handle_changeinfodata_event()
>>     functions assume the upper device is a bonding device and call bonding-
>>     specific
>>     APIs like bond_slave_get_rcu() and bond_is_slave_inactive().
>>
>> Can mlx5_handle_changeinfodata_event() actually be entered with a team
>> master?  NETDEV_CHANGEINFODATA has a single emitter in the tree, in
>> bond_option_active_slave_set():
>>
>>         call_netdevice_notifiers(NETDEV_CHANGEINFODATA, bond->dev);
>>
>> drivers/net/team/ never emits it, so this hunk looks like hardening of an
>> unreachable path rather than a second fix.  Could the changelog be narrowed
>> to the mlx5_handle_changeupper_event() path, so stable backporters see the
>> real scope?
>>
>> Also, the sentence above is wrapped mid-word ("bonding-\nspecific") in the
>> changelog.
>>
>> >
>> >       rcu_read_lock();
>>
>> [Severity: High]
>> This isn't a bug introduced by this patch, but the same pattern is still
>> present in the mlx5e TC mirred offload path, and it may be worth fixing in
>> the same series.  get_fdb_out_dev() in
>> drivers/net/ethernet/mellanox/mlx5/core/en/tc/act/mirred.c still uses the
>> generic check before a bonding-only accessor:
>>
>>         } else if (netif_is_lag_master(out_dev)) {
>>                 fdb_out_dev = bond_option_active_slave_get_rcu(netdev_priv(out_dev));
>>
>> For a team master, netdev_priv() is a struct team, and
>> bond_option_active_slave_get_rcu() reads it as struct bonding:
>>
>> include/net/bonding.h:bond_option_active_slave_get_rcu() {
>>         const struct slave *slave = rcu_dereference_rtnl(bond->curr_active_slave);
>>
>>         return bond_uses_primary(bond) && slave ? slave->dev : NULL;
>> }
>>
>> bond_uses_primary() reads BOND_MODE(bond) out of unrelated bytes of the team
>> private area, and bond->curr_active_slave aliases team->header_ops_cache
>> (struct team starts with pcpu_stats, it has no leading dev member).  If the
>> garbage mode value happens to be one of the primary modes, does this
>> dereference header_ops_cache as struct slave * and return slave->dev as a
>> bogus struct net_device *, which mlx5e_eswitch_rep() then dereferences?
>>
>> The reachable caller is parse_mirred():
>>
>>         out_dev = get_fdb_out_dev(uplink_dev, out_dev);
>>         if (!out_dev)
>>                 return -ENODEV;
>>
>> reached from tc_act_parse_mirred() when
>> netdev_port_same_parent_id(priv->netdev, out_dev) holds, which a team master
>> over mlx5 representors satisfies.  Should this site use
>> netif_is_bond_master() too, the way mpesw.c already does?
>>
>> --
>> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922135123.46205-1-avirdi%40redhat.com
>>
>
>
> --
>
> Anirudh Virdi



-- 

Anirudh Virdi


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

end of thread, other threads:[~2026-09-25  6:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-22 13:51 [PATCH net] net/mlx5: Fix slab-out-of-bounds when handling team device events Anirudh Virdi
2026-09-22 17:50 ` Shay Drori
2026-09-22 18:37 ` Mark Bloch
2026-09-24 13:54 ` netdev-bot+sashiko
     [not found]   ` <CAJHXLyHH-RG_y04yT-SV+r3+_scac15iKrMtVfNG9Ti3wQFmrw@mail.gmail.com>
2026-09-25  6:45     ` Anirudh Virdi

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