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

[Dan Williams]

On Thu, May 8, 2014 at 9:47 AM, Joe Perches <joe@xxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/