linux-kernel - Re: [External] Re: [PATCH] pid_ns: support pidns switching between sibling

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: <CAEEQ3wmerA8p2qpEAn9pGW5BW35777VFwabusyFTAEyaf-0JPg@mail.gmail.com>
Date:   Sat, 14 Oct 2023 11:41:06 +0800
From:   yunhui cui <cuiyunhui@...edance.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     akpm@...ux-foundation.org, keescook@...omium.org,
        brauner@...nel.org, jeffxu@...gle.com, frederic@...nel.org,
        mcgrof@...nel.org, cyphar@...har.com, rongtao@...tc.cn,
        linux-kernel@...r.kernel.org,
        Linux Containers <containers@...ts.linux.dev>
Subject: Re: [External] Re: [PATCH] pid_ns: support pidns switching between sibling

Hi Eric，

On Fri, Oct 13, 2023 at 9:04 PM Eric W. Biederman <ebiederm@...ssion.com> wrote:
>
> yunhui cui <cuiyunhui@...edance.com> writes:
>
> > Hi Eric,
> >
> > On Thu, Oct 12, 2023 at 11:31 AM Eric W. Biederman
> > <ebiederm@...ssion.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 attempting
> >> 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 are
> >> 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 = 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 = 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 pidns.
Do you have any plans to support this, or what are the good reasons
for not supporting it?

Thanks,
Yunhui