Received-SPF: pass (google.com: domain of linux-kernel+bounces-35823-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <2f8c0dfd-5d3a-4135-9d8e-09091fdf87eb@amd.com>
Date: Tue, 23 Jan 2024 09:52:51 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] vfio/pds: Potential memory leak in
 pds_vfio_dirty_enable()
To: Cong Liu <liucong2@kylinos.cn>, Brett Creeley <brett.creeley@amd.com>,
 Jason Gunthorpe <jgg@ziepe.ca>, Yishai Hadas <yishaih@nvidia.com>,
 Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>,
 Kevin Tian <kevin.tian@intel.com>,
 Alex Williamson <alex.williamson@redhat.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20240123011319.6954-1-liucong2@kylinos.cn>
Content-Language: en-US
From: Brett Creeley <bcreeley@amd.com>
In-Reply-To: <20240123011319.6954-1-liucong2@kylinos.cn>
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?SWovYktRTlZXMDJGbVpqdkR2NEQxY2M1RTVzaDRPYnQxc0VjSTZEbFdMaEdu?=
 =?utf-8?B?MWh1c1VxUHpKU09KZmUvcEl2WDNpdTZxblVRWXJBdHU2Tk9SRmo5bkVMN2Ny?=
 =?utf-8?B?dE5EYVVKK1RnVzRSdURWN1loNlFwZks3TmI4cDR6bzg1NklKZXRqejgxSXVp?=
 =?utf-8?B?aTZVMi9ieWJEUHZEOUJiTUlVNFJEQ0didXFWUTdIYVFoU2VWa3hCZ3A2Vmpu?=
 =?utf-8?B?RUlXUDFIdEI2OHI1YUlUQUlBcm9zY1lQYnRZQUtEY0pVUWRpdERJWlJqQlB6?=
 =?utf-8?B?VnhGekRtSzlSL0RHczdIclYycHBkakg4Wkk4WTRBbUZOcWVxdnFRaVFFWXB5?=
 =?utf-8?B?MWVGMVJKSzgvNHp1VVdWUU1BYTN2U0pSSXd4cndyR2FTMEZqRzR5TGJmUTBR?=
 =?utf-8?B?ZExQLytyNHM5WEs0aGg5ZW0rQ2hHakk2T3VBRXR5cGlXendDWHpiYkpFQlZR?=
 =?utf-8?B?enJwWE40VUlFRXVTZFFaVTJwRkpQcTZ1UWJZblBWaE5jaVQ3MzdJWjlYZzFi?=
 =?utf-8?B?SUR0emlSU3R0dTE2cHdvOUs4UHFYcHN4dTgvUFRGQm1IQ0tFY1hVYlEwTU5F?=
 =?utf-8?B?YjNBZFlZRURkZGM3U2lLZlhGT0NaYjJ1SFV3VGpEalZlU1BMRCtxdFpIdSty?=
 =?utf-8?B?OW0zbmZaK2lKTDJwekpOMGJla3MvODRzbXdjejFMaVZyWll4eGdqN01EeEly?=
 =?utf-8?B?V2wwUWc3b2FuNUh5OE1jcmxLY1k1M1JpaXlrbVdqZFQ5RXJ5NXNVS3UwaThR?=
 =?utf-8?B?WmJla3hLTXc4dCtPalBHZklFN3lKdU54YUJubzJFTUI5OWF4eDZPVW4xQW84?=
 =?utf-8?B?QUdqWkkwU2pLbjdDMzNwVWFWRVZRNFBiMGpXV3RiSXYwQ2dVOEZLemlieEtO?=
 =?utf-8?B?bDd0dmU2S2hidUw5bVhVS2FUd0w3QVhPanNXdXJlV0N0ZmUrdERLZTVvMTFl?=
 =?utf-8?B?UEpiLzRocnNjNnJJYStLZHdzZkMyNnJ3UkVZTWIwS25kTDFsc3pUUE5SOVZv?=
 =?utf-8?B?ZVY3MjF2Vk1na0VDa2tmN0txR1duM1VuWEtrRGVRalc5cW9NWGxBTzhwb2Ix?=
 =?utf-8?B?eFdnaC8vWDFQdUFRVzdVOXhleHpZdEpsT0NoaTFnUERKSmowQ2NUN0ppeG9I?=
 =?utf-8?B?aXVycTZtOWZnS0J6aTI2KzIyM2JaZGlZTkF6TW1EVlNsMW9CTXNoOTBIeEkr?=
 =?utf-8?B?M3FCTmhBN3pLejJsUmt5WU1XTExZT0Vxd0JSQTNQck52bXVCRTNWSlNaV1lz?=
 =?utf-8?B?a1R5R2FXNFRlTEI0b3pTbzZHR1JkOE5LdEQ2ZGtBZkJuci9EZlIxUmdBUHJl?=
 =?utf-8?B?KytpcUVnVGJGR1ZtOE4rMjFRd3E0dDl4SmgxNE9JNllqcThsQ2FQSUs0N2ow?=
 =?utf-8?B?aTZmdUY2M0I3eG1VT3diSW5DL0hZK05PZHRlVzdUOEE1Q3c0UEpGdkl1Wnd0?=
 =?utf-8?B?ZDNYeVhrTE84ZTZIdDFtM21vZVVnOFRWUFNMUTZBVEUvOU01aGtlYU1lQy85?=
 =?utf-8?B?L2tjTlpvRXVRNHdKR0oyNDlBWmhLaTNvWjhxdksxT2RtcWdtbUQvL0ZIS0lS?=
 =?utf-8?B?UmxNT0JLOG44MGg5STB2bU9IeWFEdnhFRU10amVQZ2kzdVJ5bU9rY0UwdUVD?=
 =?utf-8?B?Nlk3TTZHbk5PUzlQQWFyZCtCbW9FUTRackFFZEZhcGpscHhOWVVLTFBROU1v?=
 =?utf-8?B?cGpRUS9wVzM3UjJZWW5CZ3VVd2hXK2NKb2srOTNmWUNSbkZTNXpyT0JSVFRE?=
 =?utf-8?B?citqWkJnUWI1eEZIYlpQMkpwZHRVWXhCaytYRzRJZ2s0d1QxVU95Mi95NXAv?=
 =?utf-8?B?NEtGbFZkWnpzdlp3b3dQRjdJSFRaZDJhOWV4MkgzVEJtdXVaNmI1b1JKcElK?=
 =?utf-8?B?ckFYbWhFTzJLV2NHNng3ZzN3S2NkVy83WU1EQWJid0k0b1NSV29MZmd0aHhG?=
 =?utf-8?B?eGJxbTI4d3dCK21aS3lxSVBuUVBtaWpETWlJRnMrNWZLeFBmUEsxZnlQcEtq?=
 =?utf-8?B?NlhSMVpUQ3dObEFMMitkT0pqMm82ZTVVSEE0MXZ0MG8vbXZ4MnRObjdmQmtX?=
 =?utf-8?B?UVdkaVJsNFhaU0Y0aW12V0NXUkhvYjBrZEpiRDUzbHZ2bStFYzFrMHhyQjFX?=
 =?utf-8?Q?Hv9ZDaoYXlNkptt97p0Rs3gCW?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 56cf62bf-d95b-4357-fdac-08dc1c3c1fc1
X-MS-Exchange-CrossTenant-AuthSource: PH0PR12MB7982.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jan 2024 17:52:53.7251
 (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: 4w76/zohv+Rj0+onkO/HHtEAbSAZcYV/1FqAEMnIP9mhs7TxS61+UXpiabK+YIsOv+OtXcm2ARCfCo91jUf7TA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR12MB6252

On 1/22/2024 5:13 PM, Cong Liu wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> the patch releases the region_info memory if the interval_tree_iter_first()
> function fails.
> 
> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
> ---
>   drivers/vfio/pci/pds/dirty.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/pci/pds/dirty.c b/drivers/vfio/pci/pds/dirty.c
> index 8ddf4346fcd5..67919b5db127 100644
> --- a/drivers/vfio/pci/pds/dirty.c
> +++ b/drivers/vfio/pci/pds/dirty.c
> @@ -291,8 +291,11 @@ static int pds_vfio_dirty_enable(struct pds_vfio_pci_device *pds_vfio,
>          len = num_ranges * sizeof(*region_info);
> 
>          node = interval_tree_iter_first(ranges, 0, ULONG_MAX);
> -       if (!node)
> -               return -EINVAL;
> +       if (!node) {
> +               err = -EINVAL;
> +               goto out_free_region_info;
> +       }
> +

LGTM! Thanks for fixing this.

Reviewed-by: <brett.creeley@amd.com>

>          for (int i = 0; i < num_ranges; i++) {
>                  struct pds_lm_dirty_region_info *ri = &region_info[i];
>                  u64 region_size = node->last - node->start + 1;
> --
> 2.34.1
>