netdev - Re: [PATCH] ice: Fix possible double free in error handling path

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: <d2b7a43f-2a6a-44ba-affa-90d581c7d2bf@intel.com>
Date: Fri, 30 Aug 2024 15:49:39 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Ma Ke <make24@...as.ac.cn>
CC: <anthony.l.nguyen@...el.com>, <przemyslaw.kitszel@...el.com>,
	<davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <shiraz.saleem@...el.com>, <david.m.ertman@...el.com>,
	<intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <stable@...r.kernel.org>
Subject: Re: [PATCH] ice: Fix possible double free in error handling path

From: Ma Ke <make24@...as.ac.cn>
Date: Fri, 30 Aug 2024 21:33:24 +0800

> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), callback function adev_release
> calls kfree(iadev). We shouldn't call kfree(iadev) again
> in the error handling path. Set 'iadev' to NULL.
> 
> Cc: stable@...r.kernel.org

You need to add a comment with the first kernel version which has this
bug. E.g.

Cc: stable@...r.kernel.org # 9.9+

(replace 9.9 with the actual version)

BTW usually Cc: stable goes *after* "Fixes:".

> Fixes: f9f5301e7e2d ("ice: Register auxiliary device to provide RDMA")
> Signed-off-by: Ma Ke <make24@...as.ac.cn>
> ---
>  drivers/net/ethernet/intel/ice/ice_idc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_idc.c b/drivers/net/ethernet/intel/ice/ice_idc.c
> index 145b27f2a4ce..5db05f54a80c 100644
> --- a/drivers/net/ethernet/intel/ice/ice_idc.c
> +++ b/drivers/net/ethernet/intel/ice/ice_idc.c
> @@ -330,6 +330,7 @@ int ice_plug_aux_dev(struct ice_pf *pf)
>  		return ret;
>  	}
>  
> +	iadev = NULL;
>  	ret = auxiliary_device_add(adev);
>  	if (ret) {
>  		auxiliary_device_uninit(adev);

I don't understand this patch.
I checked net-next, tnguy/net-queue, tnguy/next-queue and not in a
single branch @iadev is used after auxiliary_device_add().
IOW you just introduce a dead store which will be removed by the
compiler. The kfree() above that you mentioned is called only once and
never after auxiliary_device_uninit().
Can you draw a call ladder with the related code pieces how I can
reproduce the issue?

Thanks,
Olek