linux-kernel - Re: [GIT PULL] userns fixes for 4.17-rc2

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] [day] [month] [year] [list]

Date:   Tue, 26 Jun 2018 06:39:11 +0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     Linux Containers <containers@...ts.linux-foundation.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        astrachan@...gle.com, Andrew Morton <akpm@...ux-foundation.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        David Howells <dhowells@...hat.com>,
        Oleg Nesterov <oleg@...hat.com>,
        Alexey Dobriyan <adobriyan@...il.com>
Subject: Re: [GIT PULL] userns fixes for 4.17-rc2

On Tue, Jun 26, 2018 at 4:15 AM Eric W. Biederman <ebiederm@...ssion.com> wrote:
>
> I don't understand the feedback about only accepting something like this
> during the merge window.  The entire point of my change was to remove
> subtlety.  The code was very straight forward to test.

But it *doesn't* remove subtlety - it's just subtle in a different
way. Now it's subtle in the odd pid counting instead, and how  it
generates temporary kernel mounts all the time.

And the problem with the old code apparently went unnoticed for years,
and only hits some very odd cases. So there's real risk from the
hange.

> This is a silent regression of a security feature so it is possible some
> people have upgraded their kernel and not noticed the regression but are
> affected by the information leak not honoring hidepid introduces.  That
> seems to me to be a candidate for stable and thus an rc kernel.

Definitely not in the form you posted it. It's simply not obvious enough.

> Would you prefer a patch that does less towards fixing the root cause
> for now and to be backported to stable?

It's not about whether this is "root cause" or not. It's simply about
the changes being subtle and unclear, and the commits mixing things up
and not being legible enough that anybody can just go "yeah, that's
safe and obviously a fix".

The odd pid counting optimization wasn't even *explained* for
chrissake.  Never mind that the sysctl access code was mixed in with
the other changes.

So the way to get this merged is to make each step obvious and
independent. At that point I may go "yeah, this fixes a real issue and
doesn't introduce new ones".

As it is, it's not at all obvious that it doesn't introduce new issues.

                Linus