Re: [net-next 2/2] Octeontx2-pf: Add support for DMAC_FILTER trap

[Simon Horman <horms@kernel.org>]

On Wed, Jan 14, 2026 at 12:27:43PM +0530, Hariprasad Kelam wrote:

...

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c

...

> +
> +int otx2_devlink_traps_register(struct otx2_nic *pf)
> +{
> +	const u32 groups_count = ARRAY_SIZE(otx2_trap_groups_arr);
> +	const u32 traps_count = ARRAY_SIZE(otx2_trap_items_arr);
> +	struct devlink *devlink = priv_to_devlink(pf->dl);
> +	struct otx2_trap_data *trap_data;
> +	struct otx2_trap *otx2_trap;
> +	int err, i;
> +
> +	trap_data = kzalloc(sizeof(*trap_data), GFP_KERNEL);
> +	if (!trap_data)

Hi Hariprasad,

If the code reaches this line then the function will return an
error value, and pf->dl->trap_data will be NULL.

> +		return -ENOMEM;
> +
> +	trap_data->trap_items_arr = kcalloc(traps_count,
> +					    sizeof(struct otx2_trap_item),
> +					    GFP_KERNEL);
> +	if (!trap_data->trap_items_arr) {
> +		err = -ENOMEM;
> +		goto err_trap_items_alloc;
> +	}
> +
> +	trap_data->dl = pf->dl;
> +	trap_data->traps_count = traps_count;
> +	pf->dl->trap_data = trap_data;
> +
> +	err = devlink_trap_groups_register(devlink, otx2_trap_groups_arr,
> +					   groups_count);
> +	if (err)
> +		goto err_groups_register;
> +
> +	for (i = 0; i < traps_count; i++) {
> +		otx2_trap = &otx2_trap_items_arr[i];
> +		err = devlink_traps_register(devlink, &otx2_trap->trap, 1,
> +					     pf);
> +		if (err)
> +			goto err_trap_register;
> +	}
> +
> +	return 0;
> +
> +err_trap_register:
> +	for (i--; i >= 0; i--) {
> +		otx2_trap = &otx2_trap_items_arr[i];
> +		devlink_traps_unregister(devlink, &otx2_trap->trap, 1);
> +	}
> +	devlink_trap_groups_unregister(devlink, otx2_trap_groups_arr,
> +				       groups_count);
> +err_groups_register:
> +	kfree(trap_data->trap_items_arr);
> +err_trap_items_alloc:
> +	kfree(trap_data);

And if the code reaches this line then the function will
return an error value, with pf->dl->trap_data set to a pointer
which has been freed.

> +	return err;
> +}
> +
> +void otx2_devlink_traps_unregister(struct otx2_nic *pf)
> +{
> +	struct otx2_trap_data *trap_data = pf->dl->trap_data;
> +	struct devlink *devlink = priv_to_devlink(pf->dl);
> +	const struct devlink_trap *trap;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(otx2_trap_items_arr); ++i) {
> +		trap = &otx2_trap_items_arr[i].trap;
> +		devlink_traps_unregister(devlink, trap, 1);
> +	}
> +
> +	devlink_trap_groups_unregister(devlink, otx2_trap_groups_arr,
> +				       ARRAY_SIZE(otx2_trap_groups_arr));
> +	kfree(trap_data->trap_items_arr);

But, if otx2_devlink_traps_register returns an error value then
otx2_devlink_traps_unregister will be called. And the like above
will dereference pf->dl->trap_data.

> +	kfree(trap_data);
> +}

Flagged by Claude Code with Review Prompts [1].

https://github.com/masoncl/review-prompts/

...

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> index a7feb4c392b3..5da1605a1a90 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> @@ -3282,6 +3282,10 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (err)
>  		goto err_mcam_flow_del;
>  
> +	err = otx2_devlink_traps_register(pf);
> +	if (err)
> +		goto err_traps_unregister;
> +
>  	/* Initialize SR-IOV resources */
>  	err = otx2_sriov_vfcfg_init(pf);
>  	if (err)
> @@ -3314,6 +3318,8 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	otx2_sriov_vfcfg_cleanup(pf);
>  err_pf_sriov_init:
>  	otx2_shutdown_tc(pf);
> +err_traps_unregister:
> +	otx2_devlink_traps_unregister(pf);
>  err_mcam_flow_del:
>  	otx2_mcam_flow_del(pf);
>  err_unreg_netdev:
> @@ -3514,6 +3520,7 @@ static void otx2_remove(struct pci_dev *pdev)
>  	/* Disable link notifications */
>  	otx2_cgx_config_linkevents(pf, false);
>  
> +	otx2_devlink_traps_unregister(pf);
>  	otx2_unregister_dl(pf);
>  	unregister_netdev(netdev);
>  	cn10k_ipsec_clean(pf);
> -- 
> 2.34.1
> 
>