linux-kernel - Re: [PATCH 1/2] PCI/MSI: Cache the MSIX table size

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: <Y8/6InIxcpwM5u2M@kroah.com>
Date:   Tue, 24 Jan 2023 16:32:50 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Alexander Shishkin <alexander.shishkin@...ux.intel.com>
Cc:     Leon Romanovsky <leon@...nel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        Marc Zyngier <maz@...nel.org>, darwi@...utronix.de,
        elena.reshetova@...el.com, kirill.shutemov@...ux.intel.com,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        stable@...r.kernel.org
Subject: Re: [PATCH 1/2] PCI/MSI: Cache the MSIX table size

On Tue, Jan 24, 2023 at 01:52:37PM +0200, Alexander Shishkin wrote:
> Leon Romanovsky <leon@...nel.org> writes:
> 
> > On Thu, Jan 19, 2023 at 07:06:32PM +0200, Alexander Shishkin wrote:
> >> A malicious device can change its MSIX table size between the table
> >> ioremap() and subsequent accesses, resulting in a kernel page fault in
> >> pci_write_msg_msix().
> >> 
> >> To avoid this, cache the table size observed at the moment of table
> >> ioremap() and use the cached value. This, however, does not help drivers
> >> that peek at the PCIE_MSIX_FLAGS register directly.
> >> 
> >> Signed-off-by: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> >> Reviewed-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
> >> Cc: stable@...r.kernel.org
> >> ---
> >>  drivers/pci/msi/api.c | 7 ++++++-
> >>  drivers/pci/msi/msi.c | 2 +-
> >>  include/linux/pci.h   | 1 +
> >>  3 files changed, 8 insertions(+), 2 deletions(-)
> >
> > I'm not security expert here, but not sure that this protects from anything.
> > 1. Kernel relies on working and not-malicious HW. There are gazillion ways
> > to cause crashes other than changing MSI-X.
> 
> This particular bug was preventing our fuzzing from going deeper into
> the code and reaching some more of the aforementioned gazillion bugs.

As per our documentation, if you are "fixing" things based on a tool you
have, you HAVE TO document that in the changelog.  Otherwise we just get
to reject the patch flat out (hint, this has caused more than one group
of developers to just be flat out banned for ignoring...)

And what kind of tool is now fuzzing PCI config accesses with
"malicious" devices?  Again, this is NOT something that Linux currently
even attempts to want to protect itself against.  If you are wanting to
change that model, wonderful, let's discuss that and work on defining
the scope of your new security threat model and go from there.  Don't
throw random patches at us and expect us to think they are even valid.

Again, Linux trusts PCI devices.  If you don't want to trust PCI
devices, we already have a framework in place to prevent that which is
independant of this area of the PCI code.  Use that, or let's discuss
why that is insufficient.

Note, this is NOT the first time I have told developers from Intel about
this.  Why you all keep ignoring this is beyond me, I think you keep
thinking that if you don't send patches through me, you can just ignore
my statements about this.  Odd...

greg k-h