linux-kernel - Re: [PATCH 03/10] usb: catch attempts to submit urbs with a vmalloc'd transfer buffer

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]

Date:	Thu, 8 May 2014 10:05:56 -0700
From:	Dan Williams <dan.j.williams@...el.com>
To:	Joe Perches <joe@...ches.com>
Cc:	Mathias Nyman <mathias.nyman@...ux.intel.com>,
	Greg KH <gregkh@...uxfoundation.org>,
	USB list <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Sarah Sharp <sarah.a.sharp@...ux.intel.com>,
	Alan Stern <stern@...land.harvard.edu>
Subject: Re: [PATCH 03/10] usb: catch attempts to submit urbs with a vmalloc'd
 transfer buffer

On Thu, May 8, 2014 at 9:47 AM, Joe Perches <joe@...ches.com> wrote:
> On Thu, 2014-05-08 at 19:25 +0300, Mathias Nyman wrote:
>> Save someone else the debug cycles of figuring out why a driver's
>> transfer request is failing or causing undefined system behavior.
>> Buffers submitted for dma must come from GFP allocated / DMA-able
>> memory.
>>
>> Return -EAGAIN matching the return value for dma_mapping_error() cases.
> []
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> []
>> @@ -1502,6 +1502,9 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
>>                                       ret = -EAGAIN;
>>                               else
>>                                       urb->transfer_flags |= URB_DMA_MAP_PAGE;
>> +                     } else if (is_vmalloc_addr(urb->transfer_buffer)) {
>> +                             WARN_ONCE(1, "transfer buffer not dma capable\n");
>> +                             ret = -EAGAIN;
>>                       } else {
>>                               urb->transfer_dma = dma_map_single(
>>                                               hcd->self.controller,
>
> Perhaps this could be #ifdef'd here or moved to and
> tested in dma_map_single/dma_map_single_attr instead.
>

What problem are you trying to solve?  Adding it to dma_map() means
incurring the overhead of checking on every call and it would be akin
to adding dma_mapping_error() to dma_map().  Otherwise, if it is
ifdef'd that will miss the very same driver developers that didn't
know they couldn't do that.  USB core has already committed to
validating the input buffers in this routine, what's wrong with one
more check?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/