2023-11-24 16:29:54

by Coly Li

[permalink] [raw]
Subject: Re: [PATCH] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR



> 2023年11月24日 23:14,Markus Weippert <[email protected]> 写道:
>
> 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/[email protected]/
> Cc: [email protected]
> Cc: Zheng Wang <[email protected]>
> Cc: Coly Li <[email protected]>
> Signed-off-by: Markus Weippert <[email protected]>

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]);
> }
> --


2023-11-24 16:32:28

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR

On 11/24/23 9:29 AM, Coly Li wrote:
>
>
>> 2023?11?24? 23:14?Markus Weippert <[email protected]> ???
>>
>> 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/[email protected]/
>> Cc: [email protected]
>> Cc: Zheng Wang <[email protected]>
>> Cc: Coly Li <[email protected]>
>> Signed-off-by: Markus Weippert <[email protected]>
>
> 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

2023-11-24 16:35:15

by Coly Li

[permalink] [raw]
Subject: Re: [PATCH] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR



> 2023年11月25日 00:31,Jens Axboe <[email protected]> 写道:
>
> On 11/24/23 9:29 AM, Coly Li wrote:
>>
>>
>>> 2023?11?24? 23:14?Markus Weippert <[email protected]> ???
>>>
>>> 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/[email protected]/
>>> Cc: [email protected]
>>> Cc: Zheng Wang <[email protected]>
>>> Cc: Coly Li <[email protected]>
>>> Signed-off-by: Markus Weippert <[email protected]>
>>
>> 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

2023-11-24 16:38:03

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR

On 11/24/23 9:34 AM, Coly Li wrote:
>
>
>> 2023?11?25? 00:31?Jens Axboe <[email protected]> ???
>>
>> On 11/24/23 9:29 AM, Coly Li wrote:
>>>
>>>
>>>> 2023?11?24? 23:14?Markus Weippert <[email protected]> ???
>>>>
>>>> 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/[email protected]/
>>>> Cc: [email protected]
>>>> Cc: Zheng Wang <[email protected]>
>>>> Cc: Coly Li <[email protected]>
>>>> Signed-off-by: Markus Weippert <[email protected]>
>>>
>>> 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

2023-11-24 16:43:02

by Coly Li

[permalink] [raw]
Subject: Re: [PATCH] bcache: revert replacing IS_ERR_OR_NULL with IS_ERR



> 2023年11月25日 00:35,Jens Axboe <[email protected]> 写道:
>
> On 11/24/23 9:34 AM, Coly Li wrote:
>>
>>
>>> 2023?11?25? 00:31?Jens Axboe <[email protected]> ???
>>>
>>> On 11/24/23 9:29 AM, Coly Li wrote:
>>>>
>>>>
>>>>> 2023?11?24? 23:14?Markus Weippert <[email protected]> ???
>>>>>
>>>>> 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/[email protected]/
>>>>> Cc: [email protected]
>>>>> Cc: Zheng Wang <[email protected]>
>>>>> Cc: Coly Li <[email protected]>
>>>>> Signed-off-by: Markus Weippert <[email protected]>
>>>>
>>>> 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