* [PATCH net v2] net: team: fix NULL pointer dereference in team_xmit during mode change
@ 2026-05-09 18:18 Weiming Shi
2026-05-10 15:25 ` Jakub Kicinski
0 siblings, 1 reply; 4+ messages in thread
From: Weiming Shi @ 2026-05-09 18:18 UTC (permalink / raw)
To: Jiri Pirko, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, Xiang Mei, Weiming Shi
__team_change_mode() clears team->ops with memset() before restoring
safe dummy handlers via team_adjust_ops(). A concurrent team_xmit()
running under RCU on another CPU can read team->ops.transmit during
this window and call a NULL function pointer, crashing the kernel.
The race requires CAP_NET_ADMIN (in init_user_ns) to trigger via
TEAM_CMD_OPTIONS_SET, plus AF_PACKET sendto() on a team device with
forced carrier and no ports.
BUG: kernel NULL pointer dereference, address: 0000000000000000
Oops: 0010 [#1] SMP KASAN NOPTI
RIP: 0010:0x0
Call Trace:
team_xmit (drivers/net/team/team_core.c:1853)
dev_hard_start_xmit (net/core/dev.c:3904)
__dev_queue_xmit (net/core/dev.c:4871)
packet_sendmsg (net/packet/af_packet.c:3109)
__sys_sendto (net/socket.c:2265)
Fix this on the writer side by replacing the memset()/memcpy() with
per-field updates that keep transmit and receive always valid via
smp_store_release(), paired with smp_load_acquire() on the reader
side. A synchronize_net() before exit_op() drains in-flight readers
before tearing down mode state.
Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
Reported-by: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
v2:
- Move fix from data path (reader-side NULL fallback) to
configuration path (writer-side per-field updates), as suggested
by the reviewer.
- Use smp_store_release()/smp_load_acquire() instead of plain
stores/loads for proper ordering on weakly-ordered architectures.
- Add synchronize_net() before exit_op() to drain in-flight readers
and prevent use-after-free of mode private state.
drivers/net/team/team_core.c | 46 ++++++++++++++++++++++++++----------
1 file changed, 33 insertions(+), 13 deletions(-)
diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index 0c87f9972..dabee3aa7 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -534,21 +534,22 @@ static void team_adjust_ops(struct team *team)
if (!team->tx_en_port_count || !team_is_mode_set(team) ||
!team->mode->ops->transmit)
- team->ops.transmit = team_dummy_transmit;
+ smp_store_release(&team->ops.transmit, team_dummy_transmit);
else
- team->ops.transmit = team->mode->ops->transmit;
+ smp_store_release(&team->ops.transmit, team->mode->ops->transmit);
if (!team->rx_en_port_count || !team_is_mode_set(team) ||
!team->mode->ops->receive)
- team->ops.receive = team_dummy_receive;
+ smp_store_release(&team->ops.receive, team_dummy_receive);
else
- team->ops.receive = team->mode->ops->receive;
+ smp_store_release(&team->ops.receive, team->mode->ops->receive);
}
/*
- * We can benefit from the fact that it's ensured no port is present
- * at the time of mode change. Therefore no packets are in fly so there's no
- * need to set mode operations in any special way.
+ * team_change_mode() ensures no ports are present during mode change,
+ * but lockless readers (AF_PACKET) can still reach team_xmit(). Use
+ * smp_store_release() to publish safe dummy handlers before teardown,
+ * and synchronize_net() to drain in-flight readers.
*/
static int __team_change_mode(struct team *team,
const struct team_mode *new_mode)
@@ -557,9 +558,23 @@ static int __team_change_mode(struct team *team,
if (team_is_mode_set(team)) {
void (*exit_op)(struct team *team) = team->ops.exit;
- /* Clear ops area so no callback is called any longer */
- memset(&team->ops, 0, sizeof(struct team_mode_ops));
- team_adjust_ops(team);
+ /* Install dummy handlers for locklessly-read hot-path ops
+ * first, then clear cold-path ops that are only used under
+ * RTNL.
+ */
+ smp_store_release(&team->ops.transmit, team_dummy_transmit);
+ smp_store_release(&team->ops.receive, team_dummy_receive);
+ team->ops.init = NULL;
+ team->ops.exit = NULL;
+ team->ops.port_enter = NULL;
+ team->ops.port_leave = NULL;
+ team->ops.port_change_dev_addr = NULL;
+ team->ops.port_tx_disabled = NULL;
+
+ /* Ensure in-flight readers using old handlers have finished
+ * before tearing down mode state they may depend on.
+ */
+ synchronize_net();
if (exit_op)
exit_op(team);
@@ -582,7 +597,12 @@ static int __team_change_mode(struct team *team,
}
team->mode = new_mode;
- memcpy(&team->ops, new_mode->ops, sizeof(struct team_mode_ops));
+ team->ops.init = new_mode->ops->init;
+ team->ops.exit = new_mode->ops->exit;
+ team->ops.port_enter = new_mode->ops->port_enter;
+ team->ops.port_leave = new_mode->ops->port_leave;
+ team->ops.port_change_dev_addr = new_mode->ops->port_change_dev_addr;
+ team->ops.port_tx_disabled = new_mode->ops->port_tx_disabled;
team_adjust_ops(team);
return 0;
@@ -743,7 +763,7 @@ static rx_handler_result_t team_handle_frame(struct sk_buff **pskb)
/* allow exact match delivery for disabled ports */
res = RX_HANDLER_EXACT;
} else {
- res = team->ops.receive(team, port, skb);
+ res = smp_load_acquire(&team->ops.receive)(team, port, skb);
}
if (res == RX_HANDLER_ANOTHER) {
struct team_pcpu_stats *pcpu_stats;
@@ -1845,7 +1865,7 @@ static netdev_tx_t team_xmit(struct sk_buff *skb, struct net_device *dev)
tx_success = team_queue_override_transmit(team, skb);
if (!tx_success)
- tx_success = team->ops.transmit(team, skb);
+ tx_success = smp_load_acquire(&team->ops.transmit)(team, skb);
if (tx_success) {
struct team_pcpu_stats *pcpu_stats;
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net v2] net: team: fix NULL pointer dereference in team_xmit during mode change
2026-05-09 18:18 [PATCH net v2] net: team: fix NULL pointer dereference in team_xmit during mode change Weiming Shi
@ 2026-05-10 15:25 ` Jakub Kicinski
2026-05-10 16:06 ` Weiming Shi
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Kicinski @ 2026-05-10 15:25 UTC (permalink / raw)
To: Weiming Shi
Cc: Jiri Pirko, Andrew Lunn, David S . Miller, Eric Dumazet,
Paolo Abeni, netdev, Xiang Mei
On Sat, 9 May 2026 11:18:26 -0700 Weiming Shi wrote:
> __team_change_mode() clears team->ops with memset() before restoring
> safe dummy handlers via team_adjust_ops(). A concurrent team_xmit()
> running under RCU on another CPU can read team->ops.transmit during
> this window and call a NULL function pointer, crashing the kernel.
>
> The race requires CAP_NET_ADMIN (in init_user_ns) to trigger via
> TEAM_CMD_OPTIONS_SET, plus AF_PACKET sendto() on a team device with
> forced carrier and no ports.
>
> BUG: kernel NULL pointer dereference, address: 0000000000000000
> Oops: 0010 [#1] SMP KASAN NOPTI
> RIP: 0010:0x0
> Call Trace:
> team_xmit (drivers/net/team/team_core.c:1853)
> dev_hard_start_xmit (net/core/dev.c:3904)
> __dev_queue_xmit (net/core/dev.c:4871)
> packet_sendmsg (net/packet/af_packet.c:3109)
> __sys_sendto (net/socket.c:2265)
>
> Fix this on the writer side by replacing the memset()/memcpy() with
> per-field updates that keep transmit and receive always valid via
> smp_store_release(), paired with smp_load_acquire() on the reader
> side. A synchronize_net() before exit_op() drains in-flight readers
> before tearing down mode state.
Barriers are between things. What are the release / acquire barriers
synchronizing.
> Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
> v2:
> - Move fix from data path (reader-side NULL fallback) to
> configuration path (writer-side per-field updates), as suggested
> by the reviewer.
> - Use smp_store_release()/smp_load_acquire() instead of plain
> stores/loads for proper ordering on weakly-ordered architectures.
> - Add synchronize_net() before exit_op() to drain in-flight readers
> and prevent use-after-free of mode private state.
>
> drivers/net/team/team_core.c | 46 ++++++++++++++++++++++++++----------
> 1 file changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
> index 0c87f9972..dabee3aa7 100644
> --- a/drivers/net/team/team_core.c
> +++ b/drivers/net/team/team_core.c
> @@ -534,21 +534,22 @@ static void team_adjust_ops(struct team *team)
>
> if (!team->tx_en_port_count || !team_is_mode_set(team) ||
> !team->mode->ops->transmit)
> - team->ops.transmit = team_dummy_transmit;
> + smp_store_release(&team->ops.transmit, team_dummy_transmit);
> else
> - team->ops.transmit = team->mode->ops->transmit;
> + smp_store_release(&team->ops.transmit, team->mode->ops->transmit);
>
> if (!team->rx_en_port_count || !team_is_mode_set(team) ||
> !team->mode->ops->receive)
> - team->ops.receive = team_dummy_receive;
> + smp_store_release(&team->ops.receive, team_dummy_receive);
> else
> - team->ops.receive = team->mode->ops->receive;
> + smp_store_release(&team->ops.receive, team->mode->ops->receive);
> }
>
> /*
> - * We can benefit from the fact that it's ensured no port is present
> - * at the time of mode change. Therefore no packets are in fly so there's no
> - * need to set mode operations in any special way.
> + * team_change_mode() ensures no ports are present during mode change,
> + * but lockless readers (AF_PACKET) can still reach team_xmit(). Use
Why is AF_PACKET relevant here??
> + * smp_store_release() to publish safe dummy handlers before teardown,
> + * and synchronize_net() to drain in-flight readers.
> */
> static int __team_change_mode(struct team *team,
> const struct team_mode *new_mode)
> @@ -557,9 +558,23 @@ static int __team_change_mode(struct team *team,
> if (team_is_mode_set(team)) {
> void (*exit_op)(struct team *team) = team->ops.exit;
>
> - /* Clear ops area so no callback is called any longer */
> - memset(&team->ops, 0, sizeof(struct team_mode_ops));
> - team_adjust_ops(team);
> + /* Install dummy handlers for locklessly-read hot-path ops
> + * first, then clear cold-path ops that are only used under
> + * RTNL.
> + */
> + smp_store_release(&team->ops.transmit, team_dummy_transmit);
> + smp_store_release(&team->ops.receive, team_dummy_receive);
> + team->ops.init = NULL;
> + team->ops.exit = NULL;
> + team->ops.port_enter = NULL;
> + team->ops.port_leave = NULL;
> + team->ops.port_change_dev_addr = NULL;
> + team->ops.port_tx_disabled = NULL;
> +
> + /* Ensure in-flight readers using old handlers have finished
> + * before tearing down mode state they may depend on.
> + */
> + synchronize_net();
>
> if (exit_op)
> exit_op(team);
> @@ -582,7 +597,12 @@ static int __team_change_mode(struct team *team,
> }
>
> team->mode = new_mode;
> - memcpy(&team->ops, new_mode->ops, sizeof(struct team_mode_ops));
> + team->ops.init = new_mode->ops->init;
> + team->ops.exit = new_mode->ops->exit;
> + team->ops.port_enter = new_mode->ops->port_enter;
> + team->ops.port_leave = new_mode->ops->port_leave;
> + team->ops.port_change_dev_addr = new_mode->ops->port_change_dev_addr;
> + team->ops.port_tx_disabled = new_mode->ops->port_tx_disabled;
> team_adjust_ops(team);
>
> return 0;
> @@ -743,7 +763,7 @@ static rx_handler_result_t team_handle_frame(struct sk_buff **pskb)
> /* allow exact match delivery for disabled ports */
> res = RX_HANDLER_EXACT;
> } else {
> - res = team->ops.receive(team, port, skb);
> + res = smp_load_acquire(&team->ops.receive)(team, port, skb);
> }
> if (res == RX_HANDLER_ANOTHER) {
> struct team_pcpu_stats *pcpu_stats;
> @@ -1845,7 +1865,7 @@ static netdev_tx_t team_xmit(struct sk_buff *skb, struct net_device *dev)
>
> tx_success = team_queue_override_transmit(team, skb);
> if (!tx_success)
> - tx_success = team->ops.transmit(team, skb);
> + tx_success = smp_load_acquire(&team->ops.transmit)(team, skb);
> if (tx_success) {
> struct team_pcpu_stats *pcpu_stats;
>
--
pw-bot: cr
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net v2] net: team: fix NULL pointer dereference in team_xmit during mode change
2026-05-10 15:25 ` Jakub Kicinski
@ 2026-05-10 16:06 ` Weiming Shi
2026-05-10 16:59 ` Jakub Kicinski
0 siblings, 1 reply; 4+ messages in thread
From: Weiming Shi @ 2026-05-10 16:06 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Jiri Pirko, Andrew Lunn, David S . Miller, Eric Dumazet,
Paolo Abeni, netdev, Xiang Mei
On 26-05-10 08:25, Jakub Kicinski wrote:
> On Sat, 9 May 2026 11:18:26 -0700 Weiming Shi wrote:
> > __team_change_mode() clears team->ops with memset() before restoring
> > safe dummy handlers via team_adjust_ops(). A concurrent team_xmit()
> > running under RCU on another CPU can read team->ops.transmit during
> > this window and call a NULL function pointer, crashing the kernel.
> >
> > The race requires CAP_NET_ADMIN (in init_user_ns) to trigger via
> > TEAM_CMD_OPTIONS_SET, plus AF_PACKET sendto() on a team device with
> > forced carrier and no ports.
> >
> > BUG: kernel NULL pointer dereference, address: 0000000000000000
> > Oops: 0010 [#1] SMP KASAN NOPTI
> > RIP: 0010:0x0
> > Call Trace:
> > team_xmit (drivers/net/team/team_core.c:1853)
> > dev_hard_start_xmit (net/core/dev.c:3904)
> > __dev_queue_xmit (net/core/dev.c:4871)
> > packet_sendmsg (net/packet/af_packet.c:3109)
> > __sys_sendto (net/socket.c:2265)
> >
> > Fix this on the writer side by replacing the memset()/memcpy() with
> > per-field updates that keep transmit and receive always valid via
> > smp_store_release(), paired with smp_load_acquire() on the reader
> > side. A synchronize_net() before exit_op() drains in-flight readers
> > before tearing down mode state.
>
> Barriers are between things. What are the release / acquire barriers
> synchronizing.
The handler function pointer against the mode_priv state it operates
on. On setup, init() writes mode_priv, then smp_store_release()
publishes the real handler - the paired smp_load_acquire() in the
reader ensures the handler sees that state. On teardown,
smp_store_release() publishes the dummy before synchronize_net()
drains readers, so exit_op() won't tear down state under an
in-flight reader.
>
> > Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
> > Reported-by: Xiang Mei <xmei5@asu.edu>
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> > ---
> > v2:
> > - Move fix from data path (reader-side NULL fallback) to
> > configuration path (writer-side per-field updates), as suggested
> > by the reviewer.
> > - Use smp_store_release()/smp_load_acquire() instead of plain
> > stores/loads for proper ordering on weakly-ordered architectures.
> > - Add synchronize_net() before exit_op() to drain in-flight readers
> > and prevent use-after-free of mode private state.
> >
> > drivers/net/team/team_core.c | 46 ++++++++++++++++++++++++++----------
> > 1 file changed, 33 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
> > index 0c87f9972..dabee3aa7 100644
> > --- a/drivers/net/team/team_core.c
> > +++ b/drivers/net/team/team_core.c
> > @@ -534,21 +534,22 @@ static void team_adjust_ops(struct team *team)
> >
> > if (!team->tx_en_port_count || !team_is_mode_set(team) ||
> > !team->mode->ops->transmit)
> > - team->ops.transmit = team_dummy_transmit;
> > + smp_store_release(&team->ops.transmit, team_dummy_transmit);
> > else
> > - team->ops.transmit = team->mode->ops->transmit;
> > + smp_store_release(&team->ops.transmit, team->mode->ops->transmit);
> >
> > if (!team->rx_en_port_count || !team_is_mode_set(team) ||
> > !team->mode->ops->receive)
> > - team->ops.receive = team_dummy_receive;
> > + smp_store_release(&team->ops.receive, team_dummy_receive);
> > else
> > - team->ops.receive = team->mode->ops->receive;
> > + smp_store_release(&team->ops.receive, team->mode->ops->receive);
> > }
> >
> > /*
> > - * We can benefit from the fact that it's ensured no port is present
> > - * at the time of mode change. Therefore no packets are in fly so there's no
> > - * need to set mode operations in any special way.
> > + * team_change_mode() ensures no ports are present during mode change,
> > + * but lockless readers (AF_PACKET) can still reach team_xmit(). Use
>
> Why is AF_PACKET relevant here??
>
Not specific to the bug, just a reproducer detail. Dropped.
Sending v3 with the updated changelog shortly.
> > + * smp_store_release() to publish safe dummy handlers before teardown,
> > + * and synchronize_net() to drain in-flight readers.
> > */
> > static int __team_change_mode(struct team *team,
> > const struct team_mode *new_mode)
> > @@ -557,9 +558,23 @@ static int __team_change_mode(struct team *team,
> > if (team_is_mode_set(team)) {
> > void (*exit_op)(struct team *team) = team->ops.exit;
> >
> > - /* Clear ops area so no callback is called any longer */
> > - memset(&team->ops, 0, sizeof(struct team_mode_ops));
> > - team_adjust_ops(team);
> > + /* Install dummy handlers for locklessly-read hot-path ops
> > + * first, then clear cold-path ops that are only used under
> > + * RTNL.
> > + */
> > + smp_store_release(&team->ops.transmit, team_dummy_transmit);
> > + smp_store_release(&team->ops.receive, team_dummy_receive);
> > + team->ops.init = NULL;
> > + team->ops.exit = NULL;
> > + team->ops.port_enter = NULL;
> > + team->ops.port_leave = NULL;
> > + team->ops.port_change_dev_addr = NULL;
> > + team->ops.port_tx_disabled = NULL;
> > +
> > + /* Ensure in-flight readers using old handlers have finished
> > + * before tearing down mode state they may depend on.
> > + */
> > + synchronize_net();
> >
> > if (exit_op)
> > exit_op(team);
> > @@ -582,7 +597,12 @@ static int __team_change_mode(struct team *team,
> > }
> >
> > team->mode = new_mode;
> > - memcpy(&team->ops, new_mode->ops, sizeof(struct team_mode_ops));
> > + team->ops.init = new_mode->ops->init;
> > + team->ops.exit = new_mode->ops->exit;
> > + team->ops.port_enter = new_mode->ops->port_enter;
> > + team->ops.port_leave = new_mode->ops->port_leave;
> > + team->ops.port_change_dev_addr = new_mode->ops->port_change_dev_addr;
> > + team->ops.port_tx_disabled = new_mode->ops->port_tx_disabled;
> > team_adjust_ops(team);
> >
> > return 0;
> > @@ -743,7 +763,7 @@ static rx_handler_result_t team_handle_frame(struct sk_buff **pskb)
> > /* allow exact match delivery for disabled ports */
> > res = RX_HANDLER_EXACT;
> > } else {
> > - res = team->ops.receive(team, port, skb);
> > + res = smp_load_acquire(&team->ops.receive)(team, port, skb);
> > }
> > if (res == RX_HANDLER_ANOTHER) {
> > struct team_pcpu_stats *pcpu_stats;
> > @@ -1845,7 +1865,7 @@ static netdev_tx_t team_xmit(struct sk_buff *skb, struct net_device *dev)
> >
> > tx_success = team_queue_override_transmit(team, skb);
> > if (!tx_success)
> > - tx_success = team->ops.transmit(team, skb);
> > + tx_success = smp_load_acquire(&team->ops.transmit)(team, skb);
> > if (tx_success) {
> > struct team_pcpu_stats *pcpu_stats;
> >
> --
> pw-bot: cr
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net v2] net: team: fix NULL pointer dereference in team_xmit during mode change
2026-05-10 16:06 ` Weiming Shi
@ 2026-05-10 16:59 ` Jakub Kicinski
0 siblings, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2026-05-10 16:59 UTC (permalink / raw)
To: Weiming Shi
Cc: Jiri Pirko, Andrew Lunn, David S . Miller, Eric Dumazet,
Paolo Abeni, netdev, Xiang Mei
On Mon, 11 May 2026 00:06:50 +0800 Weiming Shi wrote:
> > Barriers are between things. What are the release / acquire barriers
> > synchronizing.
>
> The handler function pointer against the mode_priv state it operates
> on. On setup, init() writes mode_priv, then smp_store_release()
> publishes the real handler - the paired smp_load_acquire() in the
> reader ensures the handler sees that state. On teardown,
> smp_store_release() publishes the dummy before synchronize_net()
> drains readers, so exit_op() won't tear down state under an
> in-flight reader.
Still does not make sense to me. You already add sync_net().
And if it's possible to switch from dummy to non-dummy mode
the ordering is inverted.
> > Why is AF_PACKET relevant here??
> >
>
> Not specific to the bug, just a reproducer detail. Dropped.
>
> Sending v3 with the updated changelog shortly.
Please don't rush new versions out.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-05-10 16:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-09 18:18 [PATCH net v2] net: team: fix NULL pointer dereference in team_xmit during mode change Weiming Shi
2026-05-10 15:25 ` Jakub Kicinski
2026-05-10 16:06 ` Weiming Shi
2026-05-10 16:59 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox