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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing SKB length validation before inserting VLAN tag causes out-of-bounds read and information leak.
- [Low] Missing explicit include of <linux/bitfield.h> for FIELD_PREP and FIELD_GET macros.
--

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

net: dsa: tag_ks8995: Add the KS8995 tag handling

This commit introduces a DSA tag driver for the Micrel KS8995 switch,
adding support for ingress and egress tagging. The driver modifies an
802.1Q ethertype tag to include the target port mask and rate-limits
untagging errors.

> diff --git a/net/dsa/tag_ks8995.c b/net/dsa/tag_ks8995.c
> new file mode 100644
> index 0000000000000..d08166f0e9ad5
> --- /dev/null
> +++ b/net/dsa/tag_ks8995.c
> @@ -0,0 +1,173 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2026 Linus Walleij <linusw@kernel.org>
> + */
> +#include <linux/etherdevice.h>
> +#include <linux/log2.h>
> +#include <linux/list.h>
> +#include <linux/net.h>
> +#include <linux/slab.h>
> +
> +#include "tag.h"

[ ... ]

> +#define KS8995M_STPID_STD	GENMASK(15, 4)
> +#define KS8995M_STPID_PORTMASK	GENMASK(3, 0)
> +#define KS8995M_STPID(portmask)	htons(ETH_P_8021Q | FIELD_PREP(KS8995M_STPID_PORTMASK, portmask))

[Severity: Low]
Does this file rely on an implicit include for the FIELD_PREP and FIELD_GET
macros?

Including <linux/bitfield.h> directly might prevent potential build failures
on configurations where the implicit include chain differs. The FIELD_GET
macro is also used later in ks8995_rcv().

> +static struct sk_buff *ks8995_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct vlan_ethhdr *hdr;
> +	u16 portmask;
> +
> +	/* Prepare the special KS8995 tags */
> +	portmask = dsa_xmit_port_mask(skb, dev);

[ ... ]

> +	/* Always add a distinct outer tag. The user port removes this field on
> +	 * egress, so reusing an existing 802.1Q tag would consume that VLAN tag.
> +	 */
> +	skb = vlan_insert_tag(skb, KS8995M_STPID(portmask), 0);

[Severity: High]
Could this lead to an out-of-bounds read or write if a short packet is sent?

If a user with CAP_NET_RAW sends a very short packet (e.g., a 1-byte payload)
via an AF_PACKET raw socket, it might reach ndo_start_xmit without being
padded to 14 bytes because needed_tailroom is not set by the tagger.

When ks8995_xmit() calls vlan_insert_tag(), it operates under the assumption
that the MAC header is at least 14 bytes long. The vlan_insert_tag()
function uses memmove() to shift 12 bytes of the MAC header. If the skb
length is less than 12 bytes, this reads uninitialized heap memory into the
MAC header, which could then leak to the network wire.

Additionally, writing the 4-byte VLAN tag at offset 12 could write beyond
the allocated buffer size and corrupt skb_shared_info.

Should this path validate that skb->len is large enough before inserting the
VLAN tag, or ensure short packets are padded first?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-ks8995-to-ksz8-v14-0-3d72692b3c97@kernel.org?part=2