[PATCH v3] team: reject CAN and IEEE 802.15.4 devices in team_port_add

[PATCH v3] team: reject CAN and IEEE 802.15.4 devices in team_port_add

[Oliver Hartkopp]

From: Jiale Yao <yaojiale02@163.com>

Enslaving a CAN or IEEE 802.15.4 device (e.g. vxcan, wpan0) to a
team master triggers a NULL pointer dereference because these
device types use different Layer 2 architectures from Ethernet and
the team driver never initializes their private mid-layer data
structures.

Reject ARPHRD_CAN, ARPHRD_IEEE802154, and ARPHRD_IEEE802154_MONITOR
devices in team_port_add(), mirroring the existing CAN check already
present in the bonding driver since commit 8ba68464e478 ("bonding:
refuse to enslave CAN devices").

Link: https://lore.kernel.org/all/b58d98e0-8fe3-4e4e-b8a5-c1c7e647ccca@hartkopp.net/
Fixes: 1d76efe1577b ("team: add support for non-ethernet devices")
Assisted-by: Claude:deepseek-v4-pro
Signed-off-by: Jiale Yao <yaojiale02@163.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
v3: [Hartkopp] fixed indention and compilation as reported by kernel test robot
---
 drivers/net/team/team_core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index ca75f08e3c59..5f1708c2a344 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -1223,10 +1223,20 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
 		netdev_err(dev, "Device %s is loopback device. Loopback devices can't be added as a team port\n",
 			   portname);
 		return -EINVAL;
 	}
 
+	if (port_dev->type == ARPHRD_CAN ||
+	    port_dev->type == ARPHRD_IEEE802154 ||
+	    port_dev->type == ARPHRD_IEEE802154_MONITOR) {
+		NL_SET_ERR_MSG(extack,
+			       "CAN and IEEE 802.15.4 devices can't be added as a team port");
+		netdev_err(dev, "Device %s is CAN or IEEE 802.15.4. These device types can't be added as a team port\n",
+			   portname);
+		return -EINVAL;
+	}
+
 	if (netif_is_team_port(port_dev)) {
 		NL_SET_ERR_MSG(extack, "Device is already a port of a team device");
 		netdev_err(dev, "Device %s is already a port "
 				"of a team device\n", portname);
 		return -EBUSY;
-- 
2.53.0

Re: [PATCH v3] team: reject CAN and IEEE 802.15.4 devices in team_port_add

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH v3] team: reject CAN and IEEE 802.15.4 devices in team_port_add

[Oliver Hartkopp]

The issue was found by code inspection (LLM-assisted, see the
Assisted-by tag), following the syzbot report against the bonding
driver (syzbot+8ed98cbd0161632bce95), which was fixed by commit
8ba68464e478 ("bonding: refuse to enslave CAN devices").

team_setup_by_port() copies port_dev->type into the team device in
the same way, so a team master with a vxcan port ends up as an
ARPHRD_CAN device without can_ml_priv, and runs into the same
NULL pointer dereference (e.g. via isotp_release() ->
can_rx_unregister()).

The same class of problem (non-CAN netdevs claiming ARPHRD_CAN
without CAN ml_priv) was already hit by syzbot in the CAN core and
mitigated there, see 8aa59e355949 ("can: af_can: fix NULL pointer
dereference in can_rx_register()") and 0acc442309a0 ("can: af_can:
fix NULL pointer dereference in can_rcv_filter",
syzbot+2d7f58292cb5b29eb5ad), both with bonding/tun.

The IEEE 802.15.4 types were added by analogy during review.

Best regards,
Oliver

On 13.10.26 10:39, netdev-bot+sinfo@kernel.org wrote:
> Hi!
> 
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
> 
>   - How the issue was discovered, e.g. hit in production, hit during
>     development, syzbot report, manual code inspection, LLM or static
>     analysis tool scan.
> 
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
> 
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

Re: [PATCH v3] team: reject CAN and IEEE 802.15.4 devices in team_port_add

[netdev-bot+sashiko]

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

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

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] The new denylist in team_port_add()
  (drivers/net/team/team_core.c:1228-1236) only rejects ARPHRD_CAN,…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the bonding
  driver, which the commit message cites as the precedent being…

--

team: reject CAN and IEEE 802.15.4 devices in team_port_add

This patch makes team_port_add() reject ports of type ARPHRD_CAN,
ARPHRD_IEEE802154 and ARPHRD_IEEE802154_MONITOR. Team never sets up the
private mid-layer data these L2 types expect, so enslaving one leads to a
NULL pointer dereference. The check mirrors the CAN check bonding has had
since commit 8ba68464e478.

Link: https://lore.kernel.org/all/b58d98e0-8fe3-4e4e-b8a5-c1c7e647ccca@hartkopp.net/

> diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
> index ca75f08e3c593..5f1708c2a3445 100644
> --- a/drivers/net/team/team_core.c
> +++ b/drivers/net/team/team_core.c
> @@ -1225,6 +1225,16 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
>  		return -EINVAL;
>  	}
>  
> +	if (port_dev->type == ARPHRD_CAN ||
> +	    port_dev->type == ARPHRD_IEEE802154 ||
> +	    port_dev->type == ARPHRD_IEEE802154_MONITOR) {

[Severity: High]
Does this list cover the whole class of devices the commit message
describes? ARPHRD_6LOWPAN devices still seem to get through
team_port_add(). Examples are lowpan0 on top of wpan0, or a Bluetooth bt0.

For the first port of a different type, the call chain
team_port_add()->team_dev_type_check_change()->team_setup_by_port() copies
the port's type onto the team device:

drivers/net/team/team_core.c:team_setup_by_port() {
    ...
	dev->type = port_dev->type;
    ...
}

At that point netdev_priv() of the team device is still a struct team. None
of the NETDEV_PRE_TYPE_CHANGE notifiers seem to refuse the change here. The
team device's inet6_dev also survives it.

The global lowpan notifier checks only the type and the inet6_dev before it
writes through lowpan_dev(dev):

net/6lowpan/core.c:lowpan_event() {
	if (dev->type != ARPHRD_6LOWPAN)
		return NOTIFY_DONE;

	idev = __in6_dev_get(dev);
	if (!idev)
		return NOTIFY_DONE;
    ...
	case NETDEV_DOWN:
		for (i = 0; i < LOWPAN_IPHC_CTX_TABLE_SIZE; i++)
			clear_bit(LOWPAN_IPHC_CTX_FLAG_ACTIVE,
				  &lowpan_dev(dev)->ctx.table[i].flags);
    ...
}

When the team device is set down, would these 16 clear_bit() calls write
into struct team? The exact offsets depend on the config. On a typical
x86_64 config without spinlock debugging, they appear to hit:

- notify_peers.dw.work.data (the pending bit)
- notify_peers.dw.timer.flags
- mcast_rejoin.count_pending
- mode_priv[0]

The last write lands at about sizeof(struct team), which is just past the
private area.

On NETDEV_UP and NETDEV_CHANGE, lowpan_is_ll() also reads
lowpan_dev(dev)->lltype. That field overlaps team->pcpu_stats.

Would an allowlist of supported port types be more robust here than a
longer denylist?

[Severity: High]
This is a pre-existing issue, not one introduced by this patch. The commit
message says this change mirrors a bonding check, but that check only
rejects CAN:

drivers/net/bonding/bond_main.c:bond_enslave() {
    ...
	if (slave_dev->type == ARPHRD_CAN) {
		BOND_NL_ERR(bond_dev, extack,
			    "CAN devices cannot be enslaved");
		return -EPERM;
	}
    ...
}

Does bonding have the same IEEE 802.15.4 hole? For a non-Ethernet first
slave, bond_enslave()->bond_setup_by_slave() copies slave_dev->type onto
the bond. A wpan device appears to pass all the remaining checks. The bond
then ends up as ARPHRD_IEEE802154 with a NULL ieee802154_ptr.

ieee802154_get_dev() filters only on the type before it dereferences that
pointer:

net/ieee802154/socket.c:ieee802154_get_dev() {
    ...
		for_each_netdev(net, tmp) {
			if (tmp->type != ARPHRD_IEEE802154)
				continue;

			pan_id = tmp->ieee802154_ptr->pan_id;
    ...
}

This path runs from bind() or connect() with a short address on an
AF_IEEE802154 SOCK_DGRAM socket. ieee802154_create() needs no capability
for SOCK_DGRAM.

Once a bond has a wpan slave, could an unprivileged user oops the kernel
this way? Should bond_enslave() get the same IEEE 802.15.4 checks in a
separate patch?

> +		NL_SET_ERR_MSG(extack,
> +			       "CAN and IEEE 802.15.4 devices can't be added as a team port");

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261013083523.4728-1-socketcan%40hartkopp.net

Re: [PATCH v3] team: reject CAN and IEEE 802.15.4 devices in team_port_add

[Oliver Hartkopp]

On 14.10.26 10:37, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 1 (1) · Medium: 0 · Low: 0
> 
> 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] The new denylist in team_port_add()
>    (drivers/net/team/team_core.c:1228-1236) only rejects ARPHRD_CAN,…
> 
> Pre-existing issues:
> - [High] This problem wasn't introduced by this patch, but the bonding
>    driver, which the commit message cites as the precedent being…
> 
> --
> 
> team: reject CAN and IEEE 802.15.4 devices in team_port_add
> 
> This patch makes team_port_add() reject ports of type ARPHRD_CAN,
> ARPHRD_IEEE802154 and ARPHRD_IEEE802154_MONITOR. Team never sets up the
> private mid-layer data these L2 types expect, so enslaving one leads to a
> NULL pointer dereference. The check mirrors the CAN check bonding has had
> since commit 8ba68464e478.
> 
> Link: https://lore.kernel.org/all/b58d98e0-8fe3-4e4e-b8a5-c1c7e647ccca@hartkopp.net/
> 
>> diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
>> index ca75f08e3c593..5f1708c2a3445 100644
>> --- a/drivers/net/team/team_core.c
>> +++ b/drivers/net/team/team_core.c
>> @@ -1225,6 +1225,16 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
>>   		return -EINVAL;
>>   	}
>>   
>> +	if (port_dev->type == ARPHRD_CAN ||
>> +	    port_dev->type == ARPHRD_IEEE802154 ||
>> +	    port_dev->type == ARPHRD_IEEE802154_MONITOR) {
> 
> [Severity: High]
> Does this list cover the whole class of devices the commit message
> describes? ARPHRD_6LOWPAN devices still seem to get through
> team_port_add(). Examples are lowpan0 on top of wpan0, or a Bluetooth bt0.

I'll send a v4 covering ARPHRD_6LOWPAN too.
> For the first port of a different type, the call chain
> team_port_add()->team_dev_type_check_change()->team_setup_by_port() copies
> the port's type onto the team device:
> 
> drivers/net/team/team_core.c:team_setup_by_port() {
>      ...
> 	dev->type = port_dev->type;
>      ...
> }
> 
> At that point netdev_priv() of the team device is still a struct team. None
> of the NETDEV_PRE_TYPE_CHANGE notifiers seem to refuse the change here. The
> team device's inet6_dev also survives it.
> 
> The global lowpan notifier checks only the type and the inet6_dev before it
> writes through lowpan_dev(dev):
> 
> net/6lowpan/core.c:lowpan_event() {
> 	if (dev->type != ARPHRD_6LOWPAN)
> 		return NOTIFY_DONE;
> 
> 	idev = __in6_dev_get(dev);
> 	if (!idev)
> 		return NOTIFY_DONE;
>      ...
> 	case NETDEV_DOWN:
> 		for (i = 0; i < LOWPAN_IPHC_CTX_TABLE_SIZE; i++)
> 			clear_bit(LOWPAN_IPHC_CTX_FLAG_ACTIVE,
> 				  &lowpan_dev(dev)->ctx.table[i].flags);
>      ...
> }
> 
> When the team device is set down, would these 16 clear_bit() calls write
> into struct team? The exact offsets depend on the config. On a typical
> x86_64 config without spinlock debugging, they appear to hit:
> 
> - notify_peers.dw.work.data (the pending bit)
> - notify_peers.dw.timer.flags
> - mcast_rejoin.count_pending
> - mode_priv[0]
> 
> The last write lands at about sizeof(struct team), which is just past the
> private area.
> 
> On NETDEV_UP and NETDEV_CHANGE, lowpan_is_ll() also reads
> lowpan_dev(dev)->lltype. That field overlaps team->pcpu_stats.
> 
> Would an allowlist of supported port types be more robust here than a
> longer denylist?

No. We had that discussion already on the ML.

team (like bonding) intentionally supports non-Ethernet ports via 
team_setup_by_port() (e.g. IPoIB), so an allowlist would regress 
existing users. The denylist targets the L2 types whose stacks rely on 
type-specific private data.


> [Severity: High]
> This is a pre-existing issue, not one introduced by this patch. The commit
> message says this change mirrors a bonding check, but that check only
> rejects CAN:
> 
> drivers/net/bonding/bond_main.c:bond_enslave() {
>      ...
> 	if (slave_dev->type == ARPHRD_CAN) {
> 		BOND_NL_ERR(bond_dev, extack,
> 			    "CAN devices cannot be enslaved");
> 		return -EPERM;
> 	}
>      ...
> }
> 
> Does bonding have the same IEEE 802.15.4 hole? For a non-Ethernet first
> slave, bond_enslave()->bond_setup_by_slave() copies slave_dev->type onto
> the bond. A wpan device appears to pass all the remaining checks. The bond
> then ends up as ARPHRD_IEEE802154 with a NULL ieee802154_ptr.
> 
> ieee802154_get_dev() filters only on the type before it dereferences that
> pointer:
> 
> net/ieee802154/socket.c:ieee802154_get_dev() {
>      ...
> 		for_each_netdev(net, tmp) {
> 			if (tmp->type != ARPHRD_IEEE802154)
> 				continue;
> 
> 			pan_id = tmp->ieee802154_ptr->pan_id;
>      ...
> }
> 
> This path runs from bind() or connect() with a short address on an
> AF_IEEE802154 SOCK_DGRAM socket. ieee802154_create() needs no capability
> for SOCK_DGRAM.
> 
> Once a bond has a wpan slave, could an unprivileged user oops the kernel
> this way? Should bond_enslave() get the same IEEE 802.15.4 checks in a
> separate patch?

Yes. When this team patch is settled and shows up in the tree, I'll 
create a follow-up for bonding for those 4 exceptions.

Best regards,
Oliver

> 
>> +		NL_SET_ERR_MSG(extack,
>> +			       "CAN and IEEE 802.15.4 devices can't be added as a team port");
> 
> [ ... ]
>