netdev - Re: [PATCH iwl-net v1] idpf: fix potential memory leak on kcalloc() failure

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: <20250411155144.GJ395307@horms.kernel.org>
Date: Fri, 11 Apr 2025 16:51:44 +0100
From: Simon Horman <horms@...nel.org>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
	Pavan Kumar Linga <pavan.kumar.linga@...el.com>,
	Aleksandr Loktionov <aleksandr.loktionov@...el.com>
Subject: Re: [PATCH iwl-net v1] idpf: fix potential memory leak on kcalloc()
 failure

On Fri, Apr 11, 2025 at 07:09:12AM +0200, Michal Swiatkowski wrote:
> On Mon, Apr 07, 2025 at 11:43:50AM +0100, Simon Horman wrote:
> > On Fri, Apr 04, 2025 at 12:54:21PM +0200, Michal Swiatkowski wrote:
> > > In case of failing on rss_data->rss_key allocation the function is
> > > freeing vport without freeing earlier allocated q_vector_idxs. Fix it.
> > > 
> > > Move from freeing in error branch to goto scheme.
> > > 
> > > Fixes: 95af467d9a4e ("idpf: configure resources for RX queues")
> > 
> > Hi Michal,
> > 
> > WRT leaking q_vector_indxs, that allocation is not present at
> > the commit cited above, so I think the correct Fixes tag for
> > that problem is the following, where that allocation was added:
> > 
> > Fixes: d4d558718266 ("idpf: initialize interrupts and enable vport")
> 
> Thanks for checking that. I agree, my fixes is wrong.
> 
> > 
> > I do note that adapter->vport_config[idx] may be allocated but
> > not freed on error in idpf_vport_alloc(). But I assume that this
> > is not a leak as it will eventually be cleaned up by idpf_remove().
> 
> Right, it will be better to free it directly for better readable.
> Probably candidate for net-next changes.

Thanks, that does sound like a nice idea.

...