Re: [PATCH net-next v19 04/13] ax88179_178a: Add HW support for AX179A-based chips

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] net: ax88179a: perm_addr overwritten by dev_addr on interface up
- [Low] net/usb: unaligned memory access via cast of u8 buf[5]
--

commit 8f7d8eac10e2d0c51938d5773370c574a2391e9a
Author: Birger Koblitz <mail@birger-koblitz.de>

ax88179_178a: Add HW support for AX179A-based chips

This commit adds bindings and hardware support for AX179A-based USB-Ethernet
controllers. It introduces a vendor mode switch for CDC-NCM fallback, adds
new device entries, and provides hardware-specific initialization sequences
via ax88179a_devices.c.

> diff --git a/drivers/net/usb/ax88179a_devices.c b/drivers/net/usb/ax88179a_devices.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/net/usb/ax88179a_devices.c
[ ... ]
> +static int ax88179a_reset(struct usbnet *dev)
> +{
> +	struct ax88179_data *ax179_data = dev->driver_priv;
> +	u16 *tmp16;
> +	u8 buf[5];
> +	u16 rxctl;
> +	u8 *tmp;
> +
> +	tmp16 = (u16 *)buf;
> +	tmp = (u8 *)buf;
[ ... ]
> +	ax88179_write_u8(dev, AX_ACCESS_MAC, AX88179A_MAC_ARC_CTRL, 1, 0);
> +	ax88179_write_u8(dev, AX_ACCESS_MAC, AX88179A_MAC_SWP_CTRL, 1, 0);
> +	ax88179_write_u8(dev, AX_ACCESS_MAC, AX88179A_MAC_TX_HDR_CKSUM, 1, 0);
> +
> +	/* Read MAC address from DTB or asix chip */
> +	ax88179_get_mac_addr(dev);
> +	memcpy(dev->net->perm_addr, dev->net->dev_addr, ETH_ALEN);

[Severity: Medium]
Is it safe to unconditionally overwrite the permanent MAC address during an
interface reset?

During routine interface resets (e.g., bringing the interface up via
usbnet_open()), the usbnet framework calls the driver's .reset callback,
which points to ax88179a_reset().

If a user changes the MAC address while the interface is down, that custom
address is correctly written to the hardware. However, when the interface
comes back up, ax88179_get_mac_addr() reads that temporary custom MAC back
from the hardware and copies it into dev->net->perm_addr.

Does this permanently clobber the immutable perm_addr with the user's
temporary MAC address and break tools like ethtool -P?

[ ... ]
> +	if (ax179_data->chip_version < AX_VERSION_AX88179A)
> +		*tmp = AX_MONITOR_MODE_PMETYPE | AX_MONITOR_MODE_PMEPOL | AX_MONITOR_MODE_RWMP;
> +	else
> +		*tmp = AX_MONITOR_MODE_RWMP;
> +	ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_MONITOR_MOD, 1, 1, tmp);
> +
> +	/* Configure default medium type => giga */
> +	*tmp16 = AX_MEDIUM_RECEIVE_EN | AX_MEDIUM_TXFLOW_CTRLEN |
> +		 AX_MEDIUM_RXFLOW_CTRLEN | AX_MEDIUM_FULL_DUPLEX;
> +	if (!ax179_data->is_ax88772d)
> +		*tmp16 |= AX_MEDIUM_GIGAMODE;
> +	ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE, 2, 2, tmp16);

[Severity: Low]
Does assigning to *tmp16 cause an unaligned memory access here?

The stack buffer u8 buf[5] has a natural alignment of 1 byte. Casting it to
u16 *tmp16 and directly assigning a 16-bit value violates C standard
aliasing and alignment rules. On architectures lacking hardware support for
unaligned accesses, this causes alignment faults.

Since this patch introduces the ax88179_write_u16() helper, could that be
used instead to avoid the unaligned cast entirely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010-ax88179a-v19-0-7475173cfb56@birger-koblitz.de?part=4