Re: git: e0d8add4af0b - main - tcp_lro: Fix for undefined behaviour.

On 1/13/23 2:19 AM, Hans Petter Selasky wrote:
> The branch main has been updated by hselasky:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=e0d8add4af0be1d37ede9a16f46424dc08f0d95e
> 
> commit e0d8add4af0be1d37ede9a16f46424dc08f0d95e
> Author:     Hans Petter Selasky <hselasky@FreeBSD.org>
> AuthorDate: 2022-11-28 22:56:16 +0000
> Commit:     Hans Petter Selasky <hselasky@FreeBSD.org>
> CommitDate: 2023-01-13 10:18:19 +0000
> 
>      tcp_lro: Fix for undefined behaviour.
>      
>      Make sure the size of the raw[] array in the lro_address union is
>      correctly set at compile time, so that static code analysis tools
>      do not report undefined behaviour.
>      
>      MFC after:      1 week
>      Sponsored by:   NVIDIA Networking
> ---
>   sys/netinet/tcp_lro.h | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/sys/netinet/tcp_lro.h b/sys/netinet/tcp_lro.h
> index 427657df47e7..e01e451c1e77 100644
> --- a/sys/netinet/tcp_lro.h
> +++ b/sys/netinet/tcp_lro.h
> @@ -34,6 +34,8 @@
>   #define _TCP_LRO_H_
>   
>   #include <sys/time.h>
> +#include <sys/param.h>

param.h should always be the first header, and it also sorts before time.h.

> +
>   #include <netinet/in.h>
>   
>   #ifndef TCP_LRO_ENTRIES
> @@ -65,8 +67,12 @@
>   
>   struct inpcb;
>   
> +/* Precompute the LRO_RAW_ADDRESS_MAX value: */
> +#define	LRO_RAW_ADDRESS_MAX \
> +	howmany(12 + 2 * sizeof(struct in6_addr), sizeof(u_long))
> +
>   union lro_address {
> -	u_long raw[1];
> +	u_long raw[LRO_RAW_ADDRESS_MAX];

In OFED this anti-pattern uses 'u_long raw[0]' which I think is probably at
least better than '[1]'.  My worry is that LRO_RAW_ADDRESS_MAX is fairly
fragile and gross, though the static assertion at least catches if it is
wrong.

In general though for both OFED and this, I think is is actually cleaner to
remove the weird aliasing 'raw' member and just use a plain (u_long *)
cast.  If you did that, you could revert this commit and have the simpler
definition of LRO_RAW_ADDRESS_MAX derived from the sizeof() the type.

-- 
John Baldwin