[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aW5HLPxp5oBzZVIM@horms.kernel.org>
Date: Mon, 19 Jan 2026 15:01:00 +0000
From: Simon Horman <horms@...nel.org>
To: Hariprasad Kelam <hkelam@...vell.com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org, kuba@...nel.org,
davem@...emloft.net, sgoutham@...vell.com, gakula@...vell.com,
jerinj@...vell.com, lcherian@...vell.com, sbhatta@...vell.com,
naveenm@...vell.com, edumazet@...gle.com, pabeni@...hat.com,
andrew+netdev@...n.ch, bbhushan2@...vell.com
Subject: Re: [net-next 2/2] Octeontx2-pf: Add support for DMAC_FILTER trap
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
>
>
Powered by blists - more mailing lists