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  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, 23 Nov 2017 07:47:40 +0000
From:   Eric Yang <yu.yang_3@....com>
To:     Robin Murphy <robin.murphy@....com>,
        Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
        "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>
CC:     Daniel Borkmann <daniel@...earbox.net>,
        Kees Cook <keescook@...omium.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Al Viro <viro@...iv.linux.org.uk>,
        Andrey Ryabinin <aryabinin@...tuozzo.com>,
        "Andrew Morton" <akpm@...ux-foundation.org>,
        Ingo Molnar <mingo@...nel.org>
Subject: RE: No check of the size passed to unmap_single in swiotlb



> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@....com]
> Sent: Wednesday, November 22, 2017 9:15 PM
> To: Eric Yang <yu.yang_3@....com>; Konrad Rzeszutek Wilk
> <konrad.wilk@...cle.com>; iommu@...ts.linux-foundation.org
> Cc: Daniel Borkmann <daniel@...earbox.net>; Kees Cook
> <keescook@...omium.org>; Geert Uytterhoeven <geert+renesas@...der.be>;
> Greg Kroah-Hartman <gregkh@...uxfoundation.org>; linux-
> kernel@...r.kernel.org; David Miller <davem@...emloft.net>; Al Viro
> <viro@...iv.linux.org.uk>; Andrey Ryabinin <aryabinin@...tuozzo.com>;
> Andrew Morton <akpm@...ux-foundation.org>; Ingo Molnar
> <mingo@...nel.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.wilk@...cle.com]
> >> Sent: Tuesday, November 21, 2017 12:27 AM
> >> To: Eric Yang <yu.yang_3@....com>; iommu@...ts.linux-foundation.org
> >> Cc: linux-kernel@...r.kernel.org; Greg Kroah-Hartman
> >> <gregkh@...uxfoundation.org>; Andrew Morton
> >> <akpm@...ux-foundation.org>; Andrey Ryabinin
> >> <aryabinin@...tuozzo.com>; David Miller <davem@...emloft.net>; Ingo
> >> Molnar <mingo@...nel.org>; Geert Uytterhoeven
> >> <geert+renesas@...der.be>; Al Viro <viro@...iv.linux.org.uk>; Kees
> >> Cook <keescook@...omium.org>; Daniel Borkmann <daniel@...earbox.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@...an 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
> > iommu@...ts.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
> >

Powered by blists - more mailing lists