linux-kernel - Re: [PATCH] scatterlist: Validate page before calling PageSlab()

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: <20191001171220.GF22532@ziepe.ca>
Date:   Tue, 1 Oct 2019 14:12:20 -0300
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Alan Mikhak <alan.mikhak@...ive.com>
Cc:     linux-kernel@...r.kernel.org, martin.petersen@...cle.com,
        alexios.zavras@...el.com, ming.lei@...hat.com,
        gregkh@...uxfoundation.org, tglx@...utronix.de,
        christophe.leroy@....fr, Palmer Dabbelt <palmer@...ive.com>,
        Paul Walmsley <paul.walmsley@...ive.com>
Subject: Re: [PATCH] scatterlist: Validate page before calling PageSlab()

On Tue, Oct 01, 2019 at 10:09:48AM -0700, Alan Mikhak wrote:
> On Tue, Oct 1, 2019 at 5:16 AM Jason Gunthorpe <jgg@...pe.ca> wrote:
> >
> > On Mon, Sep 30, 2019 at 04:22:35PM -0700, Alan Mikhak wrote:
> > > From: Alan Mikhak <alan.mikhak@...ive.com>
> > >
> > > Modify sg_miter_stop() to validate the page pointer
> > > before calling PageSlab(). This check prevents a crash
> > > that will occur if PageSlab() gets called with a page
> > > pointer that is not backed by page struct.
> > >
> > > A virtual address obtained from ioremap() for a physical
> > > address in PCI address space can be assigned to a
> > > scatterlist segment using the public scatterlist API
> > > as in the following example:
> > >
> > > my_sg_set_page(struct scatterlist *sg,
> > >                const void __iomem *ioaddr,
> > >                size_t iosize)
> > > {
> > >       sg_set_page(sg,
> > >               virt_to_page(ioaddr),
> > >               (unsigned int)iosize,
> > >               offset_in_page(ioaddr));
> > >       sg_init_marker(sg, 1);
> > > }
> > >
> > > If the virtual address obtained from ioremap() is not
> > > backed by a page struct, virt_to_page() returns an
> > > invalid page pointer. However, sg_copy_buffer() can
> > > correctly recover the original virtual address. Such
> > > addresses can successfully be assigned to scatterlist
> > > segments to transfer data across the PCI bus with
> > > sg_copy_buffer() if it were not for the crash in
> > > PageSlab() when called by sg_miter_stop().
> >
> > I thought we already agreed in general that putting things that don't
> > have struct page into the scatter list was not allowed?
> >
> > Jason
> 
> Thanks Jason for your comment.
> 
> Cost of adding page structs to a large PCI I/O address range can be
> quite substantial. Allowing PCI I/O pages without page structs may be
> desirable. Perhaps it is worth considering this cost.

This is generally agreed, but nobody has figured out a solution.

> Scatterlist has no problem doing its memcpy() from pages without a
> page struct that were obtained from ioremap(). It is only at

Calling memcpy on pointers from ioremap is very much not allowed. Code
has to use the iomem safe memcpy.

Jason