Re: [PATCH net-next 14/14] net: enetc: use kzalloc_flex() for enetc_psfp_gate allocation

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing spinlock acquisition in enetc_psfp_destroy_clsflower leading to list corruption and Use-After-Free.
- [High] Hardware State Machine Race: Stream ID rules are enabled before Stream Gate and Filter actions.
- [High] Dead code in enetc_check_flow_actions completely breaks ACL offloading for ACCEPT and DROP actions.
--

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

net: enetc: use kzalloc_flex() for enetc_psfp_gate allocation

Replace the open-coded struct_size() + kzalloc() pattern with the
kzalloc_flex() helper when allocating struct enetc_psfp_gate. This
removes the intermediate entries_size local variable and makes the
allocation site more concise.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> index 7b17bca24f260..2aa0fcaafcd23 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> @@ -1135,7 +1135,6 @@ static int enetc_psfp_parse_clsflower(struct enetc_ndev_priv *priv,
>  	struct flow_action_entry *entry;
>  	struct action_gate_entry *e;
>  	u8 sfi_overwrite = 0;
> -	int entries_size;
>  	int i, err;
>  
>  	if (f->common.chain_index >= priv->psfp_cap.max_streamid) {
> @@ -1242,8 +1241,7 @@ static int enetc_psfp_parse_clsflower(struct enetc_ndev_priv *priv,
>  		goto free_filter;
>  	}
>  
> -	entries_size = struct_size(sgi, entries, entryg->gate.num_entries);
> -	sgi = kzalloc(entries_size, GFP_KERNEL);
> +	sgi = kzalloc_flex(*sgi, entries, entryg->gate.num_entries);
>  	if (!sgi) {
>  		err = -ENOMEM;
>  		goto free_filter;

While reviewing this patch, I noticed a few pre-existing issues in the same
file.

> static int enetc_psfp_destroy_clsflower(struct enetc_ndev_priv *priv,
> 					struct flow_cls_offload *f)
> {
[ ... ]
> 	err = enetc_streamid_hw_set(priv, &filter->sid, false);
> 	if (err)
> 		return err;
> 
> 	remove_one_chain(priv, filter);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
This is a pre-existing issue, but is it safe to modify the global lists and free
the filter here without acquiring epsfp.psfp_lock?

enetc_psfp_get_stats() can be called concurrently and it accesses the stream list
without the lock:

drivers/net/ethernet/freescale/enetc/enetc_qos.c:enetc_psfp_get_stats() {
    ...
	filter = enetc_get_stream_by_index(f->common.chain_index);
	if (!filter)
		return -EINVAL;
    ...
}

Does this race lead to list corruption or a use-after-free when the stats
function dereferences the concurrently freed filter?

> static int enetc_psfp_hw_set(struct enetc_ndev_priv *priv,
> 			     struct enetc_streamid *sid,
> 			     struct enetc_psfp_filter *sfi,
> 			     struct enetc_psfp_gate *sgi,
> 			     struct enetc_psfp_meter *fmi)
> {
> 	int err;
> 
> 	err = enetc_streamid_hw_set(priv, sid, true);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
This isn't a bug introduced by this patch, but does this create a hardware state
machine race?

The frontend stream identification rule is enabled here, before the backend
components are configured.

[ ... ]
> 	err = enetc_streamgate_hw_set(priv, sgi, true);

Since the backend stream gate is enabled later, is there a window where packets
could hit uninitialized gate hardware, causing dropped packets or misapplied
policies?

> static struct actions_fwd *
> enetc_check_flow_actions(u64 acts, unsigned long long inputkeys)
> {
> 	int i;
> 
> 	for (i = 0; i < ARRAY_SIZE(enetc_act_fwd); i++)
> 		if (acts == enetc_act_fwd[i].actions &&
> 		    inputkeys & enetc_act_fwd[i].keys)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
This is a pre-existing issue, but does this condition completely break ACL
offloading for ACCEPT and DROP actions?

In the enetc_act_fwd table, ACCEPT and DROP actions define their required keys
as 0.

drivers/net/ethernet/freescale/enetc/enetc_qos.c:enetc_act_fwd[] = {
    ...
	/* example for ACL actions */
	{
		BIT(FLOW_ACTION_DROP),
		0,
		FILTER_ACTION_TYPE_ACL
	}
};

This means inputkeys & 0 will always evaluate to 0 (false), preventing them
from ever being matched.

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