lkml.org 
[lkml]   [2017]   [Nov]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRE: No check of the size passed to unmap_single in swiotlb
Date


> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Tuesday, November 21, 2017 12:27 AM
> To: Eric Yang <yu.yang_3@nxp.com>; iommu@lists.linux-foundation.org
> Cc: linux-kernel@vger.kernel.org; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Andrew Morton <akpm@linux-foundation.org>;
> Andrey Ryabinin <aryabinin@virtuozzo.com>; David Miller
> <davem@davemloft.net>; Ingo Molnar <mingo@kernel.org>; Geert
> Uytterhoeven <geert+renesas@glider.be>; Al Viro <viro@zeniv.linux.org.uk>;
> Kees Cook <keescook@chromium.org>; Daniel Borkmann
> <daniel@iogearbox.net>
> Subject: Re: No check of the size passed to unmap_single in swiotlb
>
> On Mon, Nov 20, 2017 at 08:17:14AM +0000, Eric Yang wrote:
> > Hi all,
>
> Hi!

Hi Konrad,
> >
> > During debug a device only support 32bits DMA(Qualcomm Atheros AP) in our
> LS1043A 64bits ARM SOC, we found that the invoke of dma_unmap_single -->
> swiotlb_tbl_unmap_single will unmap the passed "size" anyway even when the
> "size" is incorrect.
> >
> > If the size is larger than it should, the extra entries in io_tlb_orig_addr array
> will be refilled by INVALID_PHYS_ADDR, and it will cause the bounce buffer copy
> not happen when the one who really used the mis-freed entries doing DMA data
> transfers, and this will cause further unknow behaviors.
> >
> > Here we just fix it temporarily by adding a judge of the "size" in the
> swiotlb_tbl_unmap_single, if it is larger than it deserves, just unmap the right
> size only. Like the code:
>
> Did the DMA debug API (CONFIG_DMA_API_DEBUG) help in figuring this issue as
> well?
>

I have just enabled this config and move off the kernel fix and test, there seems the debug API
has no output for this size incorrect issue. For I only enable the config and no other operations
and my kernel is 4.9, do I missed something?

I confirm that the debug API is working, for there are other drivers triggered its warning output
like:

caam_jr 1710000.jr: DMA-API: device driver tries to free DMA memory it has not allocated
[device address=0x6800458400000000]

> >
> > [yangyu@titan dash-lts]$ git diff ./lib/swiotlb.c diff --git
> > a/lib/swiotlb.c b/lib/swiotlb.c index ad1d2962d129..58c97ede9d78
> > 100644
> > --- a/lib/swiotlb.c
> > +++ b/lib/swiotlb.c
> > @@ -591,7 +591,10 @@ void swiotlb_tbl_unmap_single(struct device *hwdev,
> phys_addr_t tlb_addr,
> > */
> > for (i = index + nslots - 1; i >= index; i--) {
> > io_tlb_list[i] = ++count;
> > - io_tlb_orig_addr[i] = INVALID_PHYS_ADDR;
> > + if(io_tlb_orig_addr[i] != orig_addr)
> > + printk("======size wrong, ally down ally down!===\n");
> > + else
> > + io_tlb_orig_addr[i] = INVALID_PHYS_ADDR;
> > }
> > /*
> > * Step 2: merge the returned slots with the preceding
> > slots,
> >
> > Although pass a right size of DMA buffer is the responsibility of the drivers, but
> Is it useful to add some size check code to prevent real damage happen?
> >
> > Regards,
> > Eric

\
 
 \ /
  Last update: 2017-11-22 04:24    [W:0.091 / U:3.468 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site