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

[SeongJae Park <sj@kernel.org>]

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

[...]