Received-SPF: pass (google.com: domain of linux-kernel+bounces-94311-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <ed1fdbab-bc63-4aaa-b516-0c03e6755824@amd.com>
Date: Wed, 6 Mar 2024 09:05:11 -0800
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,
 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: "Nelson, Shannon" <shannon.nelson@amd.com>
In-Reply-To: <20240306105714.20597-1-hyperlyzcs@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
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?OUU4NW1ya2JxK1RJU3VFUVhINGY0Zk9OZ01EY3hwRW0xWUI5UEZPMFNGS1kv?=
 =?utf-8?B?dm0yVXBMQmg2UXZLMkpEdDBKSG5lQ0VlUGxpbm1OdXJUeTRPaHRkVEZIcXo5?=
 =?utf-8?B?RWFESkxFbmFkVTArZmRlYS9HdVk3V2lwcTFRV2hCSHVqVVo2WjZhQ3YvOFQ3?=
 =?utf-8?B?Qkx2akMyd2NPRWZOYlFDUUtBMUFFbG5iL1crV05iODd2eC9FbWVDcDhXQ05s?=
 =?utf-8?B?OTgvTk9WU1lpU2c0ZDR3R1d1VUduT01BeVY0SFo4Wm95UWs2MDhUMUVMNGRr?=
 =?utf-8?B?WDMzeVBSWTdZZWtmY0J0WlJyNGZxQnVtb2NGZ3BIaDhabzVvdDhjRmNoRkdL?=
 =?utf-8?B?MERseXdXZFVqblpObXEzbVZqSTBjT3ZZTTUvZ2Z3eWVWR0NydjRnbjMzYUR4?=
 =?utf-8?B?aVViZGhyeCtvNno3RXJ3YlhzT28wSHlMaWp4R1ZzeDdVYnpYZ29NNldGaFlh?=
 =?utf-8?B?c2tHWXJFZkhNR3ovNWQ2MTA4anBOWElObWxlRjFwUFJOMUM4b2hZZTM5dFNO?=
 =?utf-8?B?SnJIeFdsNStvUFo0amo2R044ZDdwWS9YNHRnZlVlSjdXY3loWmFINkNxVlBT?=
 =?utf-8?B?cjhuNXN2cjIwWi9hSWZ5KzlMRlMvTGUrSnVNeEFFd2lOMnFxTktxZmZLZStp?=
 =?utf-8?B?RVo4dUJMdzJTcytGV0lYbGtid01NRlBYbm94QkREalBzbkVYSlE4bmwzMUlO?=
 =?utf-8?B?OUUxdDdER3J4Y29kZlhaRUdSM0dvbldDQjdDL0hyYnhmeEpoRk5Ob2hPVjhu?=
 =?utf-8?B?UkFTRWY5MEpsMXJ2WDNUTmJ4S0lQL0Jtek90dVVxUFh4L2NMNzlmbjZZTTgy?=
 =?utf-8?B?ZUU3VWNSdXZVU3RWaENJeDc4OXFmTnFCd0ZMeFd4emRoaFJOZEdGWmdPR1kr?=
 =?utf-8?B?a2F0OUR6TTM4UWFTbXA4TlZBMWNUcEtDaWE3ZitsZHRRWXRjR3BtRGJjRzh1?=
 =?utf-8?B?Sm9TWTVrbGV5SWRMOWhycTlpS1VqNk5HbHREQzBJeFNhMzVEYUhsam9VcVFY?=
 =?utf-8?B?ZG5kQmEwRUZoZFh5ZGhqZ3Z5dElUR1lWUzl2cVpwV2doanNIa2RZa3k5Y1RH?=
 =?utf-8?B?MXVhaVVRMnk2Sk93Q3k1blJUSG9TMmxTMkhLWURsSGRFckUzbXNDcE9xTG5n?=
 =?utf-8?B?dERmeEdZUjRCTVhjcSt0b3FxRnlldVoxZ2E2SGlZTGd5d2lsWHp0RnBIQzZ1?=
 =?utf-8?B?ZTVvaTAvUEFid3BWdFg4WlVjYWhKcjhYcmt0c1J6TFloTk5USmpoNnR1d3Ju?=
 =?utf-8?B?TWRJa2xEQWJRb0VPNXJ2OWg2T2pTZHdjTG82TGlndWZ3UFV5SE9NOEhzMFFQ?=
 =?utf-8?B?eW41Mk9ZeGN2eFNyWUxwZFUrTEpJWHlkVjVicGdxaGc4bXV0MFpqS0U2dzh3?=
 =?utf-8?B?dUlXb2hLWlRtajI1U01Db1ZuMDlGUUViYmpSNFZEVDVNOUxjWnI2M1BYc1FG?=
 =?utf-8?B?WTJjYVovbWs1R1k5YnNYMmhLd0xSeElqdTNydU1VNnoyWjc2N1g0UCtQc3Bi?=
 =?utf-8?B?M2VwcS9VUzVnTUs0a3hTTlVrTVpBNXI2TGp1NU9YaEc1U3IzWmZ0R05ZS0tZ?=
 =?utf-8?B?aTJMMEZONi9sbFd2TkdVNVZDMU9SeFJIK0RZVVk1MlUrTkc3eTFhK01uYXlJ?=
 =?utf-8?B?Tm5iS2hKTVh3VGdEekdFeEkyZk1yL1p6S2tDYWM1VHBnMElEaXJwVE5BTmFi?=
 =?utf-8?B?RUd3SENwdldXNjhUYTl1cGN2RlFoMHhPMDBoaWRaQS9pMEpJZGhyZVlDRnU5?=
 =?utf-8?B?anpmc2FqK2JLZzQ3TGFnZ0FHOGZra2ZPZlFWcEJ6MHFLWFpwcXc2T0tUQ25h?=
 =?utf-8?B?aTVscG0yRFBSMkZuVE5iZzVGUzhUYTBsUURUVGNHS0ZJdGFWMjBsNDlnRXBz?=
 =?utf-8?B?ekFRN1d4c24vb3FVU29xVlJFL1hvYnhsK1daeHNmbU4wazZWUFhkK1ZkdlB5?=
 =?utf-8?B?ZFMwRFE2RWxVc0lMdmVTRkYvTnN2NHJFS1F4SWRQK0hiWEsyL0g3bjRIYTRU?=
 =?utf-8?B?U0g2STJTK2VSb3FIVGp0bllqODdBdG1ZOTlnUkc2dGRvNDE3dy92TnNqNmRh?=
 =?utf-8?B?bWR1ZHhOc1VlWG9rMWRsVUNHSWt2QlJybGlzeUJncVg1akVkbTB6bmc0ZWNs?=
 =?utf-8?Q?1M+fbDdbNvD3b1SXmCc9a2TIE?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1ed285b0-e169-483a-8527-08dc3dff9707
X-MS-Exchange-CrossTenant-AuthSource: DS0PR12MB6583.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Mar 2024 17:05:14.1148
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 6hLaooO7M5YmXopTvevTgUvoEeRsJYcbnJS/ESLUW+94TJvJKBtABsVscJqCkHMd9AyrQEWIa4ii9rwVL/YTmA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4282

On 3/6/2024 2:57 AM, 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: Shannon Nelson <shannon.nelson@amd.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)
> --
> 2.36.1
>