linux-kernel - Re: [PATCH] mm/zswap: fix error pointer free in zswap_cpu_comp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <baefc4db-2e2f-4b20-b44c-eeaadfaa1c1e@cloudlinux.com>
Date: Fri, 2 Jan 2026 10:51:01 +0400
From: Pavel Butsykin <pbutsykin@...udlinux.com>
To: SeongJae Park <sj@...nel.org>
Cc: hannes@...xchg.org, yosry.ahmed@...ux.dev, nphamcs@...il.com,
 chengming.zhou@...ux.dev, akpm@...ux-foundation.org, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] mm/zswap: fix error pointer free in
 zswap_cpu_comp_prepare()

On 1/1/26 04:32, SeongJae Park wrote:
> On Wed, 31 Dec 2025 11:46:38 +0400 Pavel Butsykin <pbutsykin@...udlinux.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@...r.kernel.org
>> Signed-off-by: Pavel Butsykin <pbutsykin@...udlinux.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.