On Mon, 11 Nov 2013, Michal Nazarewicz wrote:
> Check gadget.quirk_ep_out_aligned_size to decide if buffer size requires
> to be aligned to maxpacketsize of an out endpoint. ffs_epfile_io() needs
> to pad epout buffer to match above condition if quirk is found.
>
> Signed-off-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
I think this is still wrong.
> @@ -787,7 +788,14 @@ static ssize_t ffs_epfile_io(struct file *file,
>
> /* Allocate & copy */
> if (!halt) {
> - data = kmalloc(len, GFP_KERNEL);
> + /*
> + * Controller requires buffer size to be aligned to
> + * maxpacketsize of an out endpoint.
> + */
> + data_len = read && gadget->quirk_ep_out_aligned_size ?
> + usb_ep_align_maxpacketsize(ep->ep, len) : len;
> +
> + data = kmalloc(data_len, GFP_KERNEL);
> if (unlikely(!data))
> return -ENOMEM;
>
> @@ -824,7 +832,7 @@ static ssize_t ffs_epfile_io(struct file *file,
> req->context = &done;
> req->complete = ffs_epfile_io_complete;
> req->buf = data;
> - req->length = len;
> + req->length = data_len;
IIUC, req->length should still be set to len, not to data_len.
>
> ret = usb_ep_queue(ep->ep, req, GFP_ATOMIC);
>
> @@ -836,9 +844,16 @@ static ssize_t ffs_epfile_io(struct file *file,
> ret = -EINTR;
> usb_ep_dequeue(ep->ep, req);
> } else {
> + /*
> + * XXX We may end up silently droping data here.
> + * Since data_len (i.e. req->length) may be bigger
> + * than len (after being rounded up to maxpacketsize),
> + * we may end up with more data then user space has
> + * space for.
> + */
Then this will never come up. If the host sends a packet that's too
long, you'll get a -EOVERFLOW error.
> ret = ep->status;
> if (read && ret > 0 &&
> - unlikely(copy_to_user(buf, data, ret)))
> + unlikely(copy_to_user(buf, data, min(ret, len))))
> ret = -EFAULT;
> }
> }
The reason for the quirk is that the controller may write all the
incoming data to the buffer, even if this is more data than the driver
requested.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
