Re: [PATCH net v6] net: use skb_header_pointer() for TCPv4 GSO frag_off

[Willem de Bruijn]

Guoyu Su wrote:
> Syzbot reported a KMSAN uninit-value warning in gso_features_check()
> called from netif_skb_features() [1].
> 
> gso_features_check() reads iph->frag_off to decide whether to clear
> mangleid_features. Accessing the IPv4 header via ip_hdr()/inner_ip_hdr()
> can rely on skb header offsets that are not always safe for direct
> dereference on packets injected from PF_PACKET paths.
> 
> Use skb_header_pointer() for the TCPv4 frag_off check so the header read
> is robust whether data is already linear or needs copying.
> 
> This also removes the SKB_GSO_DODGY special casing: skb_header_pointer()
> already fast-paths linear data, so a separate direct-access path is not
> needed.

Does not belong in the commit message. It's a diff vs the previous
version.
 
> [1] https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407
> 
> Link: https://lore.kernel.org/netdev/willemdebruijn.kernel.1a9f35039caab@xxxxxxxxx/
> Fixes: cbc53e08a793 ("GSO: Add GSO type for fixed IPv4 ID")
> Reported-by: syzbot+1543a7d954d9c6d00407@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407
> Tested-by: syzbot+1543a7d954d9c6d00407@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Guoyu Su <yss2813483011xxl@xxxxxxxxx>
> ---
> v6:
>  - Use skb_header_pointer() for both DODGY and non-DODGY TCPv4 GSO
>    packets in gso_features_check().
>  - Drop the SKB_GSO_DODGY special-casing for IPv4 header access.
> 
> v5: https://lore.kernel.org/netdev/20260320141459.9691-1-yss2813483011xxl@xxxxxxxxx/
> v4: https://lore.kernel.org/netdev/20260319005421.14908-1-yss2813483011xxl@xxxxxxxxx/
> v3: https://lore.kernel.org/netdev/20260312104351.185370-1-yss2813483011xxl@xxxxxxxxx/
> v2: https://lore.kernel.org/netdev/20260308083319.1255118-1-yss2813483011xxl@xxxxxxxxx/
> v1: https://lore.kernel.org/netdev/20260307162905.3697050-1-yss2813483011xxl@xxxxxxxxx/
> 
>  net/core/dev.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 14a83f2035b9..8a15ca67cfed 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3805,10 +3805,16 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb,
>  	 * segmentation-offloads.rst).
>  	 */
>  	if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) {
> -		struct iphdr *iph = skb->encapsulation ?
> -				    inner_ip_hdr(skb) : ip_hdr(skb);
> +		const struct iphdr *iph;
> +		struct iphdr _iph;
>  
> -		if (!(iph->frag_off & htons(IP_DF)))

minor: no whitespace in the middle of the variable definition block.
> +		int nhoff = skb->encapsulation ?
> +			    skb_inner_network_offset(skb) :
> +			    skb_network_offset(skb);
> +
> +		iph = skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph);
> +
> +		if (!iph || !(iph->frag_off & htons(IP_DF)))
>  			features &= ~dev->mangleid_features;
>  	}
>  
> -- 
> 2.34.1
>