From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: No check of the size passed to unmap_single in swiotlb Date: Mon, 20 Nov 2017 11:26:52 -0500 Message-ID: <20171120162652.GR24312@char.us.oracle.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Eric Yang , iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Cc: Daniel Borkmann , Kees Cook , Geert Uytterhoeven , Greg Kroah-Hartman , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , David Miller , Al Viro , Andrey Ryabinin , Andrew Morton , Ingo Molnar List-Id: iommu@lists.linux-foundation.org On Mon, Nov 20, 2017 at 08:17:14AM +0000, Eric Yang wrote: > Hi all, Hi! > > 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? > > [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