All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] block: Initialize cfqq->ioprio_class in cfq_get_queue()
@ 2017-01-23 14:06 Alexander Potapenko
  2017-01-23 15:30   ` Jens Axboe
  2017-01-23 16:03 ` Andrey Ryabinin
  0 siblings, 2 replies; 11+ messages in thread
From: Alexander Potapenko @ 2017-01-23 14:06 UTC (permalink / raw)
  To: dvyukov, kcc, axboe, tahsin; +Cc: linux-kernel, linux-block

KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of
uninitialized memory in cfq_init_cfqq():

==================================================================
BUG: KMSAN: use of unitialized memory
...
Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff8202ac97>] dump_stack+0x157/0x1d0 lib/dump_stack.c:51
 [<ffffffff813e9b65>] kmsan_report+0x205/0x360 ??:?
 [<ffffffff813eabbb>] __msan_warning+0x5b/0xb0 ??:?
 [<     inline     >] cfq_init_cfqq block/cfq-iosched.c:3754
 [<ffffffff8201e110>] cfq_get_queue+0xc80/0x14d0 block/cfq-iosched.c:3857
...
origin:
 [<ffffffff8103ab37>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
 [<ffffffff813e836b>] kmsan_internal_poison_shadow+0xab/0x150 ??:?
 [<ffffffff813e88ab>] kmsan_poison_slab+0xbb/0x120 ??:?
 [<     inline     >] allocate_slab mm/slub.c:1627
 [<ffffffff813e533f>] new_slab+0x3af/0x4b0 mm/slub.c:1641
 [<     inline     >] new_slab_objects mm/slub.c:2407
 [<ffffffff813e0ef3>] ___slab_alloc+0x323/0x4a0 mm/slub.c:2564
 [<     inline     >] __slab_alloc mm/slub.c:2606
 [<     inline     >] slab_alloc_node mm/slub.c:2669
 [<ffffffff813dfb42>] kmem_cache_alloc_node+0x1d2/0x1f0 mm/slub.c:2746
 [<ffffffff8201d90d>] cfq_get_queue+0x47d/0x14d0 block/cfq-iosched.c:3850
...
==================================================================
(the line numbers are relative to 4.8-rc6, but the bug persists
upstream)

The uninitialized struct cfq_queue is created by kmem_cache_alloc_node()
and then passed to cfq_init_cfqq(), which accesses cfqq->ioprio_class
before it's initialized.

Signed-off-by: Alexander Potapenko <glider@google.com>
---
 block/cfq-iosched.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index c73a6fcaeb9d..611061ae470a 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -3864,6 +3864,8 @@ cfq_get_queue(struct cfq_data *cfqd, bool is_sync, struct cfq_io_cq *cic,
 		goto out;
 	}
 
+	/* cfq_init_cfqq() assumes cfqq->ioprio_class is initialized. */
+	cfqq->ioprio_class = IOPRIO_CLASS_NONE;
 	cfq_init_cfqq(cfqd, cfqq, current->pid, is_sync);
 	cfq_init_prio_data(cfqq, cic);
 	cfq_link_cfqq_cfqg(cfqq, cfqg);
-- 
2.11.0.483.g087da7b7c-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] block: Initialize cfqq->ioprio_class in cfq_get_queue()
  2017-01-23 14:06 [PATCH] block: Initialize cfqq->ioprio_class in cfq_get_queue() Alexander Potapenko
@ 2017-01-23 15:30   ` Jens Axboe
  2017-01-23 16:03 ` Andrey Ryabinin
  1 sibling, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2017-01-23 15:30 UTC (permalink / raw)
  To: Alexander Potapenko, dvyukov, kcc, tahsin; +Cc: linux-kernel, linux-block

On 01/23/2017 07:06 AM, Alexander Potapenko wrote:
> KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of
> uninitialized memory in cfq_init_cfqq():
> 
> ==================================================================
> BUG: KMSAN: use of unitialized memory
> ...
> Call Trace:
>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>  [<ffffffff8202ac97>] dump_stack+0x157/0x1d0 lib/dump_stack.c:51
>  [<ffffffff813e9b65>] kmsan_report+0x205/0x360 ??:?
>  [<ffffffff813eabbb>] __msan_warning+0x5b/0xb0 ??:?
>  [<     inline     >] cfq_init_cfqq block/cfq-iosched.c:3754
>  [<ffffffff8201e110>] cfq_get_queue+0xc80/0x14d0 block/cfq-iosched.c:3857
> ...
> origin:
>  [<ffffffff8103ab37>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
>  [<ffffffff813e836b>] kmsan_internal_poison_shadow+0xab/0x150 ??:?
>  [<ffffffff813e88ab>] kmsan_poison_slab+0xbb/0x120 ??:?
>  [<     inline     >] allocate_slab mm/slub.c:1627
>  [<ffffffff813e533f>] new_slab+0x3af/0x4b0 mm/slub.c:1641
>  [<     inline     >] new_slab_objects mm/slub.c:2407
>  [<ffffffff813e0ef3>] ___slab_alloc+0x323/0x4a0 mm/slub.c:2564
>  [<     inline     >] __slab_alloc mm/slub.c:2606
>  [<     inline     >] slab_alloc_node mm/slub.c:2669
>  [<ffffffff813dfb42>] kmem_cache_alloc_node+0x1d2/0x1f0 mm/slub.c:2746
>  [<ffffffff8201d90d>] cfq_get_queue+0x47d/0x14d0 block/cfq-iosched.c:3850
> ...
> ==================================================================
> (the line numbers are relative to 4.8-rc6, but the bug persists
> upstream)
> 
> The uninitialized struct cfq_queue is created by kmem_cache_alloc_node()
> and then passed to cfq_init_cfqq(), which accesses cfqq->ioprio_class
> before it's initialized.

Patch looks fine to me, thanks. Is this a new warning? We don't seem
to have changed this path in a while, yet I wonder why this is only
surfacing now.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] block: Initialize cfqq->ioprio_class in cfq_get_queue()
@ 2017-01-23 15:30   ` Jens Axboe
  0 siblings, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2017-01-23 15:30 UTC (permalink / raw)
  To: Alexander Potapenko, dvyukov, kcc, tahsin; +Cc: linux-kernel, linux-block

On 01/23/2017 07:06 AM, Alexander Potapenko wrote:
> KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of
> uninitialized memory in cfq_init_cfqq():
> 
> ==================================================================
> BUG: KMSAN: use of unitialized memory
> ...
> Call Trace:
>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>  [<ffffffff8202ac97>] dump_stack+0x157/0x1d0 lib/dump_stack.c:51
>  [<ffffffff813e9b65>] kmsan_report+0x205/0x360 ??:?
>  [<ffffffff813eabbb>] __msan_warning+0x5b/0xb0 ??:?
>  [<     inline     >] cfq_init_cfqq block/cfq-iosched.c:3754
>  [<ffffffff8201e110>] cfq_get_queue+0xc80/0x14d0 block/cfq-iosched.c:3857
> ...
> origin:
>  [<ffffffff8103ab37>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
>  [<ffffffff813e836b>] kmsan_internal_poison_shadow+0xab/0x150 ??:?
>  [<ffffffff813e88ab>] kmsan_poison_slab+0xbb/0x120 ??:?
>  [<     inline     >] allocate_slab mm/slub.c:1627
>  [<ffffffff813e533f>] new_slab+0x3af/0x4b0 mm/slub.c:1641
>  [<     inline     >] new_slab_objects mm/slub.c:2407
>  [<ffffffff813e0ef3>] ___slab_alloc+0x323/0x4a0 mm/slub.c:2564
>  [<     inline     >] __slab_alloc mm/slub.c:2606
>  [<     inline     >] slab_alloc_node mm/slub.c:2669
>  [<ffffffff813dfb42>] kmem_cache_alloc_node+0x1d2/0x1f0 mm/slub.c:2746
>  [<ffffffff8201d90d>] cfq_get_queue+0x47d/0x14d0 block/cfq-iosched.c:3850
> ...
> ==================================================================
> (the line numbers are relative to 4.8-rc6, but the bug persists
> upstream)
> 
> The uninitialized struct cfq_queue is created by kmem_cache_alloc_node()
> and then passed to cfq_init_cfqq(), which accesses cfqq->ioprio_class
> before it's initialized.

Patch looks fine to me, thanks. Is this a new warning? We don't seem
to have changed this path in a while, yet I wonder why this is only
surfacing now.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] block: Initialize cfqq->ioprio_class in cfq_get_queue()
  2017-01-23 15:30   ` Jens Axboe
@ 2017-01-23 15:49     ` Alexander Potapenko
  -1 siblings, 0 replies; 11+ messages in thread
From: Alexander Potapenko @ 2017-01-23 15:49 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Dmitriy Vyukov, Kostya Serebryany, Tahsin Erdogan, LKML, linux-block

On Mon, Jan 23, 2017 at 4:30 PM, Jens Axboe <axboe@fb.com> wrote:
> On 01/23/2017 07:06 AM, Alexander Potapenko wrote:
>> KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of
>> uninitialized memory in cfq_init_cfqq():
>>
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> BUG: KMSAN: use of unitialized memory
>> ...
>> Call Trace:
>>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>>  [<ffffffff8202ac97>] dump_stack+0x157/0x1d0 lib/dump_stack.c:51
>>  [<ffffffff813e9b65>] kmsan_report+0x205/0x360 ??:?
>>  [<ffffffff813eabbb>] __msan_warning+0x5b/0xb0 ??:?
>>  [<     inline     >] cfq_init_cfqq block/cfq-iosched.c:3754
>>  [<ffffffff8201e110>] cfq_get_queue+0xc80/0x14d0 block/cfq-iosched.c:385=
7
>> ...
>> origin:
>>  [<ffffffff8103ab37>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktr=
ace.c:67
>>  [<ffffffff813e836b>] kmsan_internal_poison_shadow+0xab/0x150 ??:?
>>  [<ffffffff813e88ab>] kmsan_poison_slab+0xbb/0x120 ??:?
>>  [<     inline     >] allocate_slab mm/slub.c:1627
>>  [<ffffffff813e533f>] new_slab+0x3af/0x4b0 mm/slub.c:1641
>>  [<     inline     >] new_slab_objects mm/slub.c:2407
>>  [<ffffffff813e0ef3>] ___slab_alloc+0x323/0x4a0 mm/slub.c:2564
>>  [<     inline     >] __slab_alloc mm/slub.c:2606
>>  [<     inline     >] slab_alloc_node mm/slub.c:2669
>>  [<ffffffff813dfb42>] kmem_cache_alloc_node+0x1d2/0x1f0 mm/slub.c:2746
>>  [<ffffffff8201d90d>] cfq_get_queue+0x47d/0x14d0 block/cfq-iosched.c:385=
0
>> ...
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> (the line numbers are relative to 4.8-rc6, but the bug persists
>> upstream)
>>
>> The uninitialized struct cfq_queue is created by kmem_cache_alloc_node()
>> and then passed to cfq_init_cfqq(), which accesses cfqq->ioprio_class
>> before it's initialized.
>
> Patch looks fine to me, thanks. Is this a new warning? We don't seem
> to have changed this path in a while, yet I wonder why this is only
> surfacing now.

This is because KMSAN is a new tool, it's not in the trunk yet. Nobody
could see this warning before.
I don't know if kmemcheck detects this bug (and if anyone is actively
using it for testing either).
> --
> Jens Axboe
>



--=20
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Stra=C3=9Fe, 33
80636 M=C3=BCnchen

Gesch=C3=A4ftsf=C3=BChrer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] block: Initialize cfqq->ioprio_class in cfq_get_queue()
@ 2017-01-23 15:49     ` Alexander Potapenko
  0 siblings, 0 replies; 11+ messages in thread
From: Alexander Potapenko @ 2017-01-23 15:49 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Dmitriy Vyukov, Kostya Serebryany, Tahsin Erdogan, LKML, linux-block

On Mon, Jan 23, 2017 at 4:30 PM, Jens Axboe <axboe@fb.com> wrote:
> On 01/23/2017 07:06 AM, Alexander Potapenko wrote:
>> KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of
>> uninitialized memory in cfq_init_cfqq():
>>
>> ==================================================================
>> BUG: KMSAN: use of unitialized memory
>> ...
>> Call Trace:
>>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>>  [<ffffffff8202ac97>] dump_stack+0x157/0x1d0 lib/dump_stack.c:51
>>  [<ffffffff813e9b65>] kmsan_report+0x205/0x360 ??:?
>>  [<ffffffff813eabbb>] __msan_warning+0x5b/0xb0 ??:?
>>  [<     inline     >] cfq_init_cfqq block/cfq-iosched.c:3754
>>  [<ffffffff8201e110>] cfq_get_queue+0xc80/0x14d0 block/cfq-iosched.c:3857
>> ...
>> origin:
>>  [<ffffffff8103ab37>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
>>  [<ffffffff813e836b>] kmsan_internal_poison_shadow+0xab/0x150 ??:?
>>  [<ffffffff813e88ab>] kmsan_poison_slab+0xbb/0x120 ??:?
>>  [<     inline     >] allocate_slab mm/slub.c:1627
>>  [<ffffffff813e533f>] new_slab+0x3af/0x4b0 mm/slub.c:1641
>>  [<     inline     >] new_slab_objects mm/slub.c:2407
>>  [<ffffffff813e0ef3>] ___slab_alloc+0x323/0x4a0 mm/slub.c:2564
>>  [<     inline     >] __slab_alloc mm/slub.c:2606
>>  [<     inline     >] slab_alloc_node mm/slub.c:2669
>>  [<ffffffff813dfb42>] kmem_cache_alloc_node+0x1d2/0x1f0 mm/slub.c:2746
>>  [<ffffffff8201d90d>] cfq_get_queue+0x47d/0x14d0 block/cfq-iosched.c:3850
>> ...
>> ==================================================================
>> (the line numbers are relative to 4.8-rc6, but the bug persists
>> upstream)
>>
>> The uninitialized struct cfq_queue is created by kmem_cache_alloc_node()
>> and then passed to cfq_init_cfqq(), which accesses cfqq->ioprio_class
>> before it's initialized.
>
> Patch looks fine to me, thanks. Is this a new warning? We don't seem
> to have changed this path in a while, yet I wonder why this is only
> surfacing now.

This is because KMSAN is a new tool, it's not in the trunk yet. Nobody
could see this warning before.
I don't know if kmemcheck detects this bug (and if anyone is actively
using it for testing either).
> --
> Jens Axboe
>



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] block: Initialize cfqq->ioprio_class in cfq_get_queue()
  2017-01-23 15:49     ` Alexander Potapenko
  (?)
@ 2017-01-23 15:50     ` Jens Axboe
  -1 siblings, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2017-01-23 15:50 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: Dmitriy Vyukov, Kostya Serebryany, Tahsin Erdogan, LKML, linux-block

On 01/23/2017 08:49 AM, Alexander Potapenko wrote:
> On Mon, Jan 23, 2017 at 4:30 PM, Jens Axboe <axboe@fb.com> wrote:
>> On 01/23/2017 07:06 AM, Alexander Potapenko wrote:
>>> KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of
>>> uninitialized memory in cfq_init_cfqq():
>>>
>>> ==================================================================
>>> BUG: KMSAN: use of unitialized memory
>>> ...
>>> Call Trace:
>>>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>>>  [<ffffffff8202ac97>] dump_stack+0x157/0x1d0 lib/dump_stack.c:51
>>>  [<ffffffff813e9b65>] kmsan_report+0x205/0x360 ??:?
>>>  [<ffffffff813eabbb>] __msan_warning+0x5b/0xb0 ??:?
>>>  [<     inline     >] cfq_init_cfqq block/cfq-iosched.c:3754
>>>  [<ffffffff8201e110>] cfq_get_queue+0xc80/0x14d0 block/cfq-iosched.c:3857
>>> ...
>>> origin:
>>>  [<ffffffff8103ab37>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
>>>  [<ffffffff813e836b>] kmsan_internal_poison_shadow+0xab/0x150 ??:?
>>>  [<ffffffff813e88ab>] kmsan_poison_slab+0xbb/0x120 ??:?
>>>  [<     inline     >] allocate_slab mm/slub.c:1627
>>>  [<ffffffff813e533f>] new_slab+0x3af/0x4b0 mm/slub.c:1641
>>>  [<     inline     >] new_slab_objects mm/slub.c:2407
>>>  [<ffffffff813e0ef3>] ___slab_alloc+0x323/0x4a0 mm/slub.c:2564
>>>  [<     inline     >] __slab_alloc mm/slub.c:2606
>>>  [<     inline     >] slab_alloc_node mm/slub.c:2669
>>>  [<ffffffff813dfb42>] kmem_cache_alloc_node+0x1d2/0x1f0 mm/slub.c:2746
>>>  [<ffffffff8201d90d>] cfq_get_queue+0x47d/0x14d0 block/cfq-iosched.c:3850
>>> ...
>>> ==================================================================
>>> (the line numbers are relative to 4.8-rc6, but the bug persists
>>> upstream)
>>>
>>> The uninitialized struct cfq_queue is created by kmem_cache_alloc_node()
>>> and then passed to cfq_init_cfqq(), which accesses cfqq->ioprio_class
>>> before it's initialized.
>>
>> Patch looks fine to me, thanks. Is this a new warning? We don't seem
>> to have changed this path in a while, yet I wonder why this is only
>> surfacing now.
> 
> This is because KMSAN is a new tool, it's not in the trunk yet. Nobody
> could see this warning before.
> I don't know if kmemcheck detects this bug (and if anyone is actively
> using it for testing either).

Ah gotcha, I read that as KASAN. Then it makes more sense. Thanks for
clarifying.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] block: Initialize cfqq->ioprio_class in cfq_get_queue()
  2017-01-23 14:06 [PATCH] block: Initialize cfqq->ioprio_class in cfq_get_queue() Alexander Potapenko
  2017-01-23 15:30   ` Jens Axboe
@ 2017-01-23 16:03 ` Andrey Ryabinin
  2017-01-23 16:17     ` Alexander Potapenko
  1 sibling, 1 reply; 11+ messages in thread
From: Andrey Ryabinin @ 2017-01-23 16:03 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: Dmitry Vyukov, Konstantin Serebryany, axboe, tahsin, LKML, linux-block

2017-01-23 17:06 GMT+03:00 Alexander Potapenko <glider@google.com>:
> KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of
> uninitialized memory in cfq_init_cfqq():
>
> ==================================================================
> BUG: KMSAN: use of unitialized memory
> ...
> Call Trace:
>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>  [<ffffffff8202ac97>] dump_stack+0x157/0x1d0 lib/dump_stack.c:51
>  [<ffffffff813e9b65>] kmsan_report+0x205/0x360 ??:?
>  [<ffffffff813eabbb>] __msan_warning+0x5b/0xb0 ??:?
>  [<     inline     >] cfq_init_cfqq block/cfq-iosched.c:3754
>  [<ffffffff8201e110>] cfq_get_queue+0xc80/0x14d0 block/cfq-iosched.c:3857
> ...
> origin:
>  [<ffffffff8103ab37>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
>  [<ffffffff813e836b>] kmsan_internal_poison_shadow+0xab/0x150 ??:?
>  [<ffffffff813e88ab>] kmsan_poison_slab+0xbb/0x120 ??:?
>  [<     inline     >] allocate_slab mm/slub.c:1627
>  [<ffffffff813e533f>] new_slab+0x3af/0x4b0 mm/slub.c:1641
>  [<     inline     >] new_slab_objects mm/slub.c:2407
>  [<ffffffff813e0ef3>] ___slab_alloc+0x323/0x4a0 mm/slub.c:2564
>  [<     inline     >] __slab_alloc mm/slub.c:2606
>  [<     inline     >] slab_alloc_node mm/slub.c:2669
>  [<ffffffff813dfb42>] kmem_cache_alloc_node+0x1d2/0x1f0 mm/slub.c:2746
>  [<ffffffff8201d90d>] cfq_get_queue+0x47d/0x14d0 block/cfq-iosched.c:3850
> ...
> ==================================================================
> (the line numbers are relative to 4.8-rc6, but the bug persists
> upstream)
>
> The uninitialized struct cfq_queue is created by kmem_cache_alloc_node()
> and then passed to cfq_init_cfqq(), which accesses cfqq->ioprio_class
> before it's initialized.
>

struct cfq_queue is zero initialized (__GFP_ZERO).
The warning is false-positive.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] block: Initialize cfqq->ioprio_class in cfq_get_queue()
  2017-01-23 16:03 ` Andrey Ryabinin
@ 2017-01-23 16:17     ` Alexander Potapenko
  0 siblings, 0 replies; 11+ messages in thread
From: Alexander Potapenko @ 2017-01-23 16:17 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Dmitry Vyukov, Konstantin Serebryany, Jens Axboe, Tahsin Erdogan,
	LKML, linux-block

On Mon, Jan 23, 2017 at 5:03 PM, Andrey Ryabinin <ryabinin.a.a@gmail.com> w=
rote:
> 2017-01-23 17:06 GMT+03:00 Alexander Potapenko <glider@google.com>:
>> KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of
>> uninitialized memory in cfq_init_cfqq():
>>
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> BUG: KMSAN: use of unitialized memory
>> ...
>> Call Trace:
>>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>>  [<ffffffff8202ac97>] dump_stack+0x157/0x1d0 lib/dump_stack.c:51
>>  [<ffffffff813e9b65>] kmsan_report+0x205/0x360 ??:?
>>  [<ffffffff813eabbb>] __msan_warning+0x5b/0xb0 ??:?
>>  [<     inline     >] cfq_init_cfqq block/cfq-iosched.c:3754
>>  [<ffffffff8201e110>] cfq_get_queue+0xc80/0x14d0 block/cfq-iosched.c:385=
7
>> ...
>> origin:
>>  [<ffffffff8103ab37>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktr=
ace.c:67
>>  [<ffffffff813e836b>] kmsan_internal_poison_shadow+0xab/0x150 ??:?
>>  [<ffffffff813e88ab>] kmsan_poison_slab+0xbb/0x120 ??:?
>>  [<     inline     >] allocate_slab mm/slub.c:1627
>>  [<ffffffff813e533f>] new_slab+0x3af/0x4b0 mm/slub.c:1641
>>  [<     inline     >] new_slab_objects mm/slub.c:2407
>>  [<ffffffff813e0ef3>] ___slab_alloc+0x323/0x4a0 mm/slub.c:2564
>>  [<     inline     >] __slab_alloc mm/slub.c:2606
>>  [<     inline     >] slab_alloc_node mm/slub.c:2669
>>  [<ffffffff813dfb42>] kmem_cache_alloc_node+0x1d2/0x1f0 mm/slub.c:2746
>>  [<ffffffff8201d90d>] cfq_get_queue+0x47d/0x14d0 block/cfq-iosched.c:385=
0
>> ...
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> (the line numbers are relative to 4.8-rc6, but the bug persists
>> upstream)
>>
>> The uninitialized struct cfq_queue is created by kmem_cache_alloc_node()
>> and then passed to cfq_init_cfqq(), which accesses cfqq->ioprio_class
>> before it's initialized.
>>
>
> struct cfq_queue is zero initialized (__GFP_ZERO).
> The warning is false-positive.
You are totally right. I've handled __GFP_ZERO in (hopefully) every
case except for this one, and overlooked the presence of that flag in
the kmem_cache_alloc_node().
Thanks for double-checking!
Jens, sorry for the false alarm.


--=20
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Stra=C3=9Fe, 33
80636 M=C3=BCnchen

Gesch=C3=A4ftsf=C3=BChrer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] block: Initialize cfqq->ioprio_class in cfq_get_queue()
@ 2017-01-23 16:17     ` Alexander Potapenko
  0 siblings, 0 replies; 11+ messages in thread
From: Alexander Potapenko @ 2017-01-23 16:17 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Dmitry Vyukov, Konstantin Serebryany, Jens Axboe, Tahsin Erdogan,
	LKML, linux-block

On Mon, Jan 23, 2017 at 5:03 PM, Andrey Ryabinin <ryabinin.a.a@gmail.com> wrote:
> 2017-01-23 17:06 GMT+03:00 Alexander Potapenko <glider@google.com>:
>> KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of
>> uninitialized memory in cfq_init_cfqq():
>>
>> ==================================================================
>> BUG: KMSAN: use of unitialized memory
>> ...
>> Call Trace:
>>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>>  [<ffffffff8202ac97>] dump_stack+0x157/0x1d0 lib/dump_stack.c:51
>>  [<ffffffff813e9b65>] kmsan_report+0x205/0x360 ??:?
>>  [<ffffffff813eabbb>] __msan_warning+0x5b/0xb0 ??:?
>>  [<     inline     >] cfq_init_cfqq block/cfq-iosched.c:3754
>>  [<ffffffff8201e110>] cfq_get_queue+0xc80/0x14d0 block/cfq-iosched.c:3857
>> ...
>> origin:
>>  [<ffffffff8103ab37>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
>>  [<ffffffff813e836b>] kmsan_internal_poison_shadow+0xab/0x150 ??:?
>>  [<ffffffff813e88ab>] kmsan_poison_slab+0xbb/0x120 ??:?
>>  [<     inline     >] allocate_slab mm/slub.c:1627
>>  [<ffffffff813e533f>] new_slab+0x3af/0x4b0 mm/slub.c:1641
>>  [<     inline     >] new_slab_objects mm/slub.c:2407
>>  [<ffffffff813e0ef3>] ___slab_alloc+0x323/0x4a0 mm/slub.c:2564
>>  [<     inline     >] __slab_alloc mm/slub.c:2606
>>  [<     inline     >] slab_alloc_node mm/slub.c:2669
>>  [<ffffffff813dfb42>] kmem_cache_alloc_node+0x1d2/0x1f0 mm/slub.c:2746
>>  [<ffffffff8201d90d>] cfq_get_queue+0x47d/0x14d0 block/cfq-iosched.c:3850
>> ...
>> ==================================================================
>> (the line numbers are relative to 4.8-rc6, but the bug persists
>> upstream)
>>
>> The uninitialized struct cfq_queue is created by kmem_cache_alloc_node()
>> and then passed to cfq_init_cfqq(), which accesses cfqq->ioprio_class
>> before it's initialized.
>>
>
> struct cfq_queue is zero initialized (__GFP_ZERO).
> The warning is false-positive.
You are totally right. I've handled __GFP_ZERO in (hopefully) every
case except for this one, and overlooked the presence of that flag in
the kmem_cache_alloc_node().
Thanks for double-checking!
Jens, sorry for the false alarm.


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] block: Initialize cfqq->ioprio_class in cfq_get_queue()
  2017-01-23 16:17     ` Alexander Potapenko
@ 2017-01-23 18:03       ` Jens Axboe
  -1 siblings, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2017-01-23 18:03 UTC (permalink / raw)
  To: Alexander Potapenko, Andrey Ryabinin
  Cc: Dmitry Vyukov, Konstantin Serebryany, Tahsin Erdogan, LKML, linux-block

On 01/23/2017 09:17 AM, Alexander Potapenko wrote:
> On Mon, Jan 23, 2017 at 5:03 PM, Andrey Ryabinin <ryabinin.a.a@gmail.com> wrote:
>> 2017-01-23 17:06 GMT+03:00 Alexander Potapenko <glider@google.com>:
>>> KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of
>>> uninitialized memory in cfq_init_cfqq():
>>>
>>> ==================================================================
>>> BUG: KMSAN: use of unitialized memory
>>> ...
>>> Call Trace:
>>>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>>>  [<ffffffff8202ac97>] dump_stack+0x157/0x1d0 lib/dump_stack.c:51
>>>  [<ffffffff813e9b65>] kmsan_report+0x205/0x360 ??:?
>>>  [<ffffffff813eabbb>] __msan_warning+0x5b/0xb0 ??:?
>>>  [<     inline     >] cfq_init_cfqq block/cfq-iosched.c:3754
>>>  [<ffffffff8201e110>] cfq_get_queue+0xc80/0x14d0 block/cfq-iosched.c:3857
>>> ...
>>> origin:
>>>  [<ffffffff8103ab37>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
>>>  [<ffffffff813e836b>] kmsan_internal_poison_shadow+0xab/0x150 ??:?
>>>  [<ffffffff813e88ab>] kmsan_poison_slab+0xbb/0x120 ??:?
>>>  [<     inline     >] allocate_slab mm/slub.c:1627
>>>  [<ffffffff813e533f>] new_slab+0x3af/0x4b0 mm/slub.c:1641
>>>  [<     inline     >] new_slab_objects mm/slub.c:2407
>>>  [<ffffffff813e0ef3>] ___slab_alloc+0x323/0x4a0 mm/slub.c:2564
>>>  [<     inline     >] __slab_alloc mm/slub.c:2606
>>>  [<     inline     >] slab_alloc_node mm/slub.c:2669
>>>  [<ffffffff813dfb42>] kmem_cache_alloc_node+0x1d2/0x1f0 mm/slub.c:2746
>>>  [<ffffffff8201d90d>] cfq_get_queue+0x47d/0x14d0 block/cfq-iosched.c:3850
>>> ...
>>> ==================================================================
>>> (the line numbers are relative to 4.8-rc6, but the bug persists
>>> upstream)
>>>
>>> The uninitialized struct cfq_queue is created by kmem_cache_alloc_node()
>>> and then passed to cfq_init_cfqq(), which accesses cfqq->ioprio_class
>>> before it's initialized.
>>>
>>
>> struct cfq_queue is zero initialized (__GFP_ZERO).
>> The warning is false-positive.
> You are totally right. I've handled __GFP_ZERO in (hopefully) every
> case except for this one, and overlooked the presence of that flag in
> the kmem_cache_alloc_node().
> Thanks for double-checking!
> Jens, sorry for the false alarm.

No worries, I did queue up the patch, since even if it is a false
positive, it's cleaner to set this explicitly to NONE rather than
silently rely on the fact that NONE is 0.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] block: Initialize cfqq->ioprio_class in cfq_get_queue()
@ 2017-01-23 18:03       ` Jens Axboe
  0 siblings, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2017-01-23 18:03 UTC (permalink / raw)
  To: Alexander Potapenko, Andrey Ryabinin
  Cc: Dmitry Vyukov, Konstantin Serebryany, Tahsin Erdogan, LKML, linux-block

On 01/23/2017 09:17 AM, Alexander Potapenko wrote:
> On Mon, Jan 23, 2017 at 5:03 PM, Andrey Ryabinin <ryabinin.a.a@gmail.com> wrote:
>> 2017-01-23 17:06 GMT+03:00 Alexander Potapenko <glider@google.com>:
>>> KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of
>>> uninitialized memory in cfq_init_cfqq():
>>>
>>> ==================================================================
>>> BUG: KMSAN: use of unitialized memory
>>> ...
>>> Call Trace:
>>>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>>>  [<ffffffff8202ac97>] dump_stack+0x157/0x1d0 lib/dump_stack.c:51
>>>  [<ffffffff813e9b65>] kmsan_report+0x205/0x360 ??:?
>>>  [<ffffffff813eabbb>] __msan_warning+0x5b/0xb0 ??:?
>>>  [<     inline     >] cfq_init_cfqq block/cfq-iosched.c:3754
>>>  [<ffffffff8201e110>] cfq_get_queue+0xc80/0x14d0 block/cfq-iosched.c:3857
>>> ...
>>> origin:
>>>  [<ffffffff8103ab37>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
>>>  [<ffffffff813e836b>] kmsan_internal_poison_shadow+0xab/0x150 ??:?
>>>  [<ffffffff813e88ab>] kmsan_poison_slab+0xbb/0x120 ??:?
>>>  [<     inline     >] allocate_slab mm/slub.c:1627
>>>  [<ffffffff813e533f>] new_slab+0x3af/0x4b0 mm/slub.c:1641
>>>  [<     inline     >] new_slab_objects mm/slub.c:2407
>>>  [<ffffffff813e0ef3>] ___slab_alloc+0x323/0x4a0 mm/slub.c:2564
>>>  [<     inline     >] __slab_alloc mm/slub.c:2606
>>>  [<     inline     >] slab_alloc_node mm/slub.c:2669
>>>  [<ffffffff813dfb42>] kmem_cache_alloc_node+0x1d2/0x1f0 mm/slub.c:2746
>>>  [<ffffffff8201d90d>] cfq_get_queue+0x47d/0x14d0 block/cfq-iosched.c:3850
>>> ...
>>> ==================================================================
>>> (the line numbers are relative to 4.8-rc6, but the bug persists
>>> upstream)
>>>
>>> The uninitialized struct cfq_queue is created by kmem_cache_alloc_node()
>>> and then passed to cfq_init_cfqq(), which accesses cfqq->ioprio_class
>>> before it's initialized.
>>>
>>
>> struct cfq_queue is zero initialized (__GFP_ZERO).
>> The warning is false-positive.
> You are totally right. I've handled __GFP_ZERO in (hopefully) every
> case except for this one, and overlooked the presence of that flag in
> the kmem_cache_alloc_node().
> Thanks for double-checking!
> Jens, sorry for the false alarm.

No worries, I did queue up the patch, since even if it is a false
positive, it's cleaner to set this explicitly to NONE rather than
silently rely on the fact that NONE is 0.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-01-23 18:03 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-23 14:06 [PATCH] block: Initialize cfqq->ioprio_class in cfq_get_queue() Alexander Potapenko
2017-01-23 15:30 ` Jens Axboe
2017-01-23 15:30   ` Jens Axboe
2017-01-23 15:49   ` Alexander Potapenko
2017-01-23 15:49     ` Alexander Potapenko
2017-01-23 15:50     ` Jens Axboe
2017-01-23 16:03 ` Andrey Ryabinin
2017-01-23 16:17   ` Alexander Potapenko
2017-01-23 16:17     ` Alexander Potapenko
2017-01-23 18:03     ` Jens Axboe
2017-01-23 18:03       ` Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.