[<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
Powered by blists - more mailing lists