Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <db39a1c0-45e5-7458-00c5-87ebd56f3894@intel.com>
Date:   Wed, 13 Jul 2022 01:54:41 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.11.0
Subject: Re: [PATCH] kunit: executor: Fix a memory leak on failure in
 kunit_filter_tests
Content-Language: en-US
To:     David Gow <davidgow@google.com>
CC:     <kunit-dev@googlegroups.com>, <linux-kselftest@vger.kernel.org>,
        "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
        <llvm@lists.linux.dev>, Shuah Khan <skhan@linuxfoundation.org>,
        Brendan Higgins <brendanhiggins@google.com>,
        Daniel Latypov <dlatypov@google.com>
References: <20220712095627.1770312-1-davidgow@google.com>
From:   Yujie Liu <yujie.liu@intel.com>
In-Reply-To: <20220712095627.1770312-1-davidgow@google.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Zm80UEFoNGhmTjgyeDNWQ2c1emdPUm1rL2Ivd0tXQ1gxOC91SFBCZlo2THBz?=
 =?utf-8?B?RHBDNWtGL1ZYdTNNVDFHeGRLYUZKZ1V4d0tLcmFCbHZJdVRieDVLbzBoeGtu?=
 =?utf-8?B?YU5Wb1JjZEFYekZQYmNoVnpCTFlLS25HOW1GRkkyU3NRQzY1ZVVrVXVUUExP?=
 =?utf-8?B?QU9KQ1hEZG5lTWJZWkRxOXJHdnNBcXNuVkZqNDc0L2w3Zm9RdGhBOW5aWFU0?=
 =?utf-8?B?NnJYTld2Zk9VVnJia0xFbGZBMllrbkkzUVN6akI2RGlvMjlKZWV6MXRUNUpx?=
 =?utf-8?B?VWQ5SEttZHErSDM0M2FuSVRFMVFsVEppWVFaVitiR1ZPbG9pWVJicjF2aW5Q?=
 =?utf-8?B?ZmVHMkVFTmk5azM2amlrRGEwSkIyeVJhREZrSlQ0K012a0VXMkN4cmswQk9h?=
 =?utf-8?B?TEs1bEE5bVRmbG94bWlia1QxRmJndHVsOVd4UGtDWWpNMWZYU2x6cWZwN2dR?=
 =?utf-8?B?RU5IQk5SUDRjL3kyZG4wZ0xlMk5YS2FPaTVqVGV6OHRBdW8xMGRZSzF2aWx0?=
 =?utf-8?B?K25oVW1JVnBrcFROM0VyQXpYVWQvd0xKU0t0V0RJaUg5TGFrQll2UGtMVk5u?=
 =?utf-8?B?QzdNS3U1MHA1UGZoNFIvL2lkMUdubllkU0JMNG5naHZxRGxaRUQxbzNGRTFy?=
 =?utf-8?B?eUdBaWFOaU1CdE94QUlFOG5IODdRaVQrZHE5Vll2ZU4xTEM4SVBkRnZXbkE3?=
 =?utf-8?B?NDZwcEFMQmFyZnA5UmxteHNKVDN5a0IwRXBGbDdNbGdyZndMVHFlOWtSYjdy?=
 =?utf-8?B?UmRLRUVWY3VkczZBdFB3ZDhCN09xNWEzdFdnam9QVFlTdFF3SXBUNk50Lzk2?=
 =?utf-8?B?Zm5iZjdGU3F4Y1BjSzRlaUN5Z1IreXRGUy83UjNWeHB1Vm8yZi9NRHg5YUtN?=
 =?utf-8?B?U0hXdFVKOTdQeldHdHFWQ01kMWdUZ2w4MTR6aFJrTGxpRzZvMlJjYWQ0S3J2?=
 =?utf-8?B?ajBxSU1SQk0ybkhCVFNMWnVJL3Y4cHNMSTBjRkp4OFJCUkF5TVk4dEd0ZEY4?=
 =?utf-8?B?WUkwYkE4clNaRzhGT3hneXR2WHJmdTA1L3ZNV3hIeVhDRHNnUDJ1bXBkL3gz?=
 =?utf-8?B?NzNVOGR3SlFRRFJZaVduT2RwSUQzblAySzVGRnZGOVNrellMQTNGRWdGcHIy?=
 =?utf-8?B?Kzg5bDJBN3VLM3NkNmd1MFFVTVMvR1FOekhuKzBNN1BYbitqMm5jU2JoM1d1?=
 =?utf-8?B?L25tQkFvVkJlay9pcGlvZUhqbnN4bkEwR0R5Z3IzMFNubzdERzJBWnBQUXF6?=
 =?utf-8?B?WWhTSy9wMEd3U0hZem1FcXY4bEMwWWczYmoxNk5EU2lpdjIvWWxvTmI1c015?=
 =?utf-8?B?Y3NYM0YwSE4zQld0UmRGOWlQZEdpeHhqbWwvbnhGSE9BVHV5TTJmckVkcXhl?=
 =?utf-8?B?Ry91QzhOOTljNUkvOXpOUnRhUG5CTytHejIwbzB6Z0U0elNPWXA3ZGs5R0k4?=
 =?utf-8?B?T2RtSDhZOER6dTJSZVNuMEtQQjVSdWtSbUpENld0cE5RY2F5SGZDTVdSYWEx?=
 =?utf-8?B?K2dibGU3UUVxWGpoMHhnS2dpcDFWcVlhVWZYd3M2MWdUclh3MXA2WmxkQ1hK?=
 =?utf-8?B?NFhueS90Y2RNYzVWM1c0Z2JTRTl3UUVjN0JSV25pbWtkZlpVNXJ6eDc3enZh?=
 =?utf-8?B?UFR0U0E3bUY3K1lZMWg3SWc3Vm9XUmIyRUZoMUJmWXFXMWhkc041RGdkOTlF?=
 =?utf-8?B?RHdZSVBSZkRmZ29sQUZ6bzZiRlZubjNYQ001ZS96ak1yUTVaV1Y0STNReTRG?=
 =?utf-8?B?dlBnd1R4MXJkKzE4T2dPazhsNVcvNW9vaWlQNkdLZnlmbG9CcmdXM2xmWlhv?=
 =?utf-8?B?TmdabExrREJ1eW9zbTZWS1BOUTRER2s1ckJINnArcE1JY09tUEhTQ2pFVWlQ?=
 =?utf-8?B?dGhQNGpEWm4zcjlRbDFQcHNMeUNoRUJZNUxvRjZlc0M2bUJzUTJBc3Nudmda?=
 =?utf-8?B?b3h4eXg0OEp4dmtNZy9WZ3JrYTFta3lkMUNDS1p4aXJQL1luUlo1Zm1XTG1y?=
 =?utf-8?B?QU1TUGFoc29JVHVPYjJtT3YzcDhTVS9KZ0ZTa3RPVHByOWFla2Z4ODRISSt5?=
 =?utf-8?B?VmhWTGQzQjNPemE1eXlGUjE1K2RTNlh3VnZWMGd0SlFHRmVMME1wZk5DZTBt?=
 =?utf-8?Q?HcX0eZcGNknta4c9+JNnpFt53?=
X-MS-Exchange-CrossTenant-Network-Message-Id: f44017d7-ec97-4c6e-27e4-08da642f9efd
X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB5933.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Jul 2022 17:54:52.3102
 (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: VcrYeGkjocRk5Q8RC5o7zGtJDcYH76r/TI/TuSSWwzXsO/etaM46LF61MCrgEC1Nu2QlaOCyx67UFudESx9aVA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO6PR11MB5587
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi David,

On 7/12/2022 17:56, David Gow wrote:
> It's possible that memory allocation for the copy will fail, but for the

I think it is "for the _filtered_ will fail". Maybe a typo?

Thanks,
Yujie

> copy of the suite to succeed. In this case, the copy could be leaked.
> 
> Properly free 'copy' in the error case for the allocation of 'filtered'
> failing.
> 
> Note that there may also have been a similar issue in
> kunit_filter_subsuites, before it was removed in "kunit: flatten
> kunit_suite*** to kunit_suite** in .kunit_test_suites".
> 
> This was reported by clang-analyzer via the kernel test robot, here:
> https://lore.kernel.org/all/c8073b8e-7b9e-0830-4177-87c12f16349c@intel.com/
> 
> Fixes: a02353f49162 ("kunit: bail out of test filtering logic quicker if OOM")
> Reported-by: kernel test robot <yujie.liu@intel.com>
> Signed-off-by: David Gow <davidgow@google.com>
> ---
>   lib/kunit/executor.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> index 6c489d6c5e5d..5e223327196a 100644
> --- a/lib/kunit/executor.c
> +++ b/lib/kunit/executor.c
> @@ -74,8 +74,10 @@ kunit_filter_tests(const struct kunit_suite *const suite, const char *test_glob)
>   		return ERR_PTR(-ENOMEM);
>   
>   	filtered = kcalloc(n + 1, sizeof(*filtered), GFP_KERNEL);
> -	if (!filtered)
> +	if (!filtered) {
> +		kfree(copy);
>   		return ERR_PTR(-ENOMEM);
> +	}
>   
>   	n = 0;
>   	kunit_suite_for_each_test_case(suite, test_case) {