> -----Original Message-----
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: Wednesday, November 22, 2017 9:15 PM
> To: Eric Yang <yu.yan...@nxp.com>; Konrad Rzeszutek Wilk
> <konrad.w...@oracle.com>; io...@lists.linux-foundation.org
> Cc: Daniel Borkmann <dan...@iogearbox.net>; Kees Cook
> <keesc...@chromium.org>; Geert Uytterhoeven <geert+rene...@glider.be>;
> Greg Kroah-Hartman <gre...@linuxfoundation.org>; linux-
> ker...@vger.kernel.org; David Miller <da...@davemloft.net>; Al Viro
> <v...@zeniv.linux.org.uk>; Andrey Ryabinin <aryabi...@virtuozzo.com>;
> Andrew Morton <a...@linux-foundation.org>; Ingo Molnar
> <mi...@kernel.org>
> Subject: Re: No check of the size passed to unmap_single in swiotlb
>
> 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.
>
Hi Robin,
I've caught the size mismatch warning with the debug API, thanks a lot for the
help.
Regards,
Eric
> >>>
> >>> [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://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> >
> ts.linuxfoundation.org%2Fmailman%2Flistinfo%2Fiommu&data=02%7C01%7Cyu.
> >
> yang_3%40nxp.com%7Cf3851e3c8ae24f341b3608d531ab1165%7C686ea1d3bc
> 2b4c6f
> >
> a92cd99c5c301635%7C0%7C0%7C636469533155971955&sdata=7PnqUlgRlJq2H
> sXmTf
> > CmEUuWaBfUhzSd33UoK4li1Ro%3D&reserved=0
> >
