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;
-- 
2.52.0

On Tue, Dec 30, 2025 at 11:46 PM 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>

Acked-by: Nhat Pham <nphamcs@gmail.com>

On Wed, Dec 31, 2025 at 11:46:38AM +0400, Pavel Butsykin 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>

Thanks for the fix!

Acked-by: Yosry Ahmed <yosry.ahmed@linux.dev>

> ---
>  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;
> -- 
> 2.52.0
>

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()?


Thanks,
SJ

[...]

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.

On Fri, 2 Jan 2026 10:51:01 +0400 Pavel Butsykin <pbutsykin@cloudlinux.com> wrote:

> 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.

Ah, you are right.  Thank you for fixing this.  Please feel free to add

Reviewed-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ

[...]