netdev - Re: [net-next,v5,10/13] octeontx2-af: npc: cn20k: Allocate MCAM entry for flow installation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aXmEQ00dZU_psD-0@rkannoth-OptiPlex-7090>
Date: Wed, 28 Jan 2026 09:06:35 +0530
From: Ratheesh Kannoth <rkannoth@...vell.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: <sbhatta@...vell.com>, <edumazet@...gle.com>, <andrew+netdev@...n.ch>,
        <netdev@...r.kernel.org>, <sgoutham@...vell.com>, <sumang@...vell.com>,
        <pabeni@...hat.com>, <davem@...emloft.net>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [net-next,v5,10/13] octeontx2-af: npc: cn20k: Allocate MCAM
 entry for flow installation

On 2026-01-28 at 07:59:30, Jakub Kicinski (kuba@...nel.org) wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> octeontx2-af: npc: cn20k: Allocate MCAM entry for flow installation
>
> This patch extends the install_flow mailbox request message with additional
> fields that allow the AF driver to determine the required NPC MCAM entry
> type, allocate the MCAM entry, and complete the flow installation in a
> single step for CN20K.
>
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> > index 052d989f2d9a..efd994c65b32 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
>
> [ ... ]
>
> > +static int otx2_get_kw_type(struct otx2_nic *pfvf,
> > +			    struct npc_install_flow_req *fl_req,
> > +			    u8 *kw_type)
> > +{
> > +	struct npc_get_num_kws_req *req;
> > +	struct npc_get_num_kws_rsp *rsp;
> > +	u8 *src, *dst;
> > +	int off, err;
> > +	int kw_bits;
> > +
> > +	off = offsetof(struct npc_install_flow_req, packet);
> > +
> > +	mutex_lock(&pfvf->mbox.lock);
> > +
> > +	req = otx2_mbox_alloc_msg_npc_get_num_kws(&pfvf->mbox);
> > +
> > +	dst = (u8 *)&req->fl + off;
>                   ^^^
ACK.

>
> Can this dereference a NULL pointer? otx2_mbox_alloc_msg_* functions can
> return NULL on allocation failure, and this line accesses req->fl without
> first checking if req is NULL.
>
> Other similar functions in this file (e.g., otx2_mcam_pfl_info_get(),
> otx2_get_dft_rl_idx()) properly check for NULL after the mailbox
> allocation before using the returned pointer.
>
> > +	src = (u8 *)fl_req + off;
> > +
> > +	memcpy(dst, src, sizeof(struct npc_install_flow_req) - off);
>
> [ ... ]
>
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> > index 963d12ecd328..5fe077935c97 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
>
> [ ... ]
>
> > @@ -1759,8 +1857,11 @@ int rvu_mbox_handler_npc_install_flow(struct rvu *rvu,
> >  		enable = true;
> >
> >  	/* Do not allow requests from uninitialized VFs */
> > -	if (from_vf && !enable)
> > +	if (from_vf && !enable) {
> > +		rvu_npc_free_entry_for_flow_install(rvu, req->hdr.pcifunc,
> > +						    allocated, req->entry);
> >  		return NPC_FLOW_VF_NOT_INIT;
> > +	}
> >
> >  	/* PF sets VF mac & VF NIXLF is not attached, update the mac addr */
> >  	if (pf_set_vfs_mac && !enable) {
> >  		ether_addr_copy(pfvf->default_mac, req->packet.dmac);
> >  		ether_addr_copy(pfvf->mac_addr, req->packet.dmac);
> >  		set_bit(PF_SET_VF_MAC, &pfvf->flags);
> >  		return 0;
>                 ^^^^^^^^^^
>
> Does this path leak the allocated MCAM entry? When rvu_npc_alloc_entry_for_
> flow_install() allocates an entry (setting allocated=true), but then this
> early return path is taken, rvu_npc_free_entry_for_flow_install() is not
> called. All other early return paths in this function properly free the
> entry, but this one does not.
ACK.

>
> >  	}
> >
> >  	mutex_lock(&rswitch->switch_lock);