| 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
| ||
|
Message-ID: <VI1PR04MB30718EF84BD5305E67A5582A8C210@VI1PR04MB3071.eurprd04.prod.outlook.com>
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