Re: [PATCH v2 6/8] checksum: use csum_ip4_header() in udp.c and tcp.c

[Laurent Vivier <lvivier@redhat.com>]

On 2/16/24 10:08, Stefano Brivio wrote:
> On Wed, 14 Feb 2024 09:56:26 +0100
> Laurent Vivier <lvivier@redhat.com> wrote:
>
>> ...
>>   /**
>>    * udp_update_l2_buf() - Update L2 buffers with Ethernet and IPv4 addresses
>>    * @eth_d:	Ethernet destination address, NULL if unchanged
>> @@ -579,6 +562,9 @@ static void udp_splice_sendfrom(const struct ctx *c, unsigned start, unsigned n,
>>    *
>>    * Return: size of tap frame with headers
>>    */
>> +#pragma GCC diagnostic push
>> +/* ignore unaligned pointer value warning for &b->iph */
>> +#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
>>   static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport,
>>   			      const struct timespec *now)
>>   {
>> @@ -614,13 +600,14 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport,
>>   		b->iph.saddr = b->s_in.sin_addr.s_addr;
>>   	}
>>   
>> -	udp_update_check4(b);
>> +	b->iph.check = csum_ip4_header(&b->iph);
> Similar comment as I had on v1: I don't think this is safe.
>
> If &b->iph is, say, 0x2000, it's all fine: when csum_ip4_header() needs
> to access, say, ip4h->tot_len, it will dereference 0x2000 and look at
> 16 bits, 2 bytes into it.
>
> If &b->iph is 0x2001, though, csum_ip4_header() will dereference 0x2001
> and, on some architectures, boom.

I don't understand how &b->iph cannot be aligned as b should be aligned and b is defined 
using udp4_l2_buf_t structure with _attribute__ ((packed, aligned(__alignof__(unsigned 
int)))).

Thanks,
Laurent