Re: [PATCH net-next v2 1/9] net: dsa: add tag driver for LAN9645X
flat view
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-03-29 19:56:25
Also in:
linux-devicetree, lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. --- net: dsa: add tag driver for LAN9645X Add a tag driver for LAN9645x switches using a front port as the CPU port (NPI port). The driver configures a long prefix for extraction and no prefix for injection to handle the internal frame header (IFH).
quoted hunk ↗ jump to hunk
diff --git a/net/dsa/tag_lan9645x.c b/net/dsa/tag_lan9645x.c new file mode 100644 index 000000000000..9130fb0d61be --- /dev/null +++ b/net/dsa/tag_lan9645x.c
[ ... ]
quoted hunk ↗ jump to hunk
+static void lan9645x_xmit_get_vlan_info(struct sk_buff *skb, + struct net_device *br, + u32 *vlan_tci, u32 *tag_type) +{ + struct vlan_ethhdr *hdr; + u16 proto, tci; + + if (!br || !br_vlan_enabled(br)) { + *vlan_tci = 0; + *tag_type = LAN9645X_IFH_TAG_TYPE_C; + return; + } + + hdr = (struct vlan_ethhdr *)skb_mac_header(skb); + br_vlan_get_proto(br, &proto); + + if (ntohs(hdr->h_vlan_proto) == proto) { + vlan_remove_tag(skb, &tci);
Is it possible for a maliciously crafted packet (e.g., via AF_PACKET) to have a length of 14 bytes with h_vlan_proto set to ETH_P_8021Q? If so, vlan_remove_tag() would read vhdr->h_vlan_TCI at offset 14, causing an out-of-bounds read. Furthermore, if the packet contains fragments and the VLAN header is not in the linear data area, vlan_remove_tag() could corrupt the SKB when shifting the MAC header. Would it be safer to use skb_vlan_untag() instead?
quoted hunk ↗ jump to hunk
+ *vlan_tci = tci; + } else { + rcu_read_lock(); + br_vlan_get_pvid_rcu(br, &tci); + rcu_read_unlock(); + *vlan_tci = tci; + } + + *tag_type = (proto != ETH_P_8021Q) ? LAN9645X_IFH_TAG_TYPE_S : + LAN9645X_IFH_TAG_TYPE_C; +}
[ ... ]
quoted hunk ↗ jump to hunk
+static struct sk_buff *lan9645x_rcv(struct sk_buff *skb, + struct net_device *ndev) +{ + u32 src_port, qos_class, vlan_tci, tag_type, popcnt, etype_ofs; + u8 *orig_skb_data = skb->data; + struct dsa_port *dp; + u32 ifh_gap_len = 0; + u16 vlan_tpid; + u8 *ifh; + + /* DSA master already consumed DMAC,SMAC,ETYPE from long prefix. Go back + * to beginning of frame. + */ + skb_push(skb, ETH_HLEN); + /* IFH starts after our long prefix */ + ifh = skb_pull(skb, LAN9645X_LONG_PREFIX_LEN);
Could skb_pull() return NULL here if skb->len is less than 16 bytes? If it returns NULL, the blind dereference of ifh in the subsequent lan9645x_ifh_get() calls would cause a NULL pointer dereference. Additionally, is the 44-byte tag (16-byte prefix + 28-byte IFH) guaranteed to be in the linear data buffer (skb->data)? Without calling pskb_may_pull(skb, LAN9645X_TOTAL_TAG_LEN) first, accessing it via the ifh pointer might result in out-of-bounds reads if the packet payload is in paged fragments.
quoted hunk ↗ jump to hunk
+ + popcnt = lan9645x_ifh_get(ifh, IFH_POP_CNT, IFH_POP_CNT_SZ); + etype_ofs = lan9645x_ifh_get(ifh, IFH_ETYPE_OFS, IFH_ETYPE_OFS_SZ); + src_port = lan9645x_ifh_get(ifh, IFH_SRCPORT, IFH_SRCPORT_SZ);
-- pw-bot: cr