linux-kernel - Re: [PATCH] prctl: propagate has_child

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]

Message-ID: <20170123160655.GA1857@redhat.com>
Date:   Mon, 23 Jan 2017 17:06:55 +0100
From:   Oleg Nesterov <oleg@...hat.com>
To:     Pavel Tikhomirov <ptikhomirov@...tuozzo.com>
Cc:     Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Cyrill Gorcunov <gorcunov@...nvz.org>,
        John Stultz <john.stultz@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Nicolas Pitre <nicolas.pitre@...aro.org>,
        Michal Hocko <mhocko@...e.com>,
        Stanislav Kinsburskiy <skinsbursky@...tuozzo.com>,
        Mateusz Guzik <mguzik@...hat.com>,
        linux-kernel@...r.kernel.org,
        Pavel Emelyanov <xemul@...tuozzo.com>,
        Konstantin Khorenko <khorenko@...tuozzo.com>,
        Lennart Poettering <lennart@...ttering.net>
Subject: Re: [PATCH] prctl: propagate has_child_subreaper flag to every
 descendant

On 01/23, Pavel Tikhomirov wrote:
>
> >IOW. Currently CRIU can't restore the process tree with the same
> >has_child_subreaper bits if some process forks before
> >prctl(PR_SET_CHILD_SUBREAPER). It restores the tree as if prctl()
> >was called before the 1st fork.
> >
> >So you change the semantics of PR_SET_CHILD_SUBREAPER and now CRIU
> >is fine simply because you remove this feature: the sub-reaper can
> >no longer pre-fork the children which should reparent to the previous
> >reaper.
> >
> >I won't really argure, but I am not sure this is good idea...
>
> If one task uses these feature now it must be very carefull: if some our
> ancestor have enabled is_child_subreaper somewhere up the tree, forked our
> tree and after that disabled is_child_subreaper, so we already have has-flag
> and all children will inherit has-flag irrelevant to what is our order of
> fork/prctl-ing to become subreaper.

Agreed.

So let me reword my initial question, why did you make this patch? Did you
actually hit a case when a child of is_child_subreaper process doesn't have
has_child_subreaper bit set?

If yes, then perhaps that application has a reason to do this and your patch
can break it? If no, then you can probably forget this until you have a CRIU
bug report ;)

But let me repeat, I won't really argue. And I even agree that this change
makes the semantics of PR_SET_CHILD_SUBREAPER more clear, just I am always
nervous when we add the subtle user-visible changes like this, and I greatly
misundestood the changelog as if CRIU needs to do prctl(SET_CHILD_SUBREAPER)
after it has already restored the process tree and this can't work even in
the common case.

Oleg.