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]
Date:   Sun, 22 Jan 2023 10:57:14 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.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 Sun, 22 Jan 2023 09:00:04 +0000,
Leon Romanovsky <leon@...nel.org> wrote:
> 
> 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.
> 2. Device can report large table size, kernel will cache it and
> malicious device will reduce it back. It is not handled and will cause
> to kernel crash too.
> 

Indeed, this was my exact reaction reading this patch. This only makes
sure the same (potentially wrong) value is used at all times. So while
this results in a consistent use, this doesn't give much guarantee.

The only way to deal with this is to actually handle the resulting
fault, similar to what the kernel does when accessing userspace. Not
sure how possible this is with something like PCIe.

	M.

-- 
Without deviation from the norm, progress is not possible.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ