* [PATCH] bcache: Fix __bch_btree_node_alloc to make the failure behavior consistent
@ 2023-02-02 15:15 Zheng Wang
2023-02-02 16:23 ` Coly Li
2023-02-02 20:37 ` Eric Wheeler
0 siblings, 2 replies; 5+ messages in thread
From: Zheng Wang @ 2023-02-02 15:15 UTC (permalink / raw)
To: colyli
Cc: hackerzheng666, kent.overstreet, linux-bcache, linux-kernel,
alex000young, Zheng Wang
In some specific situation, the return value of __bch_btree_node_alloc may
be NULL. This may lead to poential NULL pointer dereference in caller
function like a calling chaion :
btree_split->bch_btree_node_alloc->__bch_btree_node_alloc.
Fix it by initialize return value in __bch_btree_node_alloc before return.
Fixes: cafe56359144 ("bcache: A block layer cache")
Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
---
drivers/md/bcache/btree.c | 10 ++++++----
drivers/md/bcache/super.c | 2 +-
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 147c493a989a..35346c1d7c3c 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -1090,10 +1090,12 @@ struct btree *__bch_btree_node_alloc(struct cache_set *c, struct btree_op *op,
struct btree *parent)
{
BKEY_PADDED(key) k;
- struct btree *b = ERR_PTR(-EAGAIN);
+ struct btree *b;
mutex_lock(&c->bucket_lock);
retry:
+ /* return ERR_PTR(-EAGAIN) when it fails */
+ b = ERR_PTR(-EAGAIN);
if (__bch_bucket_alloc_set(c, RESERVE_BTREE, &k.key, wait))
goto err;
@@ -1138,7 +1140,7 @@ static struct btree *btree_node_alloc_replacement(struct btree *b,
{
struct btree *n = bch_btree_node_alloc(b->c, op, b->level, b->parent);
- if (!IS_ERR_OR_NULL(n)) {
+ if (!IS_ERR(n)) {
mutex_lock(&n->write_lock);
bch_btree_sort_into(&b->keys, &n->keys, &b->c->sort);
bkey_copy_key(&n->key, &b->key);
@@ -1352,7 +1354,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
for (i = 0; i < nodes; i++) {
new_nodes[i] = btree_node_alloc_replacement(r[i].b, NULL);
- if (IS_ERR_OR_NULL(new_nodes[i]))
+ if (IS_ERR(new_nodes[i]))
goto out_nocoalesce;
}
@@ -1669,7 +1671,7 @@ static int bch_btree_gc_root(struct btree *b, struct btree_op *op,
if (should_rewrite) {
n = btree_node_alloc_replacement(b, NULL);
- if (!IS_ERR_OR_NULL(n)) {
+ if (!IS_ERR(n)) {
bch_btree_node_write_sync(n);
bch_btree_set_root(n);
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index ba3909bb6bea..92de714fe75e 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -2088,7 +2088,7 @@ static int run_cache_set(struct cache_set *c)
err = "cannot allocate new btree root";
c->root = __bch_btree_node_alloc(c, NULL, 0, true, NULL);
- if (IS_ERR_OR_NULL(c->root))
+ if (IS_ERR(c->root))
goto err;
mutex_lock(&c->root->write_lock);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] bcache: Fix __bch_btree_node_alloc to make the failure behavior consistent
2023-02-02 15:15 [PATCH] bcache: Fix __bch_btree_node_alloc to make the failure behavior consistent Zheng Wang
@ 2023-02-02 16:23 ` Coly Li
2023-02-02 17:19 ` Zheng Hacker
2023-02-02 20:37 ` Eric Wheeler
1 sibling, 1 reply; 5+ messages in thread
From: Coly Li @ 2023-02-02 16:23 UTC (permalink / raw)
To: Zheng Wang
Cc: Zheng Hacker, Kent Overstreet, linux-bcache, LKML, alex000young
> 2023年2月2日 23:15,Zheng Wang <zyytlz.wz@163.com> 写道:
>
> In some specific situation, the return value of __bch_btree_node_alloc may
> be NULL. This may lead to poential NULL pointer dereference in caller
> function like a calling chaion :
> btree_split->bch_btree_node_alloc->__bch_btree_node_alloc.
>
> Fix it by initialize return value in __bch_btree_node_alloc before return.
>
> Fixes: cafe56359144 ("bcache: A block layer cache")
> Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
> ---
> drivers/md/bcache/btree.c | 10 ++++++----
> drivers/md/bcache/super.c | 2 +-
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
> index 147c493a989a..35346c1d7c3c 100644
> --- a/drivers/md/bcache/btree.c
> +++ b/drivers/md/bcache/btree.c
> @@ -1090,10 +1090,12 @@ struct btree *__bch_btree_node_alloc(struct cache_set *c, struct btree_op *op,
> struct btree *parent)
> {
> BKEY_PADDED(key) k;
> - struct btree *b = ERR_PTR(-EAGAIN);
> + struct btree *b;
>
> mutex_lock(&c->bucket_lock);
> retry:
> + /* return ERR_PTR(-EAGAIN) when it fails */
> + b = ERR_PTR(-EAGAIN);
> if (__bch_bucket_alloc_set(c, RESERVE_BTREE, &k.key, wait))
> goto err;
>
The above change can be a single patch.
And the rested change can be another cleanup patch.
> @@ -1138,7 +1140,7 @@ static struct btree *btree_node_alloc_replacement(struct btree *b,
> {
> struct btree *n = bch_btree_node_alloc(b->c, op, b->level, b->parent);
>
> - if (!IS_ERR_OR_NULL(n)) {
> + if (!IS_ERR(n)) {
> mutex_lock(&n->write_lock);
> bch_btree_sort_into(&b->keys, &n->keys, &b->c->sort);
> bkey_copy_key(&n->key, &b->key);
> @@ -1352,7 +1354,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
>
> for (i = 0; i < nodes; i++) {
> new_nodes[i] = btree_node_alloc_replacement(r[i].b, NULL);
> - if (IS_ERR_OR_NULL(new_nodes[i]))
> + if (IS_ERR(new_nodes[i]))
> goto out_nocoalesce;
> }
>
> @@ -1669,7 +1671,7 @@ static int bch_btree_gc_root(struct btree *b, struct btree_op *op,
> if (should_rewrite) {
> n = btree_node_alloc_replacement(b, NULL);
>
> - if (!IS_ERR_OR_NULL(n)) {
> + if (!IS_ERR(n)) {
> bch_btree_node_write_sync(n);
>
> bch_btree_set_root(n);
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index ba3909bb6bea..92de714fe75e 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -2088,7 +2088,7 @@ static int run_cache_set(struct cache_set *c)
>
> err = "cannot allocate new btree root";
> c->root = __bch_btree_node_alloc(c, NULL, 0, true, NULL);
> - if (IS_ERR_OR_NULL(c->root))
> + if (IS_ERR(c->root))
> goto err;
>
> mutex_lock(&c->root->write_lock);
And there will be 2 patches as I suggested.
Thanks.
Coly Li
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] bcache: Fix __bch_btree_node_alloc to make the failure behavior consistent
2023-02-02 16:23 ` Coly Li
@ 2023-02-02 17:19 ` Zheng Hacker
0 siblings, 0 replies; 5+ messages in thread
From: Zheng Hacker @ 2023-02-02 17:19 UTC (permalink / raw)
To: Coly Li; +Cc: Zheng Wang, Kent Overstreet, linux-bcache, LKML, alex000young
Coly Li <colyli@suse.de> 于2023年2月3日周五 00:23写道:
>
>
>
> > 2023年2月2日 23:15,Zheng Wang <zyytlz.wz@163.com> 写道:
> >
> > In some specific situation, the return value of __bch_btree_node_alloc may
> > be NULL. This may lead to poential NULL pointer dereference in caller
> > function like a calling chaion :
> > btree_split->bch_btree_node_alloc->__bch_btree_node_alloc.
> >
> > Fix it by initialize return value in __bch_btree_node_alloc before return.
> >
> > Fixes: cafe56359144 ("bcache: A block layer cache")
> > Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
> > ---
> > drivers/md/bcache/btree.c | 10 ++++++----
> > drivers/md/bcache/super.c | 2 +-
> > 2 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
> > index 147c493a989a..35346c1d7c3c 100644
> > --- a/drivers/md/bcache/btree.c
> > +++ b/drivers/md/bcache/btree.c
> > @@ -1090,10 +1090,12 @@ struct btree *__bch_btree_node_alloc(struct cache_set *c, struct btree_op *op,
> > struct btree *parent)
> > {
> > BKEY_PADDED(key) k;
> > - struct btree *b = ERR_PTR(-EAGAIN);
> > + struct btree *b;
> >
> > mutex_lock(&c->bucket_lock);
> > retry:
> > + /* return ERR_PTR(-EAGAIN) when it fails */
> > + b = ERR_PTR(-EAGAIN);
> > if (__bch_bucket_alloc_set(c, RESERVE_BTREE, &k.key, wait))
> > goto err;
> >
>
>
> The above change can be a single patch.
>
> And the rested change can be another cleanup patch.
>
> > @@ -1138,7 +1140,7 @@ static struct btree *btree_node_alloc_replacement(struct btree *b,
> > {
> > struct btree *n = bch_btree_node_alloc(b->c, op, b->level, b->parent);
> >
> > - if (!IS_ERR_OR_NULL(n)) {
> > + if (!IS_ERR(n)) {
> > mutex_lock(&n->write_lock);
> > bch_btree_sort_into(&b->keys, &n->keys, &b->c->sort);
> > bkey_copy_key(&n->key, &b->key);
> > @@ -1352,7 +1354,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
> >
> > for (i = 0; i < nodes; i++) {
> > new_nodes[i] = btree_node_alloc_replacement(r[i].b, NULL);
> > - if (IS_ERR_OR_NULL(new_nodes[i]))
> > + if (IS_ERR(new_nodes[i]))
> > goto out_nocoalesce;
> > }
> >
> > @@ -1669,7 +1671,7 @@ static int bch_btree_gc_root(struct btree *b, struct btree_op *op,
> > if (should_rewrite) {
> > n = btree_node_alloc_replacement(b, NULL);
> >
> > - if (!IS_ERR_OR_NULL(n)) {
> > + if (!IS_ERR(n)) {
> > bch_btree_node_write_sync(n);
> >
> > bch_btree_set_root(n);
> > diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> > index ba3909bb6bea..92de714fe75e 100644
> > --- a/drivers/md/bcache/super.c
> > +++ b/drivers/md/bcache/super.c
> > @@ -2088,7 +2088,7 @@ static int run_cache_set(struct cache_set *c)
> >
> > err = "cannot allocate new btree root";
> > c->root = __bch_btree_node_alloc(c, NULL, 0, true, NULL);
> > - if (IS_ERR_OR_NULL(c->root))
> > + if (IS_ERR(c->root))
> > goto err;
> >
> > mutex_lock(&c->root->write_lock);
>
> And there will be 2 patches as I suggested.
>
Got it. It's a little bit late now. I might make that tomorrow ^_^
Best regards,
Zheng Wang
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] bcache: Fix __bch_btree_node_alloc to make the failure behavior consistent
2023-02-02 15:15 [PATCH] bcache: Fix __bch_btree_node_alloc to make the failure behavior consistent Zheng Wang
2023-02-02 16:23 ` Coly Li
@ 2023-02-02 20:37 ` Eric Wheeler
2023-02-03 2:08 ` Zheng Hacker
1 sibling, 1 reply; 5+ messages in thread
From: Eric Wheeler @ 2023-02-02 20:37 UTC (permalink / raw)
To: Zheng Wang
Cc: colyli, hackerzheng666, kent.overstreet, linux-bcache,
linux-kernel, alex000young
On Thu, 2 Feb 2023, Zheng Wang wrote:
> In some specific situation, the return value of __bch_btree_node_alloc may
> be NULL. This may lead to poential NULL pointer dereference in caller
> function like a calling chaion :
> btree_split->bch_btree_node_alloc->__bch_btree_node_alloc.
>
> Fix it by initialize return value in __bch_btree_node_alloc before return.
>
> Fixes: cafe56359144 ("bcache: A block layer cache")
> Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
Be sure to add `Cc: stable@vger.kernel.org`
-Eric
> ---
> drivers/md/bcache/btree.c | 10 ++++++----
> drivers/md/bcache/super.c | 2 +-
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
> index 147c493a989a..35346c1d7c3c 100644
> --- a/drivers/md/bcache/btree.c
> +++ b/drivers/md/bcache/btree.c
> @@ -1090,10 +1090,12 @@ struct btree *__bch_btree_node_alloc(struct cache_set *c, struct btree_op *op,
> struct btree *parent)
> {
> BKEY_PADDED(key) k;
> - struct btree *b = ERR_PTR(-EAGAIN);
> + struct btree *b;
>
> mutex_lock(&c->bucket_lock);
> retry:
> + /* return ERR_PTR(-EAGAIN) when it fails */
> + b = ERR_PTR(-EAGAIN);
> if (__bch_bucket_alloc_set(c, RESERVE_BTREE, &k.key, wait))
> goto err;
>
> @@ -1138,7 +1140,7 @@ static struct btree *btree_node_alloc_replacement(struct btree *b,
> {
> struct btree *n = bch_btree_node_alloc(b->c, op, b->level, b->parent);
>
> - if (!IS_ERR_OR_NULL(n)) {
> + if (!IS_ERR(n)) {
> mutex_lock(&n->write_lock);
> bch_btree_sort_into(&b->keys, &n->keys, &b->c->sort);
> bkey_copy_key(&n->key, &b->key);
> @@ -1352,7 +1354,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
>
> for (i = 0; i < nodes; i++) {
> new_nodes[i] = btree_node_alloc_replacement(r[i].b, NULL);
> - if (IS_ERR_OR_NULL(new_nodes[i]))
> + if (IS_ERR(new_nodes[i]))
> goto out_nocoalesce;
> }
>
> @@ -1669,7 +1671,7 @@ static int bch_btree_gc_root(struct btree *b, struct btree_op *op,
> if (should_rewrite) {
> n = btree_node_alloc_replacement(b, NULL);
>
> - if (!IS_ERR_OR_NULL(n)) {
> + if (!IS_ERR(n)) {
> bch_btree_node_write_sync(n);
>
> bch_btree_set_root(n);
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index ba3909bb6bea..92de714fe75e 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -2088,7 +2088,7 @@ static int run_cache_set(struct cache_set *c)
>
> err = "cannot allocate new btree root";
> c->root = __bch_btree_node_alloc(c, NULL, 0, true, NULL);
> - if (IS_ERR_OR_NULL(c->root))
> + if (IS_ERR(c->root))
> goto err;
>
> mutex_lock(&c->root->write_lock);
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] bcache: Fix __bch_btree_node_alloc to make the failure behavior consistent
2023-02-02 20:37 ` Eric Wheeler
@ 2023-02-03 2:08 ` Zheng Hacker
0 siblings, 0 replies; 5+ messages in thread
From: Zheng Hacker @ 2023-02-03 2:08 UTC (permalink / raw)
To: Eric Wheeler
Cc: Zheng Wang, colyli, kent.overstreet, linux-bcache, linux-kernel,
alex000young
Eric Wheeler <bcache@lists.ewheeler.net> 于2023年2月3日周五 04:37写道:
>
> On Thu, 2 Feb 2023, Zheng Wang wrote:
>
> > In some specific situation, the return value of __bch_btree_node_alloc may
> > be NULL. This may lead to poential NULL pointer dereference in caller
> > function like a calling chaion :
> > btree_split->bch_btree_node_alloc->__bch_btree_node_alloc.
> >
> > Fix it by initialize return value in __bch_btree_node_alloc before return.
> >
> > Fixes: cafe56359144 ("bcache: A block layer cache")
> > Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
>
> Be sure to add `Cc: stable@vger.kernel.org`
>
> -Eric
>
Thanks for your kind reminding. Will do in the next patch.
Best regards,
Zheng Wang
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-02-03 2:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-02 15:15 [PATCH] bcache: Fix __bch_btree_node_alloc to make the failure behavior consistent Zheng Wang
2023-02-02 16:23 ` Coly Li
2023-02-02 17:19 ` Zheng Hacker
2023-02-02 20:37 ` Eric Wheeler
2023-02-03 2:08 ` Zheng Hacker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).