[PATCH] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR

Markus Weippert posted 1 patch 2 years ago
drivers/md/bcache/btree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR
Posted by Markus Weippert 2 years ago
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@suse.de/
Cc: stable@vger.kernel.org
Cc: Zheng Wang <zyytlz.wz@163.com>
Cc: Coly Li <colyli@suse.de>
Signed-off-by: Markus Weippert <markus@gekmihesg.de>

---
 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]);
                }
--
Re: [PATCH] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR
Posted by Jens Axboe 2 years ago
On Fri, 24 Nov 2023 16:14:37 +0100, Markus Weippert wrote:
> 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]
> 
> [...]

Applied, thanks!

[1/1] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR
      (no commit info)

Best regards,
-- 
Jens Axboe
Re: [PATCH] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR
Posted by Coly Li 2 years ago

> 2023年11月24日 23:14,Markus Weippert <markus@gekmihesg.de> 写道:
> 
> 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@suse.de/
> Cc: stable@vger.kernel.org
> Cc: Zheng Wang <zyytlz.wz@163.com>
> Cc: Coly Li <colyli@suse.de>
> Signed-off-by: Markus Weippert <markus@gekmihesg.de>

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]);
>                }
> --
Re: [PATCH] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR
Posted by Jens Axboe 2 years ago
On 11/24/23 9:29 AM, Coly Li wrote:
> 
> 
>> 2023?11?24? 23:14?Markus Weippert <markus@gekmihesg.de> ???
>>
>> 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@suse.de/
>> Cc: stable@vger.kernel.org
>> Cc: Zheng Wang <zyytlz.wz@163.com>
>> Cc: Coly Li <colyli@suse.de>
>> Signed-off-by: Markus Weippert <markus@gekmihesg.de>
> 
> Added into my for-next.  Thanks for patching up.

We should probably get this into the current release, rather than punt
it to 6.8.

-- 
Jens Axboe
Re: [PATCH] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR
Posted by Coly Li 2 years ago

> 2023年11月25日 00:31,Jens Axboe <axboe@kernel.dk> 写道:
> 
> On 11/24/23 9:29 AM, Coly Li wrote:
>> 
>> 
>>> 2023?11?24? 23:14?Markus Weippert <markus@gekmihesg.de> ???
>>> 
>>> 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@suse.de/
>>> Cc: stable@vger.kernel.org
>>> Cc: Zheng Wang <zyytlz.wz@163.com>
>>> Cc: Coly Li <colyli@suse.de>
>>> Signed-off-by: Markus Weippert <markus@gekmihesg.de>
>> 
>> Added into my for-next.  Thanks for patching up.
> 
> We should probably get this into the current release, rather than punt
> it to 6.8.

Yes, copied. So far I don’t have other bcache patches for 6.7, I feel I might be redundant if I send you another for -rc4 series with this single patch.

Could you please directly take it into -rc4?

Thanks.

Coly Li
Re: [PATCH] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR
Posted by Jens Axboe 2 years ago
On 11/24/23 9:34 AM, Coly Li wrote:
> 
> 
>> 2023?11?25? 00:31?Jens Axboe <axboe@kernel.dk> ???
>>
>> On 11/24/23 9:29 AM, Coly Li wrote:
>>>
>>>
>>>> 2023?11?24? 23:14?Markus Weippert <markus@gekmihesg.de> ???
>>>>
>>>> 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@suse.de/
>>>> Cc: stable@vger.kernel.org
>>>> Cc: Zheng Wang <zyytlz.wz@163.com>
>>>> Cc: Coly Li <colyli@suse.de>
>>>> Signed-off-by: Markus Weippert <markus@gekmihesg.de>
>>>
>>> Added into my for-next.  Thanks for patching up.
>>
>> We should probably get this into the current release, rather than punt
>> it to 6.8.
> 
> Yes, copied. So far I don?t have other bcache patches for 6.7, I feel
> I might be redundant if I send you another for -rc4 series with this
> single patch.
> 
> Could you please directly take it into -rc4?

Sure, I'll just grab it as-is.

-- 
Jens Axboe
Re: [PATCH] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR
Posted by Coly Li 2 years ago

> 2023年11月25日 00:35,Jens Axboe <axboe@kernel.dk> 写道:
> 
> On 11/24/23 9:34 AM, Coly Li wrote:
>> 
>> 
>>> 2023?11?25? 00:31?Jens Axboe <axboe@kernel.dk> ???
>>> 
>>> On 11/24/23 9:29 AM, Coly Li wrote:
>>>> 
>>>> 
>>>>> 2023?11?24? 23:14?Markus Weippert <markus@gekmihesg.de> ???
>>>>> 
>>>>> 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@suse.de/
>>>>> Cc: stable@vger.kernel.org
>>>>> Cc: Zheng Wang <zyytlz.wz@163.com>
>>>>> Cc: Coly Li <colyli@suse.de>
>>>>> Signed-off-by: Markus Weippert <markus@gekmihesg.de>
>>>> 
>>>> Added into my for-next.  Thanks for patching up.
>>> 
>>> We should probably get this into the current release, rather than punt
>>> it to 6.8.
>> 
>> Yes, copied. So far I don?t have other bcache patches for 6.7, I feel
>> I might be redundant if I send you another for -rc4 series with this
>> single patch.
>> 
>> Could you please directly take it into -rc4?
> 
> Sure, I'll just grab it as-is.

Thanks for doing this.

Coly Li