linux-kernel - Re: [PATCH] x86/mtrr: don't copy out-of-bounds data in mtrr

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: <CA+55aFyVBvW65avVaevMaDA5LuuSrFoOnBY_UBskjCKeC24s4Q@mail.gmail.com>
Date:   Sun, 15 Jul 2018 18:46:03 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Jann Horn <jannh@...gle.com>
Cc:     Ingo Molnar <mingo@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Peter Anvin <hpa@...or.com>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andy Lutomirski <luto@...capital.net>,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH] x86/mtrr: don't copy out-of-bounds data in mtrr_write

On Sun, Jul 15, 2018 at 6:33 PM Jann Horn <jannh@...gle.com> wrote:
>
> +Linus, Andy, Al from the other thread
>
> On Mon, Jul 16, 2018 at 12:03 AM Ingo Molnar <mingo@...nel.org> wrote:
> >
> > BTW., a naive question: would it make sense to simply disallow 'special'
> > fds to be passed to setuid binaries, and fix any user-space that breaks?
> > (i.e. only allow regular files and pipes/sockets.)
>
> If we do that, we'd probably want to do the same for file descriptor
> passing through /dev/binder and SCM_RIGHTS. There are already LSM
> hooks for most of that because SELinux filters these.

I think it's unlikely to make much sense to try to limit fd passing. I
can't really see any sane model for doing so. By definition, the file
descriptor was opened by the "weak" party (non-suid), and dammit,
that's supposed to be the real permission boundary.

So I don't think "don't pass fd to suid binary" makes any sense at
all. It might break things, and all it does is to paper over the real
issue. It smells like a pointless hack that makes little sense.

The fact that we have had problems in this area is sad, but I'm hoping
people are at least a bit more aware of it.

However:

> > Also, don't allow splice() on special files either, except if the driver
> > explicitly opts in to it.
>
> In the thread "[PATCH 24/32] vfs: syscall: Add fsopen() to prepare for
> superblock creation [ver #9]", Andy Lutomirski suggested something
> similar (https://lore.kernel.org/lkml/338BC3C4-F3E7-48F0-A82E-2C7295B6640E@amacapital.net/):
> handler
> | (Al- can’t we just stop allowing splice() at all on things that
> don’t use iov_iter?)
>
> Linus suggested
> (https://lore.kernel.org/lkml/CA+55aFxGqLfu1pjT5421k7KbSd94NWU4fw0H-zJe-qSWwBfAeQ@mail.gmail.com/):
>
> | We could add a FMODE_SPLICE_READ/WRITE bit, and let people opt in to
> | splice. We probably should have.

Right. We already use FMODE_xyz to limit various operations (ie we can
already say "this file is not seekable" etc), and I have to say, the
special magiuc splice() behavior has been an issue.

We did the "FMODE_PREAD/PWRITE" flags exactly because we wanted to
have the right ESPIPE behavior from file descriptors that simply
couldn't do lseek. And splice() is very much another "some file
descriptors do not like it".

So adding FMODE_SPLICE_READ/WRITE and then just forcing subsystems to
opt in to it sounds fairly simple. The people who really want splice
tend to just want pipes, regular files and network sockets, so
starting out with those opted it should pretty much cover it.

            Linus