Re: [PATCH v2 net-next 09/15] net: enetc: add MAC address filtering support for VFs of ENETC v4

Re: [PATCH v2 net-next 09/15] net: enetc: add MAC address filtering support for VFs of ENETC v4

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing message length validation causes an out-of-bounds heap read when an untrusted VF configures multicast MAC hash filters.
- [Medium] Revoking a VF's trusted status fails to clear its multicast hash filter, allowing it to retain permissive multicast filtering.
--

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

net: enetc: add MAC address filtering support for VFs of ENETC v4

This commit adds VSI-to-PSI mailbox message support to allow VFs to request 
MAC filter configurations from the PF, including hash tables and promiscuous 
modes. It updates message handling, trust checks, and hardware register 
access to enforce permissions securely.

> --- a/drivers/net/ethernet/freescale/enetc/enetc_msg.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
[ ... ]
> @@ -111,6 +113,139 @@ static u16 enetc_msg_set_vf_primary_mac_addr(struct enetc_pf *pf, int vf_id,
[ ... ]
> +static u16 enetc_msg_set_vf_mac_hash_filter(struct enetc_pf *pf, int vf_id,
> +					    void *vf_msg)
> +{
[ ... ]
> +	switch (type) {
> +	case ENETC_MAC_FILTER_TYPE_UC:
> +		if (!trusted) {
> +			pf_msg = ENETC_PF_MSG_PERM_DENY;
> +			goto vf_state_unlock;
> +		}
> +
> +		uc_hash = (u64)msg->hash_tbl[1] << 32 | msg->hash_tbl[0];
> +		pf->ops->set_si_mac_hash_filter(hw, si_id, UC, uc_hash);
> +		break;
> +	case ENETC_MAC_FILTER_TYPE_MC:
> +		mc_hash = (u64)msg->hash_tbl[3] << 32 | msg->hash_tbl[2];
> +		if (!trusted && hweight64(mc_hash) > 8) {

[Severity: High]
Does this path miss a message length validation?

If msg->hdr.len == 0, the allocated buffer size is 32 bytes (via
ENETC_MSG_SIZE(0)). The hash_tbl array starts at offset 20. Reading
msg->hash_tbl[3] accesses bytes 32-35, which is an out-of-bounds heap read
past the 32-byte allocation.

The ENETC_MAC_FILTER_TYPE_ALL case explicitly checks !msg->hdr.len
before accessing hash_tbl[3]. Should the ENETC_MAC_FILTER_TYPE_MC case
have the same check to prevent untrusted VFs from triggering KASAN faults
or leaking adjacent heap memory?

> +			pf_msg = ENETC_PF_MSG_PERM_DENY;
> +			goto vf_state_unlock;
> +		}
> +
> +		pf->ops->set_si_mac_hash_filter(hw, si_id, MC, mc_hash);
> +		break;
[ ... ]
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
[ ... ]
> @@ -476,11 +477,25 @@ int enetc_pf_set_vf_trust(struct net_device *ndev, int vf, bool setting)
>  	vf_state = &pf->vf_state[vf];
>  	mutex_lock(&vf_state->lock);
>  
> -	if (setting)
> +	if (setting) {
>  		vf_state->flags |= ENETC_VF_FLAG_TRUSTED;
> -	else
> +	} else {
>  		vf_state->flags &= ~ENETC_VF_FLAG_TRUSTED;
>  
> +		/* Clear unicast hash filter and disable MAC promiscuous modes
> +		 * if the VF is untrusted.
> +		 */
> +		if (pf->ops->set_si_mac_hash_filter)
> +			pf->ops->set_si_mac_hash_filter(hw, vf + 1, UC, 0);

[Severity: Medium]
Should this trust revocation path also clear the multicast hash filter?

Untrusted VFs are strictly restricted to an 8-bit multicast hash filter
(hweight64(mc_hash) <= 8), while trusted VFs can set a permissive 64-bit
filter.

When ndo_set_vf_trust() clears the trusted flag, the unicast filter is
correctly zeroed, but the hardware's multicast filter is left intact.
Doesn't this allow a newly untrusted VF to retain its previously configured
permissive 64-bit multicast filter, bypassing the intended security policy?

> +
> +		mutex_lock(&pf->msg_lock);
> +		if (pf->ops->set_si_mac_promisc) {
> +			pf->ops->set_si_mac_promisc(hw, vf + 1, UC, false);
> +			pf->ops->set_si_mac_promisc(hw, vf + 1, MC, false);
> +		}
> +		mutex_unlock(&pf->msg_lock);
> +	}
> +
>  	mutex_unlock(&vf_state->lock);
>  
>  	return 0;

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

RE: [PATCH v2 net-next 09/15] net: enetc: add MAC address filtering support for VFs of ENETC v4

[Wei Fang (OSS)]

> > @@ -111,6 +113,139 @@ static u16
> enetc_msg_set_vf_primary_mac_addr(struct enetc_pf *pf, int vf_id,
> [ ... ]
> > +static u16 enetc_msg_set_vf_mac_hash_filter(struct enetc_pf *pf, int vf_id,
> > +					    void *vf_msg)
> > +{
> [ ... ]
> > +	switch (type) {
> > +	case ENETC_MAC_FILTER_TYPE_UC:
> > +		if (!trusted) {
> > +			pf_msg = ENETC_PF_MSG_PERM_DENY;
> > +			goto vf_state_unlock;
> > +		}
> > +
> > +		uc_hash = (u64)msg->hash_tbl[1] << 32 | msg->hash_tbl[0];
> > +		pf->ops->set_si_mac_hash_filter(hw, si_id, UC, uc_hash);
> > +		break;
> > +	case ENETC_MAC_FILTER_TYPE_MC:
> > +		mc_hash = (u64)msg->hash_tbl[3] << 32 | msg->hash_tbl[2];
> > +		if (!trusted && hweight64(mc_hash) > 8) {
> 
> [Severity: High]
> Does this path miss a message length validation?
> 
> If msg->hdr.len == 0, the allocated buffer size is 32 bytes (via
> ENETC_MSG_SIZE(0)). The hash_tbl array starts at offset 20. Reading
> msg->hash_tbl[3] accesses bytes 32-35, which is an out-of-bounds heap read
> past the 32-byte allocation.
> 
> The ENETC_MAC_FILTER_TYPE_ALL case explicitly checks !msg->hdr.len
> before accessing hash_tbl[3]. Should the ENETC_MAC_FILTER_TYPE_MC case
> have the same check to prevent untrusted VFs from triggering KASAN faults
> or leaking adjacent heap memory?

This is a bug introduced during the refactoring of
enetc_msg_set_vf_mac_hash_filter(). It will be fixed in next version.