Re: [PATCH net-next v10 2/4] net: dsa: tag_ks8995: Add the KS8995 tag handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Calling ilog2() with a zero value due to unvalidated portmask.
--

commit b59d7eee087d6b0879bef4bef35dc8e9d27f74bd
Author: Linus Walleij <linusw@kernel.org>

net: dsa: tag_ks8995: Add the KS8995 tag handling

This commit adds DSA tagging support for the Micrel KS8995 switch. It pushes
a special modified VLAN tag on transmit and processes it on receive to route
the frame to the appropriate user port.

> diff --git a/net/dsa/tag_ks8995.c b/net/dsa/tag_ks8995.c
> new file mode 100644
> index 0000000000000..9f6fdc1a9a076
> --- /dev/null
> +++ b/net/dsa/tag_ks8995.c
[ ... ]
> +static struct sk_buff *ks8995_rcv(struct sk_buff *skb, struct net_device *dev)
> +{
> +	int portmask;
> +	u16 etype;
[ ... ]
> +	etype = ntohs(*(__be16 *)dsa_etype_header_pos_rx(skb));
> +	if ((etype & KS8995M_STPID_STD) != ETH_P_8021Q) {
> +		netdev_dbg(dev, "%s: dropped ethertype 0x%04x\n",
> +			   __func__, etype);
> +		kfree_skb(skb);
> +		return NULL;
> +	}
[ ... ]
> +	portmask = FIELD_GET(KS8995M_STPID_PORTMASK, etype);
> +	netdev_dbg(dev, "%s: etype %04x portmask %04x (%d)\n",
> +		   __func__, etype, portmask, ilog2(portmask));
> +	skb->dev = dsa_conduit_find_user(dev, 0, ilog2(portmask));

[Severity: Medium]
Could this result in calling ilog2() with a zero value if a standard
802.1Q packet is received?

If the conduit interface receives a standard 802.1Q packet with ethertype
0x8100, the check (etype & KS8995M_STPID_STD) != ETH_P_8021Q passes because
0x8100 & 0xFFF0 is 0x8100.

The code then extracts portmask as 0x8100 & 0x000F, which is 0, and
unconditionally passes it to ilog2() in both netdev_dbg() and
dsa_conduit_find_user().

The ilog2() macro is explicitly documented as having undefined behavior when
passed 0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-ks8995-to-ksz8-v10-0-7379239eb65f@kernel.org?part=2