Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755738AbdLTQiL (ORCPT ); Wed, 20 Dec 2017 11:38:11 -0500 Received: from mail-qt0-f195.google.com ([209.85.216.195]:33592 "EHLO mail-qt0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755712AbdLTQiI (ORCPT ); Wed, 20 Dec 2017 11:38:08 -0500 X-Google-Smtp-Source: ACJfBovNgII6zvQy7T8ctg6i3/wuSDc5HcrpMULK6UPL4YmYAHRTPZOcH0k6uuxv8HVffYI2jza4va6mFwFBrRGArDQ= MIME-Version: 1.0 In-Reply-To: <20171220151331.GA3413940@devbig577.frc2.facebook.com> References: <39625861-99c0-1c15-08a6-49b9d678c4c2@redhat.com> <20171213152914.GN3919388@devbig577.frc2.facebook.com> <121dc065-89ba-98ab-68ff-e86f1a636b06@redhat.com> <20171213213756.GX3919388@devbig577.frc2.facebook.com> <20171220151331.GA3413940@devbig577.frc2.facebook.com> From: Georgios Amanakis Date: Wed, 20 Dec 2017 11:38:06 -0500 Message-ID: Subject: Re: [PATCH cgroup/for-4.15-fixes] cgroup: fix css_task_iter crash on CSS_TASK_ITER_PROC To: Tejun Heo Cc: Laura Abbott , Zefan Li , linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, regressions@leemhuis.info, Bronek Kozicki Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5216 Lines: 126 This resolves the bug for me. Thank you, George On Wed, Dec 20, 2017 at 10:13 AM, Tejun Heo wrote: > Hello, > > Applied the following to cgroup/for-4.15-fixes. Will push out to > linus later this week. I could reproduce the problem reliably and am > pretty sure this is the right fix but I'd greatly appreciate if you > guys can confirm the fix too. > > Thank you very much. > > ------ 8< ------ > From 74d0833c659a8a54735e5efdd44f4b225af68586 Mon Sep 17 00:00:00 2001 > From: Tejun Heo > Date: Wed, 20 Dec 2017 07:09:19 -0800 > > While teaching css_task_iter to handle skipping over tasks which > aren't group leaders, bc2fb7ed089f ("cgroup: add @flags to > css_task_iter_start() and implement CSS_TASK_ITER_PROCS") introduced a > silly bug. > > CSS_TASK_ITER_PROCS is implemented by repeating > css_task_iter_advance() while the advanced cursor is pointing to a > non-leader thread. However, the cursor variable, @l, wasn't updated > when the iteration has to advance to the next css_set and the > following repetition would operate on the terminal @l from the > previous iteration which isn't pointing to a valid task leading to > oopses like the following or infinite looping. > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000254 > IP: __task_pid_nr_ns+0xc7/0xf0 > PGD 0 P4D 0 > Oops: 0000 [#1] SMP > ... > CPU: 2 PID: 1 Comm: systemd Not tainted 4.14.4-200.fc26.x86_64 #1 > Hardware name: System manufacturer System Product Name/PRIME B350M-A, BIOS 3203 11/09/2017 > task: ffff88c4baee8000 task.stack: ffff96d5c3158000 > RIP: 0010:__task_pid_nr_ns+0xc7/0xf0 > RSP: 0018:ffff96d5c315bd50 EFLAGS: 00010206 > RAX: 0000000000000000 RBX: ffff88c4b68c6000 RCX: 0000000000000250 > RDX: ffffffffa5e47960 RSI: 0000000000000000 RDI: ffff88c490f6ab00 > RBP: ffff96d5c315bd50 R08: 0000000000001000 R09: 0000000000000005 > R10: ffff88c4be006b80 R11: ffff88c42f1b8004 R12: ffff96d5c315bf18 > R13: ffff88c42d7dd200 R14: ffff88c490f6a510 R15: ffff88c4b68c6000 > FS: 00007f9446f8ea00(0000) GS:ffff88c4be680000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000254 CR3: 00000007f956f000 CR4: 00000000003406e0 > Call Trace: > cgroup_procs_show+0x19/0x30 > cgroup_seqfile_show+0x4c/0xb0 > kernfs_seq_show+0x21/0x30 > seq_read+0x2ec/0x3f0 > kernfs_fop_read+0x134/0x180 > __vfs_read+0x37/0x160 > ? security_file_permission+0x9b/0xc0 > vfs_read+0x8e/0x130 > SyS_read+0x55/0xc0 > entry_SYSCALL_64_fastpath+0x1a/0xa5 > RIP: 0033:0x7f94455f942d > RSP: 002b:00007ffe81ba2d00 EFLAGS: 00000293 ORIG_RAX: 0000000000000000 > RAX: ffffffffffffffda RBX: 00005574e2233f00 RCX: 00007f94455f942d > RDX: 0000000000001000 RSI: 00005574e2321a90 RDI: 000000000000002b > RBP: 0000000000000000 R08: 00005574e2321a90 R09: 00005574e231de60 > R10: 00007f94458c8b38 R11: 0000000000000293 R12: 00007f94458c8ae0 > R13: 00007ffe81ba3800 R14: 0000000000000000 R15: 00005574e2116560 > Code: 04 74 0e 89 f6 48 8d 04 76 48 8d 04 c5 f0 05 00 00 48 8b bf b8 05 00 00 48 01 c7 31 c0 48 8b 0f 48 85 c9 74 18 8b b2 30 08 00 00 <3b> 71 04 77 0d 48 c1 e6 05 48 01 f1 48 3b 51 38 74 09 5d c3 8b > RIP: __task_pid_nr_ns+0xc7/0xf0 RSP: ffff96d5c315bd50 > > Fix it by moving the initialization of the cursor below the repeat > label. While at it, rename it to @next for readability. > > Signed-off-by: Tejun Heo > Fixes: bc2fb7ed089f ("cgroup: add @flags to css_task_iter_start() and implement CSS_TASK_ITER_PROCS") > Cc: stable@vger.kernel.org # v4.14+ > Reported-by: Laura Abbott > Reported-by: Bronek Kozicki > Reported-by: George Amanakis > Signed-off-by: Tejun Heo > --- > kernel/cgroup/cgroup.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c > index f4c2f8c..2cf06c2 100644 > --- a/kernel/cgroup/cgroup.c > +++ b/kernel/cgroup/cgroup.c > @@ -4125,26 +4125,24 @@ static void css_task_iter_advance_css_set(struct css_task_iter *it) > > static void css_task_iter_advance(struct css_task_iter *it) > { > - struct list_head *l = it->task_pos; > + struct list_head *next; > > lockdep_assert_held(&css_set_lock); > - WARN_ON_ONCE(!l); > - > repeat: > /* > * Advance iterator to find next entry. cset->tasks is consumed > * first and then ->mg_tasks. After ->mg_tasks, we move onto the > * next cset. > */ > - l = l->next; > + next = it->task_pos->next; > > - if (l == it->tasks_head) > - l = it->mg_tasks_head->next; > + if (next == it->tasks_head) > + next = it->mg_tasks_head->next; > > - if (l == it->mg_tasks_head) > + if (next == it->mg_tasks_head) > css_task_iter_advance_css_set(it); > else > - it->task_pos = l; > + it->task_pos = next; > > /* if PROCS, skip over tasks which aren't group leaders */ > if ((it->flags & CSS_TASK_ITER_PROCS) && it->task_pos && > -- > 2.9.5 >