Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Message-ID: <8ff1ddf4-a3b7-2fed-5dfd-041aaaf981d2@intel.com>
Date:   Thu, 14 Sep 2023 08:31:16 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Betterbird/102.13.0
Subject: Re: [PATCH v3] libnvdimm/of_pmem: Use devm_kstrdup instead of kstrdup
 and check its return value
To:     Chen Ni <nichen@iscas.ac.cn>, <oohall@gmail.com>,
        <dan.j.williams@intel.com>, <vishal.l.verma@intel.com>,
        <ira.weiny@intel.com>, <aneesh.kumar@linux.ibm.com>
CC:     <nvdimm@lists.linux.dev>, <linux-kernel@vger.kernel.org>
References: <20230914070328.2121-1-nichen@iscas.ac.cn>
Content-Language: en-US
From:   Dave Jiang <dave.jiang@intel.com>
In-Reply-To: <20230914070328.2121-1-nichen@iscas.ac.cn>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?K3gwaThodzE4V1l1YXdOUmpUTldtbGQwWmd0UDRMKzJBR0k4dXNKUTlUYU84?=
 =?utf-8?B?bEhjcXplNVJDRkVUMmtPZytIM1pZR2R2RG0rL3RkS0JRLzVESWZxVWl0czlR?=
 =?utf-8?B?R0JpQzIzQy9nWUxpamF3dHVJV1VwYXRBMXFYUzU2eEdUQnRlVk1wZGFhQTJX?=
 =?utf-8?B?VXRQVjE5Q0NLYktFQk5KKzhvRnU3elFQS0pxQUdlSmlianozSkRudEsxdHFz?=
 =?utf-8?B?TC9EOFErRGRlREloTHUzYmhUWWhpMEt2MzFFQnpkZGw0UVY1YldiUGNrUWo0?=
 =?utf-8?B?NDJvRDd5cXdFVGIvU1dIa2FBd2tYUG9NVi9GeFJSdG9yR0ZWaktsSm9EQ1dH?=
 =?utf-8?B?UnVHK08vTWVtVkhTSG9Nc01HYzl6UHdGc1VzakFjQmdKOG1SMDUrM0hQUWNy?=
 =?utf-8?B?dXlGVFhGWDhLb08yUnZWbFJHaGJoa0lCbStSWnpZWU9lWVdDWlJ3L1hQRkFU?=
 =?utf-8?B?c1Nad01vYWtydzRJeE9qWHZYQ0RyaENQNjZTSm55UDlYQmNsTWpQT3ZPMVc2?=
 =?utf-8?B?MEJrVlo0Vk1HdFI0WXRUVnpXYTZhTWFXSHNVMXVLQlFxOVkrSUgxcmV3UXRX?=
 =?utf-8?B?RllaWVJKWDdHTHNtaThidE1XN1BqRStZKytNL25ueGdBNGxZSEZnKzFGTE9G?=
 =?utf-8?B?K2JDRW1CUWJTbFZJRWQ2Wm9hOUNJVUdTblpFNTlWbDBTa0hPSVliVlc4UXdT?=
 =?utf-8?B?VHg3YWdxKzkrWTVYeGhRdjdjUGt1ZFBEMDVicmdselBpckpPRmEwUUJHcFJ4?=
 =?utf-8?B?TVoxQ092WkZOWFNkbVRqeFk3dEhVNmIwRWdUMlN1ckpJNGRtVmg1RUJMRG1y?=
 =?utf-8?B?MmFMcDNEeXpyVnowTFA4RlNvSFlzOU5jdVJtMEc3cDQycnJMMDRCZmEvTEZ3?=
 =?utf-8?B?VE1mVS9SdTBNNmprQThSM1U3Y08wOUpjRGZoOFZKMkNsU25zSi9PTWVlckor?=
 =?utf-8?B?bnh6cHA0U3VSa1pTWW1ZWHVZcFQvUStCOEVvN2dRUEo1SFlHSnV4UDR3OWVo?=
 =?utf-8?B?ZEp0Nml6VkoySWFxUFB2d3BxZ0RzdXlDWUFDb2I0ZzZIMW15WkFjd29IL0d6?=
 =?utf-8?B?b0kvOUhhYThSeGVvc0FHeSs0bnFwYUVGcmkwdVhNODZYZG05T3E2U292YURz?=
 =?utf-8?B?MG1BQ3Qva3l5eDR0akxpb3BFTGF0NGd4bVJXUFZZODBBWldOa2hhL0ovckxR?=
 =?utf-8?B?VWpWN1ZtLzk4MTdFb3ZSb1dHTHRMUzRzZ3BwdWZYNytKOE1WVUhLVnU4aUpM?=
 =?utf-8?B?ckdZVENPamlLU1JMZS9NMlpLdU5FaVlMRlMvYXo0R2p1akFkYXBhbnVmcEha?=
 =?utf-8?B?RGRNcHpsb3VDbXZ1QjVQbzZjUktaZnI5ZGplbkhJNWNINnNadHQ0cXd0czA1?=
 =?utf-8?B?Y3NwTmdYblpoQlhWcEVkRHFHV2FNdDUzNm55WStHZExGWkpacTJ1RFpkdUtZ?=
 =?utf-8?B?YUlHc0hRM2tuWkJKYkgvd3NpUU84MWU4VnBra0VUSGo2NnlzcHowWXFSZVJk?=
 =?utf-8?B?ei85Q1EzR3RnaVZHS0V6SlRsQXo1T2xtN2pML2tGZnZFYkdkWXdCRlgycjF6?=
 =?utf-8?B?a1l2SmVTdmF5dWY1WWxvNmxSbDhZVFFURE1CZWdrUnpxcFZ3N29Qemg1eWlX?=
 =?utf-8?B?djI4MHp2cjNCVFp3bFplRVliaWRYYmozVEFmY1lpSEFPbmpHNzFTR2RpbVd6?=
 =?utf-8?B?ZkpBZC8zZUlWTlVkVUhiS1M3dDBoeVd4c2xpM1hLNTZ5U2NhNzNINWk3YmhT?=
 =?utf-8?B?NmszS0dnVlFZMDdzMlFlY0NJQXp5VC8wcGh2c1l1ZzJmWVZqeitZdTJEY2hr?=
 =?utf-8?B?VTA5T2NUOVZKMTRZSTBUWHE5cW5BR24wNUhSUWVmcGwzUlk0bTFxdVFDL0pN?=
 =?utf-8?B?eGdyZFRhczZzS3VadHVYWkEzYXdQQjc0cVJtbVhTb25VeW04ZUxJUTZPRURW?=
 =?utf-8?B?aWx2aHlhZmRwa3c4V21ybG5HNjc3d2NGMnZZbU5KUGpVZ1VwaG1zTWI3QVAr?=
 =?utf-8?B?MlQwSlRaYlNhWWJiN3dqdHI5WWFzK0lEbEQwSFRkTTc3SWJrZVBmNGcxN3Bn?=
 =?utf-8?B?WFhTOEtVLzRYUDJ5MjFNMGsyL3NMNlpSRnBaRmdQdHFVODJjaHdxWXdRQ2xt?=
 =?utf-8?Q?G6QOoeufOTHyIGxqJ1X2DX4kU?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 402b7360-3c23-480f-7c45-08dbb537a4e7
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB5984.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Sep 2023 15:31:20.0023
 (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: QV++OSLRMotaF3+TUFCgmcF/W5tgMroSFuVV8F0Dv5LNzg5JVzF2Q+6Xh/gwHHo6nYKrLHLs/4cPlsrYgT8F5A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB6519
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Thu, 14 Sep 2023 08:37:41 -0700 (PDT)
X-Spam-Status: No, score=-2.3 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email


On 9/14/23 00:03, Chen Ni wrote:
> Use devm_kstrdup() instead of kstrdup() and check its return value to
> avoid memory leak.
> 
> Fixes: 49bddc73d15c ("libnvdimm/of_pmem: Provide a unique name for bus provider")
> Signed-off-by: Chen Ni <nichen@iscas.ac.cn>


Reviewed-by: Dave Jiang <dave.jiang@intel.com>

One unrelated comment below.

> ---
> Changelog:
> 
> v2 -> v3:
> 
> 1. Use devm_kstrdup() instead of kstrdup()
> 
> v1 -> v2:
> 
> 1. Add a fixes tag.
> 2. Update commit message.
> ---
>  drivers/nvdimm/of_pmem.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvdimm/of_pmem.c b/drivers/nvdimm/of_pmem.c
> index 1b9f5b8a6167..5765674b36f2 100644
> --- a/drivers/nvdimm/of_pmem.c
> +++ b/drivers/nvdimm/of_pmem.c
> @@ -30,7 +30,13 @@ static int of_pmem_region_probe(struct platform_device *pdev)
>  	if (!priv)
>  		return -ENOMEM;
>  
> -	priv->bus_desc.provider_name = kstrdup(pdev->name, GFP_KERNEL);
> +	priv->bus_desc.provider_name = devm_kstrdup(&pdev->dev, pdev->name,
> +							GFP_KERNEL);
> +	if (!priv->bus_desc.provider_name) {
> +		kfree(priv);

I wonder if priv should be allocated with devm_kzalloc() instead to reduce the resource management burden. 

> +		return -ENOMEM;
> +	}
> +
>  	priv->bus_desc.module = THIS_MODULE;
>  	priv->bus_desc.of_node = np;
>