netdev - Re: [PATCH net-next 3/3] net: usb: kaweth: remove eth_addr_t typedef and bcast

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: <9239ddef-c197-4fbd-8e88-691766bda3e2@suse.com>
Date: Mon, 26 Jan 2026 12:49:35 +0100
From: Oliver Neukum <oneukum@...e.com>
To: Ethan Nelson-Moore <enelsonmoore@...il.com>, netdev@...r.kernel.org,
 linux-usb@...r.kernel.org
Cc: Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next 3/3] net: usb: kaweth: remove eth_addr_t typedef
 and bcast_addr definition

Hi,

unfortunately I must NACK this patch. Let me explain:

On 25.01.26 09:34, Ethan Nelson-Moore wrote:

>   /****************************************************************
>    *     usb_eth_dev
>    ****************************************************************/
> @@ -185,7 +183,7 @@ struct kaweth_ethernet_configuration
>   	__u8 size;
>   	__u8 reserved1;
>   	__u8 reserved2;
> -	eth_addr_t hw_addr;
> +	u8 hw_addr[ETH_ALEN];

You may note that this data structure is declared __packed
and the member hw_addr is preceded by three members of type __u8.
That means that it is located on a _three_ byte alignment.

>   	__u32 statistics_mask;
>   	__le16 segment_size;
>   	__u16 max_multicast_filters;
> @@ -882,7 +880,6 @@ static int kaweth_probe(

[..]

> @@ -991,9 +988,7 @@ static int kaweth_probe(
>   	dev_info(dev, "MTU: %d\n", le16_to_cpu(kaweth->configuration.segment_size));
>   	dev_info(dev, "Read MAC address %pM\n", kaweth->configuration.hw_addr);
>   
> -	if(!memcmp(&kaweth->configuration.hw_addr,
> -                   &bcast_addr,
> -		   sizeof(bcast_addr))) {
> +	if (is_broadcast_ether_addr((const u8 *)&kaweth->configuration.hw_addr)) {

Here you are using is_broadcast_ether_addr(). Let me quote
from the documentation on this helper:

/**
  * is_broadcast_ether_addr - Determine if the Ethernet address is broadcast
  * @addr: Pointer to a six-byte array containing the Ethernet address
  *
  * Return: true if the address is the broadcast address.
  *
  * Please note: addr must be aligned to u16.
  */

That requirement is not met. This code will bomb on ARM for example.

	HTH
		Oliver

NACKED-BY: Oliver Neukum <oneukum@...e.com>