linux-kernel - Re: [PATCH v2] usb: core: setup dma_pfn_offset for USB devices and, interfaces

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3375606.ofT8N3j4Qu@wuerfel>
Date:   Mon, 12 Sep 2016 16:34:40 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Alan Stern <stern@...land.harvard.edu>
Cc:     Roger Quadros <rogerq@...com>, gregkh@...uxfoundation.org,
        ssantosh@...nel.org, grygorii.strashko@...com,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] usb: core: setup dma_pfn_offset for USB devices and, interfaces

On Monday, September 12, 2016 9:09:16 AM CEST Alan Stern wrote:
> > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> > index 0406a59..66364ea 100644
> > --- a/drivers/usb/core/message.c
> > +++ b/drivers/usb/core/message.c
> > @@ -1863,6 +1863,12 @@ free_interfaces:
> >               intf->dev.type = &usb_if_device_type;
> >               intf->dev.groups = usb_interface_groups;
> >               intf->dev.dma_mask = dev->dev.dma_mask;
> > +             /* Propagate dma_pfn_offset to USB interface.
> > +              * This is especially required by mass storage interface
> > +              * which relies on SCSI layer and scsi_calculate_bounce_limit()
> > +              * to set the bounce buffer limit based on dma_pfn_offset.
> > +              */
> > +             intf->dev.dma_pfn_offset = dev->dev.dma_pfn_offset;
> 
> I really think this comment isn't necessary.  It seems pretty obvious 
> that if you're copying DMA-related fields from one device to another 
> then you'll want to copy the dma_pfn_offset along with everything else.
> No?
> 
> The explanation in the Changelog is enough, IMO.

I asked for a code comment here as what we are doing here is a bit
fishy, but the comment (evidently) doesn't quite capture it. I would
add (above the dma_mask assignment) something like

	/*
	 * Fake a dma_mask/offset for the USB device:
	 * We cannot really use the dma-mapping API (dma_alloc_* and
	 * dma_map_*) for USB devices but instead need to use
	 * usb_alloc_coherent and pass data in 'urb's, but some subsystems
	 * manually look into the mask/offset pair to determine whether
	 * they need bounce buffers.
	 * Note: calling dma_set_mask() on a USB device would set the
	 * mask for the entire HCD, so don't do that.
	 */

	Arnd