2013-08-05 13:57:25

by Sasha Levin

[permalink] [raw]
Subject: aio: kernel BUG at fs/aio.c:646!

Hi all,

While fuzzing with trinity inside a KVM tools guest running latest -next kernel,
I've stumbled on the following spew caused by a new BUG() added in "aio: fix
io_destroy() regression by using call_rcu()".

[ 9646.599640] kernel BUG at fs/aio.c:646!
[ 9646.600119] invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[ 9646.600119] Modules linked in:
[ 9646.600119] CPU: 3 PID: 16833 Comm: trinity-child45 Tainted: G W
3.11.0-rc3-next-20130802-sasha-00001-g5e30de1-dirty #3974
[ 9646.600119] task: ffff8800c4da0000 ti: ffff8800c4d9a000 task.ti: ffff8800c4d9a000
[ 9646.600119] RIP: 0010:[<ffffffff812ee8e1>] [<ffffffff812ee8e1>] kill_ioctx+0xb1/0x100
[ 9646.600119] RSP: 0018:ffff8800c4d9be08 EFLAGS: 00010287
[ 9646.600119] RAX: ffffffffffffd866 RBX: ffff8800c3628b40 RCX: 0000000000000003
[ 9646.600119] RDX: 0000000000014cf4 RSI: 0000000000000000 RDI: 0000000000000282
[ 9646.600119] RBP: ffff8800c4d9be28 R08: 0000000000000000 R09: 0000000000000001
[ 9646.600119] R10: 0000000000000001 R11: 0000000000000000 R12: ffff8800ca30c8c0
[ 9646.609819] R13: ffff8800c4c9d438 R14: ffff8800c3628b40 R15: ffff8800ca30c8c0
[ 9646.609819] FS: 0000000000000000(0000) GS:ffff880224c00000(0000) knlGS:0000000000000000
[ 9646.609819] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 9646.609819] CR2: 0000000000000001 CR3: 00000000c4d6b000 CR4: 00000000000006e0
[ 9646.609819] Stack:
[ 9646.609819] ffff8800c4d9be28 0000000000000001 ffff8800c4c9d000 ffff8800c4c9d480
[ 9646.609819] ffff8800c4d9be78 ffffffff812ef2c0 ffffffff812ef202 0000000000000282
[ 9646.609819] ffff8800c4c9d040 ffff8800c4c9d000 ffff8800c4da0000 ffff8800c4c9d000
[ 9646.609819] Call Trace:
[ 9646.609819] [<ffffffff812ef2c0>] exit_aio+0xe0/0x100
[ 9646.609819] [<ffffffff812ef202>] ? exit_aio+0x22/0x100
[ 9646.609819] [<ffffffff81120bc1>] mmput+0x41/0xf0
[ 9646.609819] [<ffffffff81124c2d>] exit_mm+0x18d/0x1a0
[ 9646.609819] [<ffffffff811a9db5>] ? acct_collect+0x175/0x1b0
[ 9646.609819] [<ffffffff811253aa>] do_exit+0x24a/0x4d0
[ 9646.609819] [<ffffffff811256d9>] do_group_exit+0xa9/0xe0
[ 9646.609819] [<ffffffff81125727>] SyS_exit_group+0x17/0x20
[ 9646.609819] [<ffffffff8409d82c>] tracesys+0xdd/0xe2
[ 9646.609819] Code: 8d bb 68 02 00 00 be 03 00 00 00 e8 3a bf e6 ff 48 c7 c7 20 e8 0a 86 e8 3e 54
da 02 48 8b 05 cf 89 19 06 8b 53 4c 48 29 d0 73 0f <0f> 0b 0f 1f 44 00 00 eb fe 66 0f 1f 44 00 00 48
89 05 b1 89 19
[ 9646.609819] RIP [<ffffffff812ee8e1>] kill_ioctx+0xb1/0x100
[ 9646.609819] RSP <ffff8800c4d9be08>


Thanks,
Sasha


2013-08-05 16:08:31

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: aio: kernel BUG at fs/aio.c:646!

Hi Sasha,

On Mon, Aug 05, 2013 at 09:57:08AM -0400, Sasha Levin wrote:
> Hi all,
>
> While fuzzing with trinity inside a KVM tools guest running latest -next
> kernel,
> I've stumbled on the following spew caused by a new BUG() added in "aio: fix
> io_destroy() regression by using call_rcu()".

I did some investigating, and it looks like there is a problem with
db446a08c23d5475e6b08c87acca79ebb20f283c (aio: convert the ioctx list to
table lookup v3). Can you confirm if reverting this patch eliminates
the BUG() you're hitting? In my testing, I wasn't able to trigger the
BUG(), but I was able to trip up slab corruption with debugging on.
Thanks,

-ben

...
> Thanks,
> Sasha

--
"Thought is the essence of where you are now."

2013-08-05 17:20:34

by Benjamin LaHaise

[permalink] [raw]
Subject: [PATCH aio-next] aio: fix error handling and rcu usage in "convert the ioctx list to table lookup v3"

On Mon, Aug 05, 2013 at 12:08:28PM -0400, Benjamin LaHaise wrote:
> Hi Sasha,
>
> On Mon, Aug 05, 2013 at 09:57:08AM -0400, Sasha Levin wrote:
> > Hi all,
> >
> > While fuzzing with trinity inside a KVM tools guest running latest -next
> > kernel,
> > I've stumbled on the following spew caused by a new BUG() added in "aio: fix
> > io_destroy() regression by using call_rcu()".
>
> I did some investigating, and it looks like there is a problem with
> db446a08c23d5475e6b08c87acca79ebb20f283c (aio: convert the ioctx list to
> table lookup v3). Can you confirm if reverting this patch eliminates
> the BUG() you're hitting? In my testing, I wasn't able to trigger the
> BUG(), but I was able to trip up slab corruption with debugging on.

And here is a patch that should fix the problems introduced in the table
lookup patch without reverting. I will add this to the aio-next.git tree.
This bug is not present in Linus' tree.

-ben

aio.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/aio.c b/fs/aio.c
index 588aff9..3bc068c 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -475,7 +475,7 @@ static int ioctx_add_table(struct kioctx *ctx, struct mm_struct *mm)
struct aio_ring *ring;

spin_lock(&mm->ioctx_lock);
- table = rcu_dereference(mm->ioctx_table);
+ table = mm->ioctx_table;

while (1) {
if (table)
@@ -503,7 +503,7 @@ static int ioctx_add_table(struct kioctx *ctx, struct mm_struct *mm)
table->nr = new_nr;

spin_lock(&mm->ioctx_lock);
- old = rcu_dereference(mm->ioctx_table);
+ old = mm->ioctx_table;

if (!old) {
rcu_assign_pointer(mm->ioctx_table, table);
@@ -579,10 +579,6 @@ static struct kioctx *ioctx_alloc(unsigned nr_events)
if (ctx->req_batch < 1)
ctx->req_batch = 1;

- err = ioctx_add_table(ctx, mm);
- if (err)
- goto out_cleanup_noerr;
-
/* limit the number of system wide aios */
spin_lock(&aio_nr_lock);
if (aio_nr + nr_events > (aio_max_nr * 2UL) ||
@@ -595,13 +591,18 @@ static struct kioctx *ioctx_alloc(unsigned nr_events)

percpu_ref_get(&ctx->users); /* io_setup() will drop this ref */

+ err = ioctx_add_table(ctx, mm);
+ if (err)
+ goto out_cleanup_put;
+
pr_debug("allocated ioctx %p[%ld]: mm=%p mask=0x%x\n",
ctx, ctx->user_id, mm, ctx->nr_events);
return ctx;

+out_cleanup_put:
+ percpu_ref_put(&ctx->users);
out_cleanup:
err = -EAGAIN;
-out_cleanup_noerr:
aio_free_ring(ctx);
out_freepcpu:
free_percpu(ctx->cpu);
@@ -626,7 +627,7 @@ static void kill_ioctx(struct mm_struct *mm, struct kioctx *ctx)
struct kioctx_table *table;

spin_lock(&mm->ioctx_lock);
- table = rcu_dereference(mm->ioctx_table);
+ table = mm->ioctx_table;

WARN_ON(ctx != table->table[ctx->id]);
table->table[ctx->id] = NULL;

2013-08-06 21:57:49

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH aio-next] aio: fix error handling and rcu usage in "convert the ioctx list to table lookup v3"

On 08/05/2013 01:20 PM, Benjamin LaHaise wrote:
> On Mon, Aug 05, 2013 at 12:08:28PM -0400, Benjamin LaHaise wrote:
>> Hi Sasha,
>>
>> On Mon, Aug 05, 2013 at 09:57:08AM -0400, Sasha Levin wrote:
>>> Hi all,
>>>
>>> While fuzzing with trinity inside a KVM tools guest running latest -next
>>> kernel,
>>> I've stumbled on the following spew caused by a new BUG() added in "aio: fix
>>> io_destroy() regression by using call_rcu()".
>>
>> I did some investigating, and it looks like there is a problem with
>> db446a08c23d5475e6b08c87acca79ebb20f283c (aio: convert the ioctx list to
>> table lookup v3). Can you confirm if reverting this patch eliminates
>> the BUG() you're hitting? In my testing, I wasn't able to trigger the
>> BUG(), but I was able to trip up slab corruption with debugging on.
>
> And here is a patch that should fix the problems introduced in the table
> lookup patch without reverting. I will add this to the aio-next.git tree.
> This bug is not present in Linus' tree.

[snip]

Old error is gone, but now seeing this, which seems related.

ctx = table->table[id];
if (ctx->user_id == ctx_id) { <--- here
percpu_ref_get(&ctx->users);
ret = ctx;
}

[ 542.182026] BUG: unable to handle kernel NULL pointer dereference at 0000000000000038
[ 542.183221] IP: [<ffffffff812ef78d>] lookup_ioctx+0x8d/0xe0
[ 542.183956] PGD 1b6e69067 PUD 1b6e6a067 PMD 0
[ 542.184593] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[ 542.185394] Modules linked in:
[ 542.185866] CPU: 2 PID: 22471 Comm: trinity-child36 Tainted: G W
3.11.0-rc4-next-20130806-sasha-00002-gb144a3f #3977
[ 542.187428] task: ffff88020bc40000 ti: ffff8801b6e7e000 task.ti: ffff8801b6e7e000
[ 542.188384] RIP: 0010:[<ffffffff812ef78d>] [<ffffffff812ef78d>] lookup_ioctx+0x8d/0xe0
[ 542.189408] RSP: 0018:ffff8801b6e7ff18 EFLAGS: 00010297
[ 542.190015] RAX: ffff88020a64a1b0 RBX: 00000000007f866d RCX: 0000000000000000
[ 542.190015] RDX: 0000000000000000 RSI: ffff88020bc40950 RDI: 0000000000000282
[ 542.190015] RBP: ffff8801b6e7ff48 R08: 0000000000000000 R09: 0000000000000000
[ 542.190015] R10: 0000000000000001 R11: 0000000000000000 R12: ffff88020bffc000
[ 542.190015] R13: 0000000000000000 R14: 0000000000000000 R15: 8000000000008000
[ 542.190015] FS: 00007fa96f2b8700(0000) GS:ffff880224a00000(0000) knlGS:0000000000000000
[ 542.190015] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 542.190015] CR2: 0000000000000001 CR3: 00000001b6e68000 CR4: 00000000000006e0
[ 542.190015] Stack:
[ 542.190015] ffffffff812ef747 ffffffff81074268 00000000007f866d 0000000000000678
[ 542.190015] 00007fa96f2b86a8 00007fff70fb7170 ffff8801b6e7ff78 ffffffff812f1103
[ 542.190015] 8000000000008000 00007fff70fb7170 00007fa96f2b86a8 00000000007f866d
[ 542.190015] Call Trace:
[ 542.190015] [<ffffffff812ef747>] ? lookup_ioctx+0x47/0xe0
[ 542.202270] [<ffffffff81074268>] ? syscall_trace_enter+0x28/0x230
[ 542.202270] [<ffffffff812f1103>] SyS_io_destroy+0x13/0x110
[ 542.202270] [<ffffffff840a3e2c>] tracesys+0xdd/0xe2
[ 542.202270] Code: 02 00 00 00 48 c7 c7 e0 65 a6 85 e8 7e 7c ea ff 49 8b 84 24 80 04 00 00 48 85
c0 74 21 44 3b 68 10 73 1b 45 89 ed 4e 8b 74 e8 18 <49> 39 5e 38 75 0d 4c 89 f7 e8 c5 fe ff ff eb 06
0f 1f 00 45 31
[ 542.202270] RIP [<ffffffff812ef78d>] lookup_ioctx+0x8d/0xe0
[ 542.202270] RSP <ffff8801b6e7ff18>
[ 542.202270] CR2: 0000000000000038


Thanks,
Sasha

2013-08-07 00:52:09

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: [PATCH aio-next] aio: fix error handling and rcu usage in "convert the ioctx list to table lookup v3"

On Tue, Aug 06, 2013 at 05:57:32PM -0400, Sasha Levin wrote:
> Old error is gone, but now seeing this, which seems related.
>
> ctx = table->table[id];
> if (ctx->user_id == ctx_id) { <--- here
> percpu_ref_get(&ctx->users);
> ret = ctx;
> }
...

Why am I not surprised. That should be fixed with the patch below. I'll
post some patches for the libaio test suite tomorrow to check these cases
explicitly and scan for any others that need to be added. Thanks again,
Sasha.

-ben
--
"Thought is the essence of where you are now."

diff --git a/fs/aio.c b/fs/aio.c
index 3bc068c..c3f005d 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -812,7 +812,7 @@ static struct kioctx *lookup_ioctx(unsigned long ctx_id)
goto out;

ctx = table->table[id];
- if (ctx->user_id == ctx_id) {
+ if (ctx && ctx->user_id == ctx_id) {
percpu_ref_get(&ctx->users);
ret = ctx;
}