Re: [PATCH v2 net-next 08/15] net: enetc: move mac_filter from struct enetc_pf to struct enetc_si

Re: [PATCH v2 net-next 08/15] net: enetc: move mac_filter from struct enetc_pf to struct enetc_si

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Silent 32-bit truncation of 64-bit MAC hash table bitmap
--

commit d6f9f9bacee81eb61d86d4e3d383dcddb4c66cbb
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: move mac_filter from struct enetc_pf to struct enetc_si

The mac_filter array currently resides in struct enetc_pf and is used to
track unicast and multicast MAC address filters for the PF. Since struct
enetc_si is the common structure shared between the PF and VF drivers,
move mac_filter into struct enetc_si to prepare for MAC filter support
in the VF driver.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> @@ -225,11 +225,12 @@ static void enetc4_pf_set_mac_hash_filter(struct enetc_pf *pf, int type)
>  	struct net_device *ndev = pf->si->ndev;
>  	struct enetc_mac_filter *mac_filter;
>  	struct enetc_hw *hw = &pf->si->hw;
> +	struct enetc_si *si = pf->si;
>  	struct netdev_hw_addr *ha;
>  
>  	netif_addr_lock_bh(ndev);
>  	if (type & ENETC_MAC_FILTER_TYPE_UC) {
> -		mac_filter = &pf->mac_filter[UC];
> +		mac_filter = &si->mac_filter[UC];
>  		enetc_reset_mac_addr_filter(mac_filter);
>  		netdev_for_each_uc_addr(ha, ndev)
>  			enetc_add_mac_addr_ht_filter(mac_filter, ha->addr);

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to dereference
the mac_hash_table bitmap directly?

The mac_hash_table is declared as a 64-bit bitmap. On 32-bit architectures,
an unsigned long is 32 bits, making the bitmap an array of two unsigned
longs.

When the code dereferences this bitmap in enetc4_pf_set_mac_hash_filter():

    enetc4_pf_set_si_uc_hash_filter(hw, 0, *mac_filter->mac_hash_table);

It reads only the first unsigned long, capturing just the lower 32 bits.
When this is implicitly promoted to u64 for enetc4_pf_set_si_uc_hash_filter(),
the upper 32 bits will always be zero.

This same truncation also happens in enetc4_pf_set_uc_exact_filter() and
in enetc_sync_mac_filters() when it passes *f->mac_hash_table to
enetc_set_mac_ht_flt().

Furthermore, the enetc_set_mac_ht_flt() function itself takes an unsigned
long:

    static void enetc_set_mac_ht_flt(struct enetc_si *si, int si_idx, int type,
                                     unsigned long hash)

Because hash is truncated to 32 bits on 32-bit systems, upper_32_bits(hash)
will always evaluate to 0, leaving the upper MAC hash hardware registers
unprogrammed.

Could this lead to incorrect multicast and unicast MAC filtering on 32-bit
systems?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610091844.3423693-1-wei.fang@oss.nxp.com?part=8