linux-kernel - Re: [PATCH RFC] setns: return 0 directly if try to reassociate with current namespace

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 08 Oct 2014 10:55:49 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Serge Hallyn <serge.hallyn@...ntu.com>
Cc:	Chen Hanxiao <chenhanxiao@...fujitsu.com>,
	containers@...ts.linux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC] setns: return 0 directly if try to reassociate with current namespace

Serge Hallyn <serge.hallyn@...ntu.com> writes:

> Quoting Chen Hanxiao (chenhanxiao@...fujitsu.com):
>> We could use setns to join the current ns,
>> which did a lot of unnecessary work.
>> This patch will check this senario and
>> return 0 directly.
>> 
>> Signed-off-by: Chen Hanxiao <chenhanxiao@...fujitsu.com>
>
> Plus it's just asking for trouble.
>
> I would ack this, except you need to fclose(file) on the
> return paths.  So just set err = 0 and goto out.

I completely disagree.

Nacked-by: "Eric W. Biederman" <ebiederm@...ssion.com>

This patch adds a new code path to test, and gets that new code path
wrong.  So unless there is a performance advantage for some real world
case I don't see the point.  Is there real software that is rejoining
the a current namespace.

This patch changes the behavior of CLONE_NEWNS (which always does a
chdir and chroot) when you change into the current namespace.

This patch changes the behavior of CLONE_NEWUSER which current errors
out.

This code adds a big switch statement to code that is otherwise table
driven.  With the result that two pieces of code must be looked at
and modified whenever we want to tweak the behavior of setns for a
namespace.

So in general I think this piece of code is a maintenance disaster,
with no apparent redeem virtues.

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/