linux-kernel - Re: [PATCH] fs: don't let getdents return bogus names

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: <CAG48ez1=rW2fBmx0pwu+yQCGpq2Bu7XnMYRkYJt2Dnj5y1USFw@mail.gmail.com>
Date:   Mon, 16 Jul 2018 22:20:24 +0200
From:   Jann Horn <jannh@...gle.com>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     rth@...ddle.net, ink@...assic.park.msu.ru,
        Matt Turner <mattst88@...il.com>,
        linux-fsdevel@...r.kernel.org,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        "Theodore Y. Ts'o" <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        linux-alpha@...r.kernel.org,
        kernel list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] fs: don't let getdents return bogus names

On Mon, Jul 16, 2018 at 9:57 PM Al Viro <viro@...iv.linux.org.uk> wrote:
>
> On Mon, Jul 16, 2018 at 09:48:43PM +0200, Jann Horn wrote:
> > When you e.g. run `find` on a directory for which getdents returns
> > "filenames" that contain slashes, `find` passes those "filenames" back to
> > the kernel, which then interprets them as paths. That could conceivably
> > cause userspace to do something bad when accessing something like an
> > untrusted USB stick, but I'm not aware of any specific example.
> >
> > Instead of returning bogus filenames to userspace, return -EUCLEAN.
>
> Because there's such a lot of userland code that expect and handles that
> error value...

Empirically, for example, "find" seems to handle it (by printing an
error message to its caller).

> I'm not sure if this mitigation is actually better than "just return it
> as-is", TBH.

Turning it into something that behaves approximately like an I/O error
seems way better to me than letting userspace run around wildly and
potentially (depending on the filename and what userspace is trying to
do) mess with unrelated files belonging to entirely different mounts.
For accidental corruption, that's probably not an issue; but when
dealing with something like a malicious vfat filesystem on a USB
stick, I imagine it could be problematic.