Received-SPF: pass (google.com: domain of linux-kernel+bounces-90997-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: <8ebecb51-3806-49bd-9319-66087fe9336a@amd.com>
Date: Mon, 4 Mar 2024 09:21:56 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net V2] net: pds_core: Fix possible double free in error
 handling path
To: hyper <hyperlyzcs@gmail.com>, brett.creeley@amd.com, davem@davemloft.net,
 edumazet@google.com, kuba@kernel.org, pabeni@redhat.com
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, jitxie@tencent.com,
 huntazhang@tencent.com
References: <333dca5e-fae7-4684-afa8-10b8fdd48bf6@amd.com>
 <20240303084954.14498-1-hyperlyzcs@gmail.com>
Content-Language: en-US
From: "Nelson, Shannon" <shannon.nelson@amd.com>
In-Reply-To: <20240303084954.14498-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?aCtiamVCbmVDVTZ0N05BSzFJOEtOVEVrVTBHRFZnR1RUREJjbm82K0UxNi9Y?=
 =?utf-8?B?TXlNUVhRMHZmbHVIc0RWazJUY054blpZek5DRnEvT3EwQVBqZWM1bDFBNnZK?=
 =?utf-8?B?WTdxSmFkVDc2QVZRRjVIMlJPUzAxeG9RN1EyTU1oWmp0SytqaHRBeU5kb1J2?=
 =?utf-8?B?TEVCb2tnYlRId3dOMzQ3dXkvT2dJUmcxOUowaCtVYk5KcFdZcUg5VWRDSnFF?=
 =?utf-8?B?dFp0TGk4dWVwQlVhMkt3UkJyTGtTT3pad3BzMTZCMTVCY04yeHlSWXFPeTUy?=
 =?utf-8?B?akpJb2gyQWJwM0R1T0RpT0JiQ2pSQXc1ZUJQd25aanlUamt5LzB0VHo3eHFv?=
 =?utf-8?B?VWUxaXJBQXhFWEwwQTBpTDl4MXV4ejk1TTRXOVJhYlA1VWZEbHRYT3dvdTg3?=
 =?utf-8?B?bDhvQWx4REpKWlhXWC9xS3Z4RDNkbWtHSXFMVmo2b2tpU1F6ZEx4TUdwcFFM?=
 =?utf-8?B?YUpXMklHa1RjWHVjTlRSYmY5K3QxWkZHcTBEcFp0cVNLVlk5Qzk2a3FqSlJ0?=
 =?utf-8?B?VG1sVGxuWlpGdFRkcEppcDZOZjFWTEFTenN6emhqZkdIbEQ2M29hdStTd0JG?=
 =?utf-8?B?QUZoZVpQQlRObFY0Sk1xaVNWeGR4ZDBPQ0NuYTVYeW13TFR1VWhRSk1tZTFw?=
 =?utf-8?B?RjRDaVJPWjFvczdqd0ZORjZUTGZQYWtXY2o5RExaV1BLeHNmTDBlZlpCck5r?=
 =?utf-8?B?blB6NVVIaUxVYzM1M2lURnB2ZkovZUFaR3lMVjRBUHh3dDRmTFlwTTJ1TnBO?=
 =?utf-8?B?bXhRTTNxa1lVZVVzYWdqOXlncjd4RWxVK2RhNXdLQWxDeTBYT25hcWNwTEkx?=
 =?utf-8?B?MG8rL0k0R2FhZExGS0VnOGVNYklOOW51QmxjUlFtQUFqU1VtZ1gvckxUR3ZX?=
 =?utf-8?B?TzM2V3N6NjRBNnVGVXhUM1BQMm1SaUlwRExPbjV3T2F6SlNqMmd1VHF3M1Qx?=
 =?utf-8?B?cnlBcTlyMlVjbldtQkJSTmxmNDl5SVVJU1N2TFk0R1VTUHE1YU1kVVVtL1VG?=
 =?utf-8?B?MElsRU9Ka3VZV0VNbDBxUXY4VmpWekphdXRZNG96clRaanQ1ZmduYWJUUU1j?=
 =?utf-8?B?VjJiS2ZsNE5ETEJtVU1WelE5b29wZk5yeG5tdjAzbVhlUmQzZkMwSXllY0p0?=
 =?utf-8?B?NlZTNXU2aHQzY0x3L01lZFNDRXBiaWFNd21LR2VjNmo4TGRjTVA5aHczWHFa?=
 =?utf-8?B?aUJ6MFFQNEp3QnpJL3BUOU1TdHVUUjE1RFE2RU9Yb28yaExwVjdXMzhuOTlD?=
 =?utf-8?B?bDg0S0c2VitaWWdNSmdyZWpIclpyKzE1cFZVUFVSdUo5c0V2Qm5aRGU2SCtV?=
 =?utf-8?B?Z2VqUVAyQUM2eDZCY0VTbU1BVHNGcWFiNnFlMDJBOERPVlBxM0JscmwrNTNr?=
 =?utf-8?B?YzA5TzhyYVNiK1lXK0tuWHZPUUQxeE4rcGVPdDJUMjcrelYzbThCK0JSa1M4?=
 =?utf-8?B?RitabThnMkNmZTVkaThDYmY1WjVCYXM5OTJDQ2dPY3E2QTJOZnJNbXVvTnZQ?=
 =?utf-8?B?SWFjU05DaVZLUG45bmk2M3lJQU8ybEE4SUN3ZVNUMUxIY2R3R3pWRUF3cVRK?=
 =?utf-8?B?RlhvNCs2VTBFMWlFZjcxTW1QY0gwT3UxeU40MDlGYlFmUzNLRnNJYmhzL1la?=
 =?utf-8?B?em9UNTFEakZic0d3L3ptNmNneE96RUtYSzZUYWFrdC9VWWdCcmtCYTBPNGFz?=
 =?utf-8?B?aXdUdmdWMjd0MTJsYU90WmNKNmFWM3hqZjA5SUdOQXFmcnhSN0ROc0NYVC94?=
 =?utf-8?B?amVIQmw3b2E5TVRIN2pDZER6eTN3RjNPc2hNRkNGVWpKM214OE1GNmQxQXNq?=
 =?utf-8?B?SXZzbnd4RmpYTDlPRDlkTkR6NjZJblVzWjdCQ05sbFAvUVVPeWRkRzNFRjlP?=
 =?utf-8?B?TXJrR1M4RHFxWHdVb1psSUZFM1FFYWR5RVg2VTBJWHVjanFvb3hjakE4OXBp?=
 =?utf-8?B?MlRRaXFZZWNwVE5wU1ZhdVUzSVVFS0xpTE9uTjNxSUNkQXR2WHBWZXhYNDVn?=
 =?utf-8?B?bWNYY2xENEVNSGJzRTQrRUk1ZFJyVThQSzFDdU9yOEVacXhkWERNQUQvZmFR?=
 =?utf-8?B?bzhXNk5sV2VyeGl0V1dvcGZhWE85K3Y4aHM4enNISG9RelZIbXdJSExYNlgw?=
 =?utf-8?Q?lIVBpCfwQCrsN3FLhppfinKcY?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: fc349b89-46bc-46e4-e5d4-08dc3c6f992a
X-MS-Exchange-CrossTenant-AuthSource: DS0PR12MB6583.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Mar 2024 17:21:59.0856
 (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: 5yMcudy3ewBLuByjakAVdvbJLWx0lvj5yF4xR8Xu6Oub71WcWwfVPnNpG8k6F/lC3y7FswH9h91g2OFqcJOnmQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR12MB6127

On 3/3/2024 12:49 AM, hyper 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: hyper <hyperlyzcs@gmail.com>

Thanks.

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
>