netdev - Re: ether_addr_equal_64bits breakage with gcc-12

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: <20220212090811.uuzk6d76agw2vv73@pengutronix.de>
Date:   Sat, 12 Feb 2022 10:08:11 +0100
From:   Marc Kleine-Budde <mkl@...gutronix.de>
To:     Kees Cook <keescook@...omium.org>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: ether_addr_equal_64bits breakage with gcc-12

On 11.02.2022 17:31:22, Kees Cook wrote:
> > Maybe Kees will have as suggestion - Kees, are there any best practices
> > for dealing with such issues? For the reference we do a oversized load
> > from a structure (read 8B of a 6B array):
> 
> Wheee.
> 
> So, the short theoretical "don't do that" scenario would be "what
> happens if":
> 
>         struct page *page;
>         void *ptr;
> 	unsigned char *eth_addr;
> 
>         page = alloc_pages(GFP_KERNEL, 0);
> 	...
>         ptr = page_address(page);
> 	...
> 	/* "eth_addr" at end of allocated memory */
> 	eth_addr = ptr + PAGE_SIZE - 6;
> 	/* access fault... */
> 	ether_addr_equal_64bits(eth_addr, ...);
> 
> But, yes, pragmatically, this is likely extremely rare.
> 
> Regardless, with the other cases like this that got fixed like this, it
> was a matter of finding a way to represent the "actual" available memory
> (best), or telling the compiler what real contract is (less good).
> 
> It looks like alignment isn't a concern, so I'd say adjust the prototype
> to reflect the reality, and go with:
> 
> 
> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> index 2ad71cc90b37..92b10e67d5f8 100644
> --- a/include/linux/etherdevice.h
> +++ b/include/linux/etherdevice.h
> @@ -134,7 +134,7 @@ static inline bool is_multicast_ether_addr(const u8 *addr)
>  #endif
>  }
>  
> -static inline bool is_multicast_ether_addr_64bits(const u8 addr[6+2])
> +static inline bool is_multicast_ether_addr_64bits(const u8 *addr)
>  {
>  #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
>  #ifdef __BIG_ENDIAN
> @@ -372,8 +372,7 @@ static inline bool ether_addr_equal(const u8 *addr1, const u8 *addr2)
>   * Please note that alignment of addr1 & addr2 are only guaranteed to be 16 bits.
>   */
>  
> -static inline bool ether_addr_equal_64bits(const u8 addr1[6+2],
> -					   const u8 addr2[6+2])
> +static inline bool ether_addr_equal_64bits(const u8 *addr1, const u8 *addr2)
>  {
>  #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
>  	u64 fold = (*(const u64 *)addr1) ^ (*(const u64 *)addr2);
> 

With that patch the warning is gone.

Tested-by: Marc Kleine-Budde <mkl@...gutronix.de>

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)