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

From: Coly Li
Date: Fri Nov 24 2023 - 11:29:42 EST




> 2023年11月24日 23:14,Markus Weippert <markus@xxxxxxxxxxxx> 写道:
>
> Commit 028ddcac477b ("bcache: Remove unnecessary NULL point check in
> node allocations") replaced IS_ERR_OR_NULL by IS_ERR. This leads to a
> NULL pointer dereference.
>
> BUG: kernel NULL pointer dereference, address: 0000000000000080
> Call Trace:
> ? __die_body.cold+0x1a/0x1f
> ? page_fault_oops+0xd2/0x2b0
> ? exc_page_fault+0x70/0x170
> ? asm_exc_page_fault+0x22/0x30
> ? btree_node_free+0xf/0x160 [bcache]
> ? up_write+0x32/0x60
> btree_gc_coalesce+0x2aa/0x890 [bcache]
> ? bch_extent_bad+0x70/0x170 [bcache]
> btree_gc_recurse+0x130/0x390 [bcache]
> ? btree_gc_mark_node+0x72/0x230 [bcache]
> bch_btree_gc+0x5da/0x600 [bcache]
> ? cpuusage_read+0x10/0x10
> ? bch_btree_gc+0x600/0x600 [bcache]
> bch_gc_thread+0x135/0x180 [bcache]
>
> The relevant code starts with:
>
> new_nodes[0] = NULL;
>
> for (i = 0; i < nodes; i++) {
> if (__bch_keylist_realloc(&keylist, bkey_u64s(&r[i].b->key)))
> goto out_nocoalesce;
> // ...
> out_nocoalesce:
> // ...
> for (i = 0; i < nodes; i++)
> if (!IS_ERR(new_nodes[i])) { // IS_ERR_OR_NULL before
> 028ddcac477b
> btree_node_free(new_nodes[i]); // new_nodes[0] is NULL
> rw_unlock(true, new_nodes[i]);
> }
>
> This patch replaces IS_ERR() by IS_ERR_OR_NULL() to fix this.
>
> Fixes: 028ddcac477b ("bcache: Remove unnecessary NULL point check in
> node allocations")
> Link:
> https://lore.kernel.org/all/3DF4A87A-2AC1-4893-AE5F-E921478419A9@xxxxxxx/
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Zheng Wang <zyytlz.wz@xxxxxxx>
> Cc: Coly Li <colyli@xxxxxxx>
> Signed-off-by: Markus Weippert <markus@xxxxxxxxxxxx>

Added into my for-next. Thanks for patching up.

Coly Li


>
> ---
> drivers/md/bcache/btree.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
> index de3019972..261596791 100644
> --- a/drivers/md/bcache/btree.c
> +++ b/drivers/md/bcache/btree.c
> @@ -1522,7 +1522,7 @@ static int btree_gc_coalesce(struct btree *b,
> struct btree_op *op,
> bch_keylist_free(&keylist);
>
> for (i = 0; i < nodes; i++)
> - if (!IS_ERR(new_nodes[i])) {
> + if (!IS_ERR_OR_NULL(new_nodes[i])) {
> btree_node_free(new_nodes[i]);
> rw_unlock(true, new_nodes[i]);
> }
> --