Re: [PATCH net v2] net: validate IPv4 checksum start in skb_csum_hwoffload_help()

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Nov 11, 2026 at 12:04:52PM +0000, Dmitrii Iakunin wrote:
> Commit 6785011f8b16 ("ipv4: validate checksum_start before completing
> checksum") added checks so that malformed CHECKSUM_PARTIAL metadata
> cannot make software checksum completion write the folded checksum
> into the IPv4 header.

... after validation but before ip_do_fragment received?

> skb_csum_hwoffload_help() completes checksums in software when the
> device features cannot satisfy the request (e.g. via validate_xmit_skb(),
> ieee80211_tx_skb_fixup(), openvswitch queue_userspace_packet() or
> ppp_bridge_xmit()). Packets built by AF_PACKET with PACKET_VNET_HDR or
> by tap with VIRTIO_NET_HDR_F_NEEDS_CSUM can reach it with a csum_start
> that points inside the IPv4 header, and this path was not covered by
> the earlier validation.

And so what? it doesn't go to ip_do_fragment. if af_packet
wants to inject malformed packets, it already can.

> 
> At the sw_checksum label, for IPv4 packets, require the fixed IPv4
> header to be pullable and reject csum_start values that fall before the
> end of the IPv4 header. Return -EINVAL so callers drop the packet.
> 
> Unlike the sites added by that commit, this path cannot rely on
> earlier validation, so:
> 
>  - Locate the IPv4 header at the depth returned by __vlan_get_protocol()
>    rather than at network_header. After skb_vlan_untag() pops only the
>    outer tag of a QinQ frame and the skb is redirected by tc mirred or
>    bpf_redirect, network_header still points at the inner VLAN tag, and
>    ihl would be read from the TCI.
> 
>  - Reject ihl < 5. ip_rcv_core() has not checked it on these paths, and
>    with a sender-chosen ihl of 0..4 a csum_start inside the fixed 20-byte
>    header would otherwise pass the check.
> 
>  - Skip the check when the network offset is negative. On the PPP
>    channel-bridge path (L2TP or PPTP), the outer headers have been pulled
>    while network_header and protocol still describe the outer IPv4
>    header. The IPv4 header of the packet being checksummed cannot be
>    located there, and those packets used to be forwarded.
> 
> Hardware-offloaded checksums and SCTP CRC handling are unchanged.
> 
> This change only covers the software fallback in
> skb_csum_hwoffload_help(). Other places that call skb_checksum_help()
> directly are not covered: drivers that fall back to software after the
> core offered hardware offload (e.g. wireguard, igb, tg3), and core
> callers such as xsk tx_sw_csum and netem corruption.

what does this mean?

> As the earlier
> commit notes, validating the metadata on input remains the complete fix.
> 
> Cc: stable@vger.kernel.org
> Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
> Fixes: bfd5f4a3d605 ("packet: Add GSO/csum offload support.")
> Assisted-by: claude-opus-5.5
> Signed-off-by: Dmitrii Iakunin <zeil@nebius.com>
> 
> ---
> v2:
> - Locate the IPv4 header using the parsed VLAN depth.
> - Reject IPv4 headers with ihl < 5.
> - Skip validation when the network offset is negative.
> - Narrow the subject and document software-fallback paths not covered.
> - Update the Fixes tags to identify the original introduction points.
> 
> v1: https://lore.kernel.org/lkml/20261102140633.42564-1-zeil@nebius.com/
> 
>  net/core/dev.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index df1a840b0334..fafcb8f7ff9b 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4111,6 +4111,9 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb,
>  int skb_csum_hwoffload_help(struct sk_buff *skb,
>  			    const netdev_features_t features)
>  {
> +	__be16 protocol;
> +	int offset;
> +
>  	if (unlikely(skb_csum_is_sctp(skb)))
>  		return !!(features & NETIF_F_SCTP_CRC) ? 0 :
>  			skb_crc32c_csum_help(skb);
> @@ -4130,6 +4133,26 @@ int skb_csum_hwoffload_help(struct sk_buff *skb,
>  	}
>  
>  sw_checksum:
> +	offset = skb_network_offset(skb);
> +	protocol = skb->protocol;
> +	if (eth_type_vlan(protocol))
> +		protocol = __vlan_get_protocol(skb, protocol, &offset);
> +
> +	/* Use the parsed VLAN depth: network_header may point at an inner
> +	 * tag. If the network offset is negative, network_header refers
> +	 * to bytes preceding the current packet data and cannot be used
> +	 * to locate the IPv4 header being validated.
> +	 */
> +	if (offset >= 0 && protocol == htons(ETH_P_IP)) {
> +		const struct iphdr *iph;
> +
> +		if (!pskb_may_pull(skb, offset + sizeof(*iph)))
> +			return -EINVAL;
> +		iph = (const struct iphdr *)(skb->data + offset);
> +		if (iph->ihl < 5 ||
> +		    skb_checksum_start_offset(skb) < offset + iph->ihl * 4)
> +			return -EINVAL;
> +	}
>  	return skb_checksum_help(skb);
>  }
>  EXPORT_SYMBOL(skb_csum_hwoffload_help);
> -- 
> 2.52.0
> 
>