linux-kernel - Re: [PATCH 1/3] fix setsid() for sub-namespace /sbin/init

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: <m1r6icn0s9.fsf@ebiederm.dsl.xmission.com>
Date:	Mon, 26 Nov 2007 14:40:22 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Oleg Nesterov <oleg@...sign.ru>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Pavel Emelyanov <xemul@...nvz.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] fix setsid() for sub-namespace /sbin/init

Oleg Nesterov <oleg@...sign.ru> writes:

> On 11/26, Eric W. Biederman wrote:
>>
>> Oleg Nesterov <oleg@...sign.ru> writes:
>> 
>> > sys_setsid() still deals with pid_t's from the global namespace. This means
>> > that the "session > 1" check can't help for sub-namespace init, setsid()
> can't
>> > succeed because copy_process(CLONE_NEWPID) populates PIDTYPE_PGID/SID links.
>> 
>> We can do even better.  We can remove the misguided code from
>> copy_process(CLONE_NEWPID) that populates the PIDTYPE_PGID/SID links
>> and generally does set setsid by hand,
>
> Yes you are right. IIRC there was a patch from you, but I didn't follow the
> discussion, sorry, so I don't know what was the verdict.

Since session == pgrp == 0 is the historical start condition for /sbin/init there
is no problem from the session perspective, it in fact is better.

The only case that might have cared was setting si_pid when sending signals,
and it turns out it is both simple and necessary to handle that case across
namespaces anyway.

So there is no reason not to handle this.

> If we remove that "almost setsid" from copy_process(), we can remove the fat
> comment and the "session != 1" chunk from setsid().
>
>> and the code from kernel_init
>> that call set_special_pid(), allowing us to remove the special case
>> entirely.
>
> This is different, perhaps we can keep this call. kernel_thread(kernel_init)
> attaches /sbin/init to init_struct_pid. Nothing bad, and a "good" init should
> do setsid() anyway. But who knows? Some special environment may expect that
> getpgrp() != 0. Not that I really disagree on this issue though.

init starting with session == pgrp == 0 is historical linux behavior.  I consider
the current 2.6 behavior a temporary aberation from the historical linux behavior.

sysvinit does call setsid.  And nothing really bad will happen if someone forgets
to call setsid, in some obscure version of init.

Plus once we do this the code will be easier to maintain because we have
removed one obscure special case.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/