linux-kernel - Re: dmapool regression in next

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:   Thu, 6 Dec 2018 10:11:58 -0500
From:   Tony Battersby <tonyb@...ernetics.com>
To:     Krzysztof Kozlowski <krzk@...nel.org>, tony@...mide.com
Cc:     akpm@...ux-foundation.org, Stephen Rothwell <sfr@...b.auug.org.au>,
        andy.shevchenko@...il.com, hch@....de, john.garry@...wei.com,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        linux@...linux.org.uk, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-omap@...r.kernel.org
Subject: Re: dmapool regression in next

On 12/6/18 4:25 AM, Krzysztof Kozlowski wrote:
> On Thu, 6 Dec 2018 at 02:31, Tony Lindgren <tony@...mide.com> wrote:
>> Hi,
>>
>> Looks like with commit 26abe88e830d ("mm/dmapool.c: improve scalability
>> of dma_pool_free()") I'm now getting spammed with lots of "(bad vaddr)"
>> on at least omap4 pandaboard, see below.
>>
>> Any ideas what might be going wrong?
>>
>> Regards,
>>
>> Tony
>>
>> 8< ---------------------
>> omap-dma-engine 4a056000.dma-controller: dma_pool_free 4a056000.dma-controller, (ptrval) (bad vaddr)/0xbe800000
>> omap-dma-engine 4a056000.dma-controller: dma_pool_free 4a056000.dma-controller, (ptrval) (bad vaddr)/0xbe80001c
>> omap-dma-engine 4a056000.dma-controller: dma_pool_free 4a056000.dma-controller, (ptrval) (bad vaddr)/0xbe800038
>> ...
> I see it as well on all my Exynos boards, since yesterday's next. In
> my case it is the USB EHCI driver:
> exynos-ehci 12110000.usb: dma_pool_free ehci_qtd, (ptrval) (bad
> vaddr)/0xb8844180
> Full log here:
> https://krzk.eu/#/builders/1/builds/2937/steps/12/logs/serial0
>
> Best regards,
> Krzysztof
>
Here is the prototype:

void dma_pool_free(struct dma_pool *pool, void *vaddr, dma_addr_t dma);

With the old code, the 'dma' value had to be correct for use with
pool_find_page(), or else you would get an error.  If the 'vaddr' value
was incorrect, it would corrupt the dmapool freelist, but you wouldn't
get an error unless DMAPOOL_DEBUG was enabled.

With my patch applied, 'vaddr' has to be correct for virt_to_page().  My
code also checks that 'dma' is consistent with 'vaddr' even if
DMAPOOL_DEBUG is disabled, since the check is fast and it will prevent
problems like this in the future.

So if a buggy driver passes in a good value for 'dma' but a bad value
for 'vaddr', then it may have appeared to work previously (but with
possible data corruption, depending on the circumstances), but my patch
will expose the problem.  You can confirm by reverting my dmapool
patches and enabling DMAPOOL_DEBUG, which is at the top of mm/dmapool.c:

#if defined(CONFIG_DEBUG_SLAB) || defined(CONFIG_SLUB_DEBUG_ON)
#define DMAPOOL_DEBUG 1
#endif

Tony Battersby