On 22/11/17 03:23, Eric Yang wrote:


-----Original Message-----
From: Konrad Rzeszutek Wilk [mailto:konrad.w...@oracle.com]
Sent: Tuesday, November 21, 2017 12:27 AM
To: Eric Yang <yu.yan...@nxp.com>; io...@lists.linux-foundation.org
Cc: linux-kernel@vger.kernel.org; Greg Kroah-Hartman
<gre...@linuxfoundation.org>; Andrew Morton <a...@linux-foundation.org>;
Andrey Ryabinin <aryabi...@virtuozzo.com>; David Miller
<da...@davemloft.net>; Ingo Molnar <mi...@kernel.org>; Geert
Uytterhoeven <geert+rene...@glider.be>; Al Viro <v...@zeniv.linux.org.uk>;
Kees Cook <keesc...@chromium.org>; Daniel Borkmann
<dan...@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]

By default, dma-debug will only log the first error it sees - you can adjust this with the "num_errors" or "all_errors" sysfs controls, and use the driver filter to hide errors from other drivers if necessary.

Robin.


[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
_______________________________________________
iommu mailing list
io...@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to