linux-kernel - Re: [PATCH net-next v1 1/1] net: selftests: add PHY-loopback test for bad TCP checksums

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250709194039.72202043@kernel.org>
Date: Wed, 9 Jul 2025 19:40:39 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Simon Horman
 <horms@...nel.org>, kernel@...gutronix.de, linux-kernel@...r.kernel.org,
 netdev@...r.kernel.org, Maxime Chevallier <maxime.chevallier@...tlin.com>
Subject: Re: [PATCH net-next v1 1/1] net: selftests: add PHY-loopback test
 for bad TCP checksums

On Tue,  8 Jul 2025 14:28:23 +0200 Oleksij Rempel wrote:
>  	if (attr->tcp) {
>  		int l4len = skb->len - skb_transport_offset(skb);
>  
> -		thdr->check = ~tcp_v4_check(l4len, ihdr->saddr, ihdr->daddr, 0);
> -		skb->csum_start = skb_transport_header(skb) - skb->head;
> -		skb->csum_offset = offsetof(struct tcphdr, check);
> +		if (attr->bad_csum) {
> +			__sum16 good_csum;
> +			u16 bad_csum;
> +
> +			skb->ip_summed = CHECKSUM_NONE;
> +			thdr->check = 0;
> +			skb->csum = skb_checksum(skb, skb_transport_offset(skb),
> +						 l4len, 0);
> +			good_csum = csum_tcpudp_magic(ihdr->saddr, ihdr->daddr,
> +						      l4len, IPPROTO_TCP,
> +						      skb->csum);
> +
> +			/* Flip the least-significant bit.  This is fast,
> +			 * deterministic, and cannot accidentally turn the
> +			 * checksum back into a value the stack treats as valid
> +			 * (0 or 0xFFFF).
> +			 */
> +			bad_csum = (__force u16)good_csum ^ 0x0001;
> +			if (bad_csum == 0 || bad_csum == 0xFFFF) {
> +				/* If the checksum is 0 or 0xFFFF, flip another
> +				 * bit to ensure it is not valid.
> +				 */
> +				bad_csum ^= 0x0002;
> +			}
> +
> +			thdr->check = (__force __sum16)bad_csum;
> +		} else {
> +			skb->csum = 0;
> +			skb->ip_summed = CHECKSUM_PARTIAL;
> +			thdr->check = ~tcp_v4_check(l4len, ihdr->saddr,
> +						    ihdr->daddr, 0);
> +			skb->csum_start = skb_transport_header(skb) - skb->head;
> +			skb->csum_offset = offsetof(struct tcphdr, check);
> +		}
>  	} else {
> +		skb->csum = 0;
> +		skb->ip_summed = CHECKSUM_PARTIAL;
>  		udp4_hwcsum(skb, ihdr->saddr, ihdr->daddr);
>  	}

I think it'd be simpler if - after setting up CHECKSUM_PARTIAL
we called skb_checksum_help() to get the correct checksum filled in
and then did the bad checksum mangling.

BTW mangling like this should be idiomatic enough to avoid the comment:

	thdr->check = thdr->check ^ 1 ?: CSUM_MANGLED_0;
-- 
pw-bot: cr