From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751092AbdAWLzi (ORCPT ); Mon, 23 Jan 2017 06:55:38 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50174 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750714AbdAWLzh (ORCPT ); Mon, 23 Jan 2017 06:55:37 -0500 Date: Mon, 23 Jan 2017 12:55:34 +0100 From: Oleg Nesterov To: Pavel Tikhomirov Cc: Ingo Molnar , Peter Zijlstra , Andrew Morton , Cyrill Gorcunov , John Stultz , Thomas Gleixner , Nicolas Pitre , Michal Hocko , Stanislav Kinsburskiy , Mateusz Guzik , linux-kernel@vger.kernel.org, Pavel Emelyanov , Konstantin Khorenko Subject: Re: [PATCH] prctl: propagate has_child_subreaper flag to every descendant Message-ID: <20170123115534.GA11827@redhat.com> References: <20170119164346.4214-1-ptikhomirov@virtuozzo.com> <20170120181359.GA17205@redhat.com> <4908be49-d3c3-366d-0fd1-05249ef4ecef@virtuozzo.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4908be49-d3c3-366d-0fd1-05249ef4ecef@virtuozzo.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Mon, 23 Jan 2017 11:55:38 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/22, Pavel Tikhomirov wrote: > > > > >Hmm. could you explain how this change helps CRIU? I mean, why > >restorer can't do prctl(CHILD_SUBREAPER) before the first fork? > > Imagine we have these tree in pidns: > > 1: has_child_subreaper == 0 && is_child_subreaper == 0 > |-2: has_child_subreaper == 0 && is_child_subreaper == 1 > | |-3: has_child_subreaper == 0 && is_child_subreaper == 0 > | | |-5: has_child_subreaper == 0 && is_child_subreaper == 0 > | |-4: has_child_subreaper == 1 && is_child_subreaper == 0 > | | |-6: has_child_subreaper == 1 && is_child_subreaper == 0 > > before c/r: If 4 dies 6 will reparent to 2, if 3 dies 5 will reparent to 1. > after c/r: (where restorer had is_child_subreaper == 1, everybody in the > tree will have has_child_subreaper == 1) Everybody will reparent to 2. This is clear, but this can only happen if 2 forks 3 and after that sets is_child_subreaper, right? And if someone actually does this then your patch can break this application, no? 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... At least I think this should be clearly documented. > >You don't need this new member and descendants_lock. task_struct has > >the ->real_parent pointer so you can work the tree without recursion. > > Sorry I don't get how I can walk down the tree of all descendants with help > of ->real_parent pointer, can you please point on some example or explain a > bit more? (I see task_is_descendant() in security/yama/yama_lsm.c but we > will need to check it for every process, not only descendants, the latter > can be a lot faster.) I'll send a patch, probably a generic helper makes sense. Btw task_is_descendant() looks wrong at first glance. Oleg.