This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at:
https://sashiko.dev/#/patchset/[email protected]?part=1
---
igc: Support ACPI-based MAC passthrough
This commit implements support for seamlessly passing through a system MAC
address provided in the ACPI table to an I225/I226 network controller
attached via a Thunderbolt dock.
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> b/drivers/net/ethernet/intel/igc/igc_main.c
> index 1fb5f3cbe93c5..abffd385f3e8c 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
[ ... ]
> @@ -7106,6 +7108,58 @@ static enum hrtimer_restart
> igc_qbv_scheduling_timer(struct hrtimer *timer)
> return HRTIMER_NORESTART;
> }
>
> +static bool igc_get_acpi_mac_passthru(u8 *mac)
> +{
[ ... ]
> + acpi_status status;
> + u8 buf[ETH_ALEN];
[Severity: High]
Does this array lack a 16-bit alignment guarantee?
Because it is declared as a simple byte array on the stack, the compiler is
free to place it at an odd memory address.
> + int i;
> +
> + if (!IS_ENABLED(CONFIG_ACPI))
> + return false;
> +
> + for (i = 0; i < ARRAY_SIZE(sources) && !mac_found; i++) {
[ ... ]
> + if (hex2bin(buf, obj->string.pointer + 9, ETH_ALEN) ||
> + !is_valid_ether_addr(buf))
> + goto free_obj;
> +
> + ether_addr_copy(mac, buf);
[Severity: High]
Can passing this buffer to is_valid_ether_addr() and ether_addr_copy() cause
an alignment trap on architectures that strictly enforce memory alignment?
Both of these helpers cast the pointer to a u16 pointer to perform 16-bit
memory operations when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is not set.
If buf ends up placed at an odd address, this could cause a kernel panic.
Could this be avoided by defining buf as a u16 array or by adding an explicit
__aligned(2) attribute?
> + mac_found = true;
> +free_obj:
> + ACPI_FREE(obj);
> + }
> +
> + return mac_found;
> +}
> +
