Re: [PATCH v4 net-next 13/15] net: enetc: implement ndo_set_rx_mode_async for ENETC v4 VF

[sashiko-bot@kernel.org]

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

New issues:
- [High] Denied promiscuous mode requests for untrusted VFs silently ignore explicit unicast/multicast hash filters, breaking network connectivity.
- [Medium] Unprivileged untrusted VF users can cause kernel log spam on the VF because `dev_err` is unconditionally called upon expected permission denials.

Pre-existing issues:
- [High] Out-of-bounds heap read in the PF driver when parsing mailbox messages with small lengths.
--

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

net: enetc: implement ndo_set_rx_mode_async for ENETC v4 VF

This commit implements ndo_set_rx_mode_async for ENETC v4 VFs to handle
MAC filter changes asynchronously. It adds helpers to send promiscuous
mode and MAC hash filter messages to the PF over the VSI mailbox.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_vf.c b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> index 322705202d49b..8cf38c426dae3 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
[ ... ]
> @@ -213,6 +213,193 @@ static int enetc_vf_setup_tc(struct net_device *ndev, enum tc_setup_type type,
>  	}
>  }
>  
> +static int enetc_vf_set_mac_promisc(struct enetc_si *si, int type,
> +				    bool promisc, bool flush_macs)
> +{
[ ... ]
> +static int enetc_vf_enable_iff_promisc(struct enetc_si *si)
> +{
> +	int err;
> +
> +	err = enetc_vf_set_mac_promisc(si, ENETC_MAC_FILTER_TYPE_ALL,
> +				       true, true);
> +	if (err)
> +		dev_err(&si->pdev->dev,
> +			"Failed to enable promiscuous mode, err: %pe\n",
> +			ERR_PTR(err));

[Severity: Medium]
When expected permission denials occur, does the unconditional use of
dev_err() override the intended silent failure path in the underlying
message sender, causing log spam?

If an admin has set IFF_PROMISC or IFF_ALLMULTI, any subsequent
unprivileged user action that updates multicast lists (e.g.
IP_ADD_MEMBERSHIP) triggers a netdev rx mode sync, resulting in a -EACCES
and a log print. Could this be used by local unprivileged users to cause
a local DoS or log spam on the VF?

> +
> +	return err;
> +}
> +
> +static int enetc_vf_disable_iff_promisc(struct enetc_si *si,
> +					struct netdev_hw_addr_list *uc,
> +					struct netdev_hw_addr_list *mc)
> +{
[ ... ]
> +static int enetc_vf_enable_iff_allmulti(struct enetc_si *si,
> +					struct netdev_hw_addr_list *uc)
> +{
> +	int err;
> +
> +	err = enetc_vf_set_mac_promisc(si, ENETC_MAC_FILTER_TYPE_MC,
> +				       true, true);
> +	if (err) {
> +		dev_err(&si->pdev->dev,
> +			"Failed to enable multicast promiscuous mode, err: %pe\n",
> +			ERR_PTR(err));

[Severity: Medium]
Will this also cause log spam when multicast promiscuous mode requests are
predictably denied for the same reasons as above?

> +		return err;
> +	}
> +
[ ... ]
> +static int enetc_vf_set_rx_mode(struct net_device *ndev,
> +				struct netdev_hw_addr_list *uc,
> +				struct netdev_hw_addr_list *mc)
> +{
> +	struct enetc_ndev_priv *priv = netdev_priv(ndev);
> +	struct enetc_si *si = priv->si;
> +	int err;
> +
> +	if (ndev->flags & IFF_PROMISC)
> +		err = enetc_vf_enable_iff_promisc(si);
> +	else if (ndev->flags & IFF_ALLMULTI)
> +		err = enetc_vf_enable_iff_allmulti(si, uc);
> +	else
> +		err = enetc_vf_disable_iff_promisc(si, uc, mc);
> +
> +	/* If the error code is -EOPNOTSUPP or -EACCES or -EPERM, return 0
> +	 * directly to avoid meaningless retries.
> +	 */
> +	if (err == -EOPNOTSUPP || err == -EACCES || err == -EPERM)
> +		return 0;

[Severity: High]
When promiscuous mode is denied for an untrusted VF, will this code
silently drop explicitly requested unicast and multicast addresses?

Since enetc_vf_set_rx_mode() catches the -EACCES error from the PF and
returns 0 without attempting to fallback and program the explicit address
lists, could this cause the VF to drop legitimate traffic (e.g. IPv6
Neighbor Discovery) when promiscuous mode requests fail?

> +
> +	return err;
> +}

[Severity: High]
This is a pre-existing issue, but when an untrusted VF sends a mailbox
message with an artificially small length, does it cause an out-of-bounds
heap read in the PF driver?

Looking at enetc_msg_handle_rxmsg() in
drivers/net/ethernet/freescale/enetc/enetc_msg.c, it allocates a buffer
whose size depends entirely on the VF-controlled len field without
enforcing a minimum size:

	msg = kzalloc_objs(*msg, msg_size);

Then, in enetc_msg_set_vf_mac_hash_filter(), the code validates len != 0
but fails to ensure the buffer is large enough for the array dereferences
up to index 3:

	case ENETC_MAC_FILTER_TYPE_ALL:
		if (!msg->hdr.len) {
			pf_msg = ENETC_PF_MSG_INV_LEN;
			goto vf_state_unlock;
		}

		uc_hash = (u64)msg->hash_tbl[1] << 32 | msg->hash_tbl[0];
		mc_hash = (u64)msg->hash_tbl[3] << 32 | msg->hash_tbl[2];

If a VF sends ENETC_MSG_SET_MAC_HASH_TABLE with len=1, it will read past
the allocated buffer bounds. Can this be used by an unprivileged untrusted
VF to directly attack the host PF?

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