linux-kernel - Re: [PATCH] dma-mapping: Lift address space checks out of debug code

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] [day] [month] [year] [list]

Message-ID: <20191005082754.GB12308@lst.de>
Date:   Sat, 5 Oct 2019 10:27:54 +0200
From:   Christoph Hellwig <hch@....de>
To:     Kees Cook <keescook@...omium.org>
Cc:     Robin Murphy <robin.murphy@....com>,
        Christoph Hellwig <hch@....de>,
        Laura Abbott <labbott@...hat.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Allison Randal <allison@...utok.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Stephen Boyd <swboyd@...omium.org>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Semmle Security Reports <security-reports@...mle.com>,
        iommu@...ts.linux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dma-mapping: Lift address space checks out of debug
 code

On Thu, Oct 03, 2019 at 02:38:43PM -0700, Kees Cook wrote:
> > I think it would be reasonable to pull the is_vmalloc_addr() check inline,
> > as that probably covers 90+% of badness (especially given vmapped stacks),
> > and as you say should be reliably cheap everywhere. Callers are certainly
> > expected to use dma_mapping_error() and handle failure, so refusing to do a
> > bogus mapping operation should be OK API-wise - ultimately if a driver goes
> > ahead and uses DMA_MAPPING_ERROR as an address anyway, that's not likely to
> > be any *more* catastrophic than if it did the same with whatever nonsense
> > virt_to_phys() of a vmalloc address had returned.
> 
> What do you think about the object_is_on_stack() check? That does a
> dereference through "current" to find the stack bounds...

I can be persuaded about just the vmalloc check as people tend to get
a lot of vmalloc alloctions without knowing these days.  But what I'd
really like to see is a new config option that enables relatively
cheap checks without the full dma debugging infrastructure.  That way
you can turn those on at least for all development builds, and can
easily benchmark having the checks vs not.