Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
MIME-Version: 1.0
References: <20231011065446.53034-1-cuiyunhui@bytedance.com>
 <87sf6gcyb3.fsf@email.froward.int.ebiederm.org> <CAEEQ3wm8oE1dXo7AuU3gY90oF_G21OqNZwnkqhhd1-_j4wWbYQ@mail.gmail.com>
 <87r0lyad40.fsf@email.froward.int.ebiederm.org>
In-Reply-To: <87r0lyad40.fsf@email.froward.int.ebiederm.org>
From:   yunhui cui <cuiyunhui@bytedance.com>
Date:   Sat, 14 Oct 2023 11:41:06 +0800
Message-ID: <CAEEQ3wmerA8p2qpEAn9pGW5BW35777VFwabusyFTAEyaf-0JPg@mail.gmail.com>
Subject: Re: [External] Re: [PATCH] pid_ns: support pidns switching between sibling
To:     "Eric W. Biederman" <ebiederm@xmission.com>
Cc:     akpm@linux-foundation.org, keescook@chromium.org,
        brauner@kernel.org, jeffxu@google.com, frederic@kernel.org,
        mcgrof@kernel.org, cyphar@cyphar.com, rongtao@cestc.cn,
        linux-kernel@vger.kernel.org,
        Linux Containers <containers@lists.linux.dev>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Eric=EF=BC=8C

On Fri, Oct 13, 2023 at 9:04=E2=80=AFPM Eric W. Biederman <ebiederm@xmissio=
n.com> wrote:
>
> yunhui cui <cuiyunhui@bytedance.com> writes:
>
> > Hi Eric,
> >
> > On Thu, Oct 12, 2023 at 11:31=E2=80=AFAM Eric W. Biederman
> > <ebiederm@xmission.com> wrote:
> >>
> >> The check you are deleting is what verifies the pid namespaces you are
> >> attempting to change pid_ns_for_children to is a member of the tasks
> >> current pid namespace (aka task_active_pid_ns).
> >>
> >>
> >> There is a perfectly good comment describing why what you are attempti=
ng
> >> to do is unsupportable.
> >>
> >>         /*
> >>          * Only allow entering the current active pid namespace
> >>          * or a child of the current active pid namespace.
> >>          *
> >>          * This is required for fork to return a usable pid value and
> >>          * this maintains the property that processes and their
> >>          * children can not escape their current pid namespace.
> >>          */
> >>
> >>
> >> If you pick a pid namespace that does not meet the restrictions you ar=
e
> >> removing the pid of the new child can not be mapped into the pid
> >> namespace of the parent that called setns.
> >>
> >> AKA the following code can not work.
> >>
> >> pid =3D fork();
> >> if (!pid) {
> >>         /* child */
> >>         do_something();
> >>         _exit(0);
> >> }
> >> waitpid(pid);
> >
> > Sorry, I don't understand what you mean here.
>
> What I mean is that if your simple patch was adopted,
> then the classic way of controlling a fork would fail.
>
>         pid =3D fork()
>         ^--------------- Would return 0 for both parent and child
>         ^--------------- Look at pid_nr_ns to understand.
>         if (!pid() {
>                 /* child */
>                 do_something();
>                 _exit(0);
>         }
>         waitpid(pid);

okay, The reason here is that pid_nr_ns has no pid in the current
pidns of the child process, and returns 0.
Can this also support sibling traversal? If so, it means that the
process also has a pid in its sibling's pidns.


>
> For your use case there are more serious problems as well.  The entire
> process hierarchy built would be incorrect.   Which means children
> signaling parents when they exit would be incorrect, and that parents
> would not be able to wait on their children.

Therefore, support for slibing pidns must be added to the entire logic of p=
idns.
Do you have any plans to support this, or what are the good reasons
for not supporting it?

Thanks,
Yunhui