mirror of
https://github.com/torvalds/linux.git
synced 2026-05-31 02:24:24 +02:00
net: team: fix NULL pointer dereference in team_xmit during mode change
__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 a mode change (CAP_NET_ADMIN) concurrent with
transmit on the team device.
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)
The original code assumed that no ports means no traffic, so mode
changes could freely memset()/memcpy() the ops. AF_PACKET with
forced carrier breaks that assumption.
Prevent the race instead of making it safe: replace memset()/memcpy()
with per-field updates that never touch transmit or receive. Those
two handlers are managed solely by team_adjust_ops(), which already
installs dummies when tx_en_port_count == 0 (always true during mode
change since no ports are present). WRITE_ONCE/READ_ONCE prevent
store/load tearing on the handler pointers.
synchronize_net() before exit_op() drains in-flight readers that may
still reference old mode state from before port removal switched the
handlers to dummies.
Fixes: 3d249d4ca7 ("net: introduce ethernet teaming device")
Reported-by: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Link: https://patch.msgid.link/20260521081159.1491563-3-bestswngs@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
parent
f229426072
commit
25fe708bbc
|
|
@ -534,21 +534,23 @@ 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;
|
||||
WRITE_ONCE(team->ops.transmit, team_dummy_transmit);
|
||||
else
|
||||
team->ops.transmit = team->mode->ops->transmit;
|
||||
WRITE_ONCE(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;
|
||||
WRITE_ONCE(team->ops.receive, team_dummy_receive);
|
||||
else
|
||||
team->ops.receive = team->mode->ops->receive;
|
||||
WRITE_ONCE(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 can still reach team_xmit(). Avoid touching
|
||||
* transmit/receive -- they are already set to dummies by
|
||||
* team_adjust_ops() since no ports are enabled. synchronize_net()
|
||||
* drains in-flight readers before destroying old mode state.
|
||||
*/
|
||||
static int __team_change_mode(struct team *team,
|
||||
const struct team_mode *new_mode)
|
||||
|
|
@ -557,9 +559,21 @@ 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);
|
||||
/* Clear cold-path ops used only under RTNL. transmit and
|
||||
* receive are already dummies (no ports) so leave them
|
||||
* alone -- overwriting them is the source of the race.
|
||||
*/
|
||||
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;
|
||||
|
||||
/* Wait for in-flight readers before tearing down mode
|
||||
* state they may reference.
|
||||
*/
|
||||
synchronize_net();
|
||||
|
||||
if (exit_op)
|
||||
exit_op(team);
|
||||
|
|
@ -582,7 +596,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 +762,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 = READ_ONCE(team->ops.receive)(team, port, skb);
|
||||
}
|
||||
if (res == RX_HANDLER_ANOTHER) {
|
||||
struct team_pcpu_stats *pcpu_stats;
|
||||
|
|
@ -1845,7 +1864,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 = READ_ONCE(team->ops.transmit)(team, skb);
|
||||
if (tx_success) {
|
||||
struct team_pcpu_stats *pcpu_stats;
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user