Re: [PATCH] mm/zswap: fix error pointer free in zswap_cpu_comp_prepare()

[Pavel Butsykin <pbutsykin@cloudlinux.com>]

On 1/1/26 04:32, SeongJae Park wrote:
> On Wed, 31 Dec 2025 11:46:38 +0400 Pavel Butsykin <pbutsykin@cloudlinux.com> wrote:
> 
>> crypto_alloc_acomp_node() may return ERR_PTR(), but the fail path checks
>> only for NULL and can pass an error pointer to crypto_free_acomp().
>> Use IS_ERR_OR_NULL() to only free valid acomp instances.
>>
>> Fixes: 779b9955f643 ("mm: zswap: move allocations during CPU init outside the lock")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Pavel Butsykin <pbutsykin@cloudlinux.com>
>> ---
>>   mm/zswap.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/zswap.c b/mm/zswap.c
>> index 5d0f8b13a958..ac9b7a60736b 100644
>> --- a/mm/zswap.c
>> +++ b/mm/zswap.c
>> @@ -787,7 +787,7 @@ static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
>>   	return 0;
>>   
>>   fail:
>> -	if (acomp)
>> +	if (!IS_ERR_OR_NULL(acomp))
>>   		crypto_free_acomp(acomp);
>>   	kfree(buffer);
>>   	return ret;
> 
> I understand you are keeping NULL case to keep the old behavior.  But, seems
> the case cannot happen to me for following reasons.
> 
> First of all, the old NULL check was only for crypto_alloc_acomp_node()
> failure.  But crypto_alloc_acomp_node() seems not returning NULL, to by breif
> look of the code.  And the failure check of crypto_alloc_acomp_node() is
> actually doing only IS_ERR() check.
> 
> So, it seems IS_ERR() here is enough.  Or, if I missed a case that
> crypto_alloc_acomp_node() returns NULL, the above crypto_alloc_acomp_node()
> failure check should be updated to use IS_ERR_OR_NULL()?
> 

We have 'goto fail;' right before crypto_alloc_acomp_node() for the case 
where kmalloc_node fails to allocate memory. In that case, 'acomp' will 
still be initialized to NULL.