Received-SPF: pass (google.com: domain of linux-kernel+bounces-93996-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <5fe277d9-56a9-4105-a915-bf178e556a22@intel.com>
Date: Wed, 6 Mar 2024 14:44:59 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net V3] net: pds_core: Fix possible double free in error
 handling path
Content-Language: en-US
To: Yongzhi Liu <hyperlyzcs@gmail.com>, <pabeni@redhat.com>,
	<shannon.nelson@amd.com>, <brett.creeley@amd.com>, <davem@davemloft.net>,
	<edumazet@google.com>, <kuba@kernel.org>
CC: <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<jitxie@tencent.com>, <huntazhang@tencent.com>
References: <9c4da99a83bceb4680aac9588151cc8190ff07e0.camel@redhat.com>
 <20240306105714.20597-1-hyperlyzcs@gmail.com>
From: Wojciech Drewek <wojciech.drewek@intel.com>
In-Reply-To: <20240306105714.20597-1-hyperlyzcs@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eFFmS2ZMQ3VmTXdRbGNLVjBJK3pPTmp2eVZIQjN3a0hsanBWbWJPUXJYV0xW?=
 =?utf-8?B?TVlyRnNqS3locjBLM1J4dnJVSEk3cjVhMjJUOUE5OXBSbW1oS1YxMlBzamx0?=
 =?utf-8?B?WFZhUGVhbUNnK1R3aHhDN0hEbXVhUFVmZURQRS8ybW1lQmVkd0I3WDg4cWZa?=
 =?utf-8?B?ZmNGaVU5RVNKTXo3bE9hOGUvSGpOWVpFT3VvWFdmT1o2TklnaVFjd2s2UVhr?=
 =?utf-8?B?aTNHYzNQMzBTNDhVRWhieEM5ZDM1OHJ1TjFaeDljMGxMZTdHdWM5TW0rRjdD?=
 =?utf-8?B?U0hjYzBsajg2bWFCaDQ2cjVzOW9VTTRhYjhQK2haZ2pVemNzYThtak96OC9M?=
 =?utf-8?B?c1RjWlUzbUE2RjFxKzh1bUp6dUtiajJhRnQ0WlBsZk9IRXdVTVdONUFoZ2dX?=
 =?utf-8?B?SHVQRjdRTjU1VHZERllGNnpLWHdKK0ppNEFNb3BwdS9taTRqOHptaVp4SWl0?=
 =?utf-8?B?eUdER0Q4aVpob3JHQmRNMzRxVkw1UTlScEFtNGNRbndBTDJHN2s5MThubEVF?=
 =?utf-8?B?clR1VnBiSUxPS3A2d0RMbE5TR3F0Zmd6TnpMNFVFaU1nV2FWaFAySDhNVFQ1?=
 =?utf-8?B?T0NLeFF3elRIWGVveHpvYlF5djAvWi9aSUc5c2dHWFNSeGNacDJ6MXFKOWdh?=
 =?utf-8?B?TnR3M3QzNjd6YktIQVFNRHhERlYrWFFLSm5mQ2dwQ1FyR3YvTkc5dVcwdlBs?=
 =?utf-8?B?Z1k4NTRnMFV3cXFtMUxPSDMxM2lJR25Va0FLeVk1cGhyK1NzRDk0VVFhWDNE?=
 =?utf-8?B?Q2hZcmJYdUVkL1pxUDNaTzBDVG5YNVFjbno1WitxRjBOUU14U2EzOTFCdFpM?=
 =?utf-8?B?NTUyaDBFRk5PNjFzWndEQThKaVZLOXdsRWlRMC9oeFgvTERENHpLUWFzVW10?=
 =?utf-8?B?UWk5UFZrdXlnUmNLWW9aMEp1cGZFcXd1TUE2ZFlsSnprMC9ZcXEwY21kcDVm?=
 =?utf-8?B?WUtIc2VFVkRsYzM0QU02WFYxQ2g2dHR3NkZsSkhoTlI2T3BVREtIZUdrR0xW?=
 =?utf-8?B?aW0yODVSczh2dzNlNFlQWmdMRWxvZTM5M2tEK2NrK2JOWGlSMUo1anI5Y3F5?=
 =?utf-8?B?ckhZYlJDRlV3Qk5CNmZLVUMwNEZNY0w5UEVJbldVb04reUhUOVY4TVhIM2w3?=
 =?utf-8?B?bmwwbytscU13RHV0SXp3OHY2QzR1MTNkdVFBT1NSb1d2RTBMWTVqUlFZNWtt?=
 =?utf-8?B?ZXJPY3ZmeDlvVngxZzlwUktxbHNNcGRhRkxmMGM5aVYyakYyNWlpNFpCaGV1?=
 =?utf-8?B?aS9vSTdINXhsaU44a0pEZ3FFOUlaeHJwOTZEOVdQWXYzQWMyNmpLanhtTXY0?=
 =?utf-8?B?Z1k1U2VFRU9tK2dBaDk4RHN6RHh3aTd1REZkRndLY0JzZCtSR0FEN0VIa1lW?=
 =?utf-8?B?VWQrVkx5MkE2am1VODdjMW9BNmN1cll5WTNkakhxM2F1N29XZmFSTGxKVWps?=
 =?utf-8?B?VXMwVlhPYVVqQm1PU1JOOWd1Ry91em9PR0k3NitqNmdsek4vZmpYdTNneklp?=
 =?utf-8?B?K2hDdkE1dklsM1pmZGliZUdQK2VxYlpNTUVzdzhCMkxzZ0hEbk55ZEdXMVlh?=
 =?utf-8?B?eHczUmViWk5jN2JEOGZlbTNOMXFzczg2RUlRQktudllOdSthS2hNZjdtazhU?=
 =?utf-8?B?dUtqTEdlM0orYVRzYzVLODIxZ3hDMnZWZmNQbU05cWdVcXJHUTJNeFpyVHlx?=
 =?utf-8?B?clR5Qzl2RWVnSlhXSlFxUUZwSFRzWDh1bS9HaG55ZHl3Yk0xaFBkTkVIaWN1?=
 =?utf-8?B?dlZ4bzFwd1JRWEJPZjZndGI3RzVOK1dXWW5lMngzTlVvU0dwL3NDZmdMRGtx?=
 =?utf-8?B?bkM5QVpQTHhWc0xkMEJWTkt4R2RDU2hka1czVTJXSHJsWGZ5V3BnY2hJYnBy?=
 =?utf-8?B?SEQ4aVNFVm9vdHYyWk05eGdqNkEwTXo1dllVbmR5N1VtVkRubVpxVHRWV3NO?=
 =?utf-8?B?Vm1IT25aOXNINlV3SUF4Z0t6STJxS000RnB0RU9wbTdQd0ltc0VWbjl6OWZj?=
 =?utf-8?B?ZDB2WFdPZjZtU0lUM1c2Q1lUZWdsK0UvSU9pYTZxazZDNm1Ic0FEUlRMU09u?=
 =?utf-8?B?a29yUXBmWkZnenNaSE1TSjJYRE8vUUNpdG9id0E1REFpR2RRa252NnRiL2gv?=
 =?utf-8?B?aXBoVStVNURNaEdheGlvaWNkU3BMNnZiL3Z3cDU4cEMyUFBlY1JDOFo4MEsy?=
 =?utf-8?B?WlE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 75c66c65-7db3-4559-2dc6-08dc3de3a0c8
X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB5776.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Mar 2024 13:45:04.6387
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: uPNGp9snSBZLi7OBFdeE0oFsL+MOte+Q8yWJcFNWPx6ucZ0oMPOI+nD0c/AVEgGrRdloGMZOO6FmNqsRyGz91kK88HDWleIzos2/Xbllf5k=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5309
X-OriginatorOrg: intel.com


On 06.03.2024 11:57, Yongzhi Liu wrote:
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), Callback function pdsc_auxbus_dev_release
> calls kfree(padev) to free memory. We shouldn't call kfree(padev)
> again in the error handling path.
> 
> Fix this by cleaning up the redundant kfree() and putting
> the error handling back to where the errors happened.
> 
> Fixes: 4569cce43bc6 ("pds_core: add auxiliary_bus devices")
> Signed-off-by: Yongzhi Liu <hyperlyzcs@gmail.com>
> ---

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

>  drivers/net/ethernet/amd/pds_core/auxbus.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amd/pds_core/auxbus.c b/drivers/net/ethernet/amd/pds_core/auxbus.c
> index 11c23a7f3172..fd1a5149c003 100644
> --- a/drivers/net/ethernet/amd/pds_core/auxbus.c
> +++ b/drivers/net/ethernet/amd/pds_core/auxbus.c
> @@ -160,23 +160,19 @@ static struct pds_auxiliary_dev *pdsc_auxbus_dev_register(struct pdsc *cf,
>  	if (err < 0) {
>  		dev_warn(cf->dev, "auxiliary_device_init of %s failed: %pe\n",
>  			 name, ERR_PTR(err));
> -		goto err_out;
> +		kfree(padev);
> +		return ERR_PTR(err);
>  	}
>  
>  	err = auxiliary_device_add(aux_dev);
>  	if (err) {
>  		dev_warn(cf->dev, "auxiliary_device_add of %s failed: %pe\n",
>  			 name, ERR_PTR(err));
> -		goto err_out_uninit;
> +		auxiliary_device_uninit(aux_dev);
> +		return ERR_PTR(err);
>  	}
>  
>  	return padev;
> -
> -err_out_uninit:
> -	auxiliary_device_uninit(aux_dev);
> -err_out:
> -	kfree(padev);
> -	return ERR_PTR(err);
>  }
>  
>  int pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)