linux-kernel - Re: sched/autogroup: race if !sysctl_sched_autogroup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 11 Nov 2016 17:57:43 +0100
From:   Oleg Nesterov <oleg@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Ingo Molnar <mingo@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Mike Galbraith <efault@....de>, hartsjc@...hat.com,
        vbendel@...hat.com, vlovejoy@...hat.com,
        linux-kernel@...r.kernel.org
Subject: Re: sched/autogroup: race if !sysctl_sched_autogroup_enabled ?

On 11/10, Oleg Nesterov wrote:
>
> And the 3rd case which I didn't think about yesterday. And now I really hope
> it can explain the vmcore we have.
>
> If sysctl_sched_autogroup_enabled was enabled and then disabled, it is
> possible that the "autogrouped" process runs with ag->kref.refcount == 1,
> and if it does setsid() it frees its active task_group.

And yet another problem ;)

The exiting thread must call sched_move_task() somewhere before exit_notify()
or it can run with the freed task_group() after that. And this means that the
no-longer-needed PF_EXITING check in task_wants_autogroup() will be needed
again. Simple, but needs the comments/changelog...

> So I am going to send the patch which simply moves the sysctl check from
> autogroup_move_group() to sched_autogroup_create_attach(), but perhaps I
> should split this change?
>
> I mean, the first patch for -stable could just remove the current check,
> the 2nd one will add it into sched_autogroup_create_attach().

No, this is not enough, see above.

I am starting to think that we should just move ->autogroup from signal_struct
to task_struct. This will simplify the code and fix all these problems. But
I need a simple fix for backporting anyway.

Oleg.