bcache: revert replacing IS_ERR_OR_NULL with IS_ERR again

[PATCH V3] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR again

Posted by liequan che 1 year, 2 months ago

Commit 028ddcac477b ("bcache: Remove unnecessary NULL point check in
node allocations") leads a NULL pointer deference in cache_set_flush().

1721         if (!IS_ERR_OR_NULL(c->root))
1722                 list_add(&c->root->list, &c->btree_cache);

From the above code in cache_set_flush(), if previous registration code
fails before allocating c->root, it is possible c->root is NULL as what
it is initialized. Also __bch_btree_node_alloc() never returns NULL but
c->root is possible to be NULL at above line 1721.

This patch replaces IS_ERR() by IS_ERR_OR_NULL() to fix this.

Fixes: 028ddcac477b ("bcache: Remove unnecessary NULL point check in
node allocations")
Signed-off-by: Liequan Che <cheliequan@inspur.com>
Cc: stable@vger.kernel.org
Cc: Zheng Wang <zyytlz.wz@163.com>
Cc: Coly Li <colyli@suse.de>
---
 drivers/md/bcache/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index e7abfdd77c3b..e42f1400cea9 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1718,7 +1718,7 @@ static CLOSURE_CALLBACK(cache_set_flush)
        if (!IS_ERR_OR_NULL(c->gc_thread))
                kthread_stop(c->gc_thread);

-       if (!IS_ERR(c->root))
+       if (!IS_ERR_OR_NULL(c->root))
                list_add(&c->root->list, &c->btree_cache);

        /*
--
2.46.0

Re: [PATCH V3] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR again

Posted by Coly Li 1 year, 2 months ago


> 2024年11月27日 11:01，liequan che <liequanche@gmail.com> 写道：
> 
> Commit 028ddcac477b ("bcache: Remove unnecessary NULL point check in
> node allocations") leads a NULL pointer deference in cache_set_flush().
> 
> 1721         if (!IS_ERR_OR_NULL(c->root))
> 1722                 list_add(&c->root->list, &c->btree_cache);
> 
> From the above code in cache_set_flush(), if previous registration code
> fails before allocating c->root, it is possible c->root is NULL as what
> it is initialized. Also __bch_btree_node_alloc() never returns NULL but
> c->root is possible to be NULL at above line 1721.
> 
> This patch replaces IS_ERR() by IS_ERR_OR_NULL() to fix this.


OK, this time the commit log makes a lot sense. It is clear to me.


> 
> Fixes: 028ddcac477b ("bcache: Remove unnecessary NULL point check in
> node allocations")
> Signed-off-by: Liequan Che <cheliequan@inspur.com>
> Cc: stable@vger.kernel.org
> Cc: Zheng Wang <zyytlz.wz@163.com>
> Cc: Coly Li <colyli@suse.de>
> ---
> drivers/md/bcache/super.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index e7abfdd77c3b..e42f1400cea9 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -1718,7 +1718,7 @@ static CLOSURE_CALLBACK(cache_set_flush)
>        if (!IS_ERR_OR_NULL(c->gc_thread))
>                kthread_stop(c->gc_thread);
> 
> -       if (!IS_ERR(c->root))
> +       if (!IS_ERR_OR_NULL(c->root))
>                list_add(&c->root->list, &c->btree_cache);
> 
>        /*
> —
> 2.46.0

It is fine to me. I am in travel these days, and will handle this patch by end of this week.

Thanks for composing this patch.

Coly Li

Re: [PATCH V3] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR again

Posted by 邹明哲 1 year, 2 months ago

LGTM

reviewed by Mingzhe Zou

Original:
From：Coly Li <colyli@suse.de>
Date：2024-11-27 11:17:01(中国 (GMT+08:00))
To：liequan che <liequanche@gmail.com>
Cc：mingzhe.zou@easystack.cn<mingzhe.zou@easystack.cn> , Kent Overstreet <kent.overstreet@gmail.com> , linux-bcache <linux-bcache@vger.kernel.org> , linux-kernel <linux-kernel@vger.kernel.org>
Subject：Re: [PATCH V3] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR again
&gt; 2024年11月27日 11:01，liequan che <liequanche@gmail.com> 写道：
&gt; 
&gt; Commit 028ddcac477b ("bcache: Remove unnecessary NULL point check in
&gt; node allocations") leads a NULL pointer deference in cache_set_flush().
&gt; 
&gt; 1721         if (!IS_ERR_OR_NULL(c-&gt;root))
&gt; 1722                 list_add(&amp;c-&gt;root-&gt;list, &amp;c-&gt;btree_cache);
&gt; 
&gt; From the above code in cache_set_flush(), if previous registration code
&gt; fails before allocating c-&gt;root, it is possible c-&gt;root is NULL as what
&gt; it is initialized. Also __bch_btree_node_alloc() never returns NULL but
&gt; c-&gt;root is possible to be NULL at above line 1721.
&gt; 
&gt; This patch replaces IS_ERR() by IS_ERR_OR_NULL() to fix this.


OK, this time the commit log makes a lot sense. It is clear to me.


&gt; 
&gt; Fixes: 028ddcac477b ("bcache: Remove unnecessary NULL point check in
&gt; node allocations")
&gt; Signed-off-by: Liequan Che <cheliequan@inspur.com>
&gt; Cc: stable@vger.kernel.org
&gt; Cc: Zheng Wang <zyytlz.wz@163.com>
&gt; Cc: Coly Li <colyli@suse.de>
&gt; ---
&gt; drivers/md/bcache/super.c | 2 +-
&gt; 1 file changed, 1 insertion(+), 1 deletion(-)
&gt; 
&gt; diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
&gt; index e7abfdd77c3b..e42f1400cea9 100644
&gt; --- a/drivers/md/bcache/super.c
&gt; +++ b/drivers/md/bcache/super.c
&gt; @@ -1718,7 +1718,7 @@ static CLOSURE_CALLBACK(cache_set_flush)
&gt;        if (!IS_ERR_OR_NULL(c-&gt;gc_thread))
&gt;                kthread_stop(c-&gt;gc_thread);
&gt; 
&gt; -       if (!IS_ERR(c-&gt;root))
&gt; +       if (!IS_ERR_OR_NULL(c-&gt;root))
&gt;                list_add(&amp;c-&gt;root-&gt;list, &amp;c-&gt;btree_cache);
&gt; 
&gt;        /*
&gt; —
&gt; 2.46.0

It is fine to me. I am in travel these days, and will handle this patch by end of this week.

Thanks for composing this patch.

Coly Li
</colyli@suse.de></zyytlz.wz@163.com></cheliequan@inspur.com></liequanche@gmail.com></linux-kernel@vger.kernel.org></linux-bcache@vger.kernel.org></kent.overstreet@gmail.com></mingzhe.zou@easystack.cn></liequanche@gmail.com></colyli@suse.de>