On Wed, 26 Oct 2005, Ronen Shitrit wrote:
> Hi
>
> I'm running an application which uses the USB as disk on key, and I
>
> This application allocate a memory space which isn't initialized, And
> write it to the scsi device (USB diskonkey), Since the buffer isn't
> initialize and the App uses a direct mode, which means the buffer isn't
> Copy from user space to kernel space, when the kernel looks for the
> physical address of the Buffer it get page exception.
> The page exception detects that this buffer isn't going to be used for
> write and its not initialized, Therefore it uses the kernel zero page,
> which is a reserved page filled with zeros. (used in order to Accelerate
> page mapping).
> This page is located at physical address 0. (on the ARM arch)
> When this page is passed to the USB stack, the USB host mistranslate the
> buffer with a read request.
>
> Fix:
> The USB stack should check for zero length, and not for zero buffer.
> (see patch attached)
>
> Please comment.
Although I'm not an expert on this code, your fix still looks wrong to me.
Here's the current code:
/*
* data transfer stage: buffer setup
*/
if (likely (len > 0))
buf = urb->transfer_dma;
else
buf = 0;
/* for zero length DATA stages, STATUS is always IN */
if (!buf || is_input)
token |= (1 /* "in" */ << 8);
/* else it's already initted to "out" pid (0 << 8) */
The comment "for zero length DATA stages, STATUS is always IN" is true
only for control transfers. You changed "!buf" to "!len", but it seems to
me the whole zero-length test should be moved earlier, to the section that
takes care of control transfers. You also missed another test for (buf !=
0) later on.
I suggest something more like the patch below.
Alan Stern
Signed-off-by: Alan Stern <[EMAIL PROTECTED]>
---
Index: usb-2.6/drivers/usb/host/ehci-q.c
===================================================================
--- usb-2.6.orig/drivers/usb/host/ehci-q.c
+++ usb-2.6/drivers/usb/host/ehci-q.c
@@ -514,18 +514,18 @@ qh_urb_transaction (
qtd->urb = urb;
qtd_prev->hw_next = QTD_NEXT (qtd->qtd_dma);
list_add_tail (&qtd->qtd_list, head);
+
+ /* for zero length DATA stages, STATUS is always IN */
+ if (len == 0)
+ token |= (1 /* "in" */ << 8);
}
/*
* data transfer stage: buffer setup
*/
- if (likely (len > 0))
- buf = urb->transfer_dma;
- else
- buf = 0;
+ buf = urb->transfer_dma;
- /* for zero length DATA stages, STATUS is always IN */
- if (!buf || is_input)
+ if (is_input)
token |= (1 /* "in" */ << 8);
/* else it's already initted to "out" pid (0 << 8) */
@@ -572,7 +572,7 @@ qh_urb_transaction (
* control requests may need a terminating data "status" ack;
* bulk ones may need a terminating short packet (zero length).
*/
- if (likely (buf != 0)) {
+ if (likely (len != 0)) {
int one_more = 0;
if (usb_pipecontrol (urb->pipe)) {
-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc.
Get Certified Today * Register for a JBoss Training Course
Free Certification Exam for All Training Attendees Through End of 2005
Visit http://www.jboss.com/services/certification for more information
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
