| 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
| ||
|
Message-ID: <40351ed6-2907-3966-e69a-a564173b3682@infradead.org>
Date: Mon, 15 Mar 2021 11:25:03 -0700
From: Randy Dunlap <rdunlap@...radead.org>
To: "Hongren Zheng (Zenithal)" <i@...ithal.me>,
Valentina Manea <valentina.manea.m@...il.com>,
Shuah Khan <shuah@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Márton Németh <nm127@...email.hu>,
Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org
Cc: Alexandre Demers <alexandre.f.demers@...il.com>,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
usbip-devel@...ts.sourceforge.net
Subject: Re: [PATCH v3] docs: usbip: Fix major fields and descriptions in
protocol
Hi,
Some comments inline.
On 3/15/21 1:40 AM, Hongren Zheng (Zenithal) wrote:
> The old document for usbip protocol is misleading and hard to read:
> * Some fields in header are incorrect
> * Explanation of some fields are unclear or even wrong
> * Padding of header (namely all headers have the same length) is
> not explicitly point out, which is crucial for stream protocol like
> TCP
>
> These fixes are made through reading usbip kernel drivers and userland
> codes. Also I have implemented one usbip server.
>
> Major changes:
>
> PATCH v2:
>
> PATCH v3:
>
> Co-developed-by: Alexandre Demers <alexandre.f.demers@...il.com>
> Signed-off-by: Hongren Zheng (Zenithal) <i@...ithal.me>
> ---
> Documentation/usb/usbip_protocol.rst | 288 ++++++++++++++-------------
> 1 file changed, 155 insertions(+), 133 deletions(-)
>
> diff --git a/Documentation/usb/usbip_protocol.rst b/Documentation/usb/usbip_protocol.rst
> index 988c832166cd..ed423cdda236 100644
> --- a/Documentation/usb/usbip_protocol.rst
> +++ b/Documentation/usb/usbip_protocol.rst
> @@ -254,65 +283,75 @@ OP_REP_IMPORT:
> | 0x13E | 1 | | bNumInterfaces |
> +-----------+--------+------------+---------------------------------------------------+
>
> -USBIP_CMD_SUBMIT:
> - Submit an URB
> +The following four commands have a common basic header called
> +'usbip_header_basic', and their headers, called 'usbip_header' (before URB
> +payload), have the same length, therefore paddings are needed.
>
> -+-----------+--------+------------+---------------------------------------------------+
> -| Offset | Length | Value | Description |
> -+===========+========+============+===================================================+
> -| 0 | 4 | 0x00000001 | command: Submit an URB |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 4 | 4 | | seqnum: the sequence number of the URB to submit |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 8 | 4 | | devid |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0xC | 4 | | direction: |
> -| | | | |
> -| | | | - 0: USBIP_DIR_OUT |
> -| | | | - 1: USBIP_DIR_IN |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x10 | 4 | | ep: endpoint number, possible values are: 0...15 |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x14 | 4 | | transfer_flags: possible values depend on the |
> -| | | | URB transfer type, see below |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x18 | 4 | | transfer_buffer_length |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x1C | 4 | | start_frame: specify the selected frame to |
> -| | | | transmit an ISO frame, ignored if URB_ISO_ASAP |
> -| | | | is specified at transfer_flags |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x20 | 4 | | number_of_packets: number of ISO packets |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x24 | 4 | | interval: maximum time for the request on the |
> -| | | | server-side host controller |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x28 | 8 | | setup: data bytes for USB setup, filled with |
> -| | | | zeros if not used |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x30 | | | URB data. For ISO transfers the padding between |
> -| | | | each ISO packets is not transmitted. |
> -+-----------+--------+------------+---------------------------------------------------+
> +usbip_header_basic:
>
> ++-----------+--------+---------------------------------------------------+
> +| Offset | Length | Description |
> ++===========+========+===================================================+
> +| 0 | 4 | command |
> ++-----------+--------+---------------------------------------------------+
> +| 4 | 4 | seqnum: sequential number that identifies requests|
> +| | | and corresponding responses; |
> +| | | incremented per connection |
> ++-----------+--------+---------------------------------------------------+
> +| 8 | 4 | devid: specifies a remote USB device uniquely |
> +| | | instead of busnum and devnum; |
> +| | | for client (request), this value is |
> +| | | ((busnum << 16) | devnum); |
> +| | | for server (response), this shall be set to 0 |
> ++-----------+--------+---------------------------------------------------+
> +| 0xC | 4 | direction: |
> +| | | |
> +| | | - 0: USBIP_DIR_OUT |
> +| | | - 1: USBIP_DIR_IN |
> +| | | |
> +| | | only used by client, for server this shall be 0 |
> ++-----------+--------+---------------------------------------------------+
> +| 0x10 | 4 | ep: endpoint number |
> +| | | only used by client, for server this shall be 0 |
This could use a ';' after the '0' above for better readability.
> +| | | for UNLINK, this shall be 0 |
> ++-----------+--------+---------------------------------------------------+
>
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | Allowed transfer_flags | value | control | interrupt | bulk | isochronous |
> - +=========================+============+=========+===========+==========+=============+
> - | URB_SHORT_NOT_OK | 0x00000001 | only in | only in | only in | no |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_ISO_ASAP | 0x00000002 | no | no | no | yes |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_NO_TRANSFER_DMA_MAP | 0x00000004 | yes | yes | yes | yes |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_ZERO_PACKET | 0x00000040 | no | no | only out | no |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_NO_INTERRUPT | 0x00000080 | yes | yes | yes | yes |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_FREE_BUFFER | 0x00000100 | yes | yes | yes | yes |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_DIR_MASK | 0x00000200 | yes | yes | yes | yes |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> +USBIP_CMD_SUBMIT:
> + Submit an URB
>
> ++-----------+--------+---------------------------------------------------+
> +| Offset | Length | Description |
> ++===========+========+===================================================+
> +| 0 | 20 | usbip_header_basic, 'command' shall be 0x00000001 |
> ++-----------+--------+---------------------------------------------------+
> +| 0x14 | 4 | transfer_flags: possible values depend on the |
> +| | | URB transfer_flags, |
> +| | | but with URB_NO_TRANSFER_DMA_MAP masked |
> ++-----------+--------+---------------------------------------------------+
> +| 0x18 | 4 | transfer_buffer_length: |
> +| | | use URB transfer_buffer_length |
> ++-----------+--------+---------------------------------------------------+
> +| 0x1C | 4 | start_frame: use URB start_frame; |
> +| | | initial frame for ISO transfer |
transfer;
> +| | | shall be set to 0 if not ISO transfer |
> ++-----------+--------+---------------------------------------------------+
> +| 0x20 | 4 | number_of_packets: number of ISO packets |
packets;
> +| | | shall be set to 0xffffffff if not ISO transfer |
> ++-----------+--------+---------------------------------------------------+
> +| 0x24 | 4 | interval: maximum time for the request on the |
> +| | | server-side host controller |
> ++-----------+--------+---------------------------------------------------+
> +| 0x28 | 8 | setup: data bytes for USB setup, filled with |
> +| | | zeros if not used |
> ++-----------+--------+---------------------------------------------------+
> +| 0x30 | n | tranfer_buffer. |
> +| | | If direction is USBIP_DIR_OUT then n equals |
> +| | | transfer_buffer_length; otherwise n equals 0 |
equals 0.
> +| | | For ISO transfers the padding between each ISO |
> +| | | between each ISO packets is not transmitted. |
> ++-----------+--------+---------------------------------------------------+
> +| 0x30+n | m | iso_packet_descriptor |
> ++-----------+--------+---------------------------------------------------+
>
> USBIP_RET_SUBMIT:
> Reply for submitting an URB
> @@ -320,92 +359,75 @@ USBIP_RET_SUBMIT:
> +-----------+--------+------------+---------------------------------------------------+
> | Offset | Length | Value | Description |
> +===========+========+============+===================================================+
> -| 0 | 4 | 0x00000003 | command |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 4 | 4 | | seqnum: URB sequence number |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 8 | 4 | | devid |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0xC | 4 | | direction: |
> -| | | | |
> -| | | | - 0: USBIP_DIR_OUT |
> -| | | | - 1: USBIP_DIR_IN |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x10 | 4 | | ep: endpoint number |
> +| 0 | 20 | | usbip_header_basic, 'command' shall be 0x00000003 |
> +-----------+--------+------------+---------------------------------------------------+
> | 0x14 | 4 | | status: zero for successful URB transaction, |
> | | | | otherwise some kind of error happened. |
> +-----------+--------+------------+---------------------------------------------------+
> | 0x18 | 4 | n | actual_length: number of URB data bytes |
bytes;
> +| | | | use URB actual_length |
> +-----------+--------+------------+---------------------------------------------------+
> -| 0x1C | 4 | | start_frame: for an ISO frame the actually |
> -| | | | selected frame for transmit. |
> +| 0x1C | 4 | | start_frame: use URB start_frame; |
> +| | | | initial frame for ISO transfer |
transfer;
> +| | | | shall be set to 0 if not ISO transfer |
> +-----------+--------+------------+---------------------------------------------------+
> -| 0x20 | 4 | | number_of_packets |
> +| 0x20 | 4 | | number_of_packets: number of ISO packets |
packets;
> +| | | | shall be set to 0xffffffff if not ISO transfer |
> +-----------+--------+------------+---------------------------------------------------+
> | 0x24 | 4 | | error_count |
> +-----------+--------+------------+---------------------------------------------------+
> -| 0x28 | 8 | | setup: data bytes for USB setup, filled with |
> -| | | | zeros if not used |
used;
> +| 0x28 | 8 | | padding, shall be set to 0 |
> +-----------+--------+------------+---------------------------------------------------+
> -| 0x30 | n | | URB data bytes. For ISO transfers the padding |
> +| 0x30 | n | | tranfer_buffer. |
transfer_buffer.
> +| | | | If direction is USBIP_DIR_OUT then n equals |
> +| | | | transfer_buffer_length; otherwise n equals 0 |
equals 0.
> +| | | | For ISO transfers the padding between each ISO |
> | | | | between each ISO packets is not transmitted. |
> +-----------+--------+------------+---------------------------------------------------+
> +| 0x30+n | m | | iso_packet_descriptor |
> ++-----------+--------+------------+---------------------------------------------------+
>
> USBIP_CMD_UNLINK:
> Unlink an URB
>
> -+-----------+--------+------------+---------------------------------------------------+
> -| Offset | Length | Value | Description |
> -+===========+========+============+===================================================+
> -| 0 | 4 | 0x00000002 | command: URB unlink command |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 4 | 4 | | seqnum: URB sequence number to unlink: |
> -| | | | |
> -| | | | FIXME: |
> -| | | | is this so? |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 8 | 4 | | devid |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0xC | 4 | | direction: |
> -| | | | |
> -| | | | - 0: USBIP_DIR_OUT |
> -| | | | - 1: USBIP_DIR_IN |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x10 | 4 | | ep: endpoint number: zero |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x14 | 4 | | seqnum: the URB sequence number given previously |
> -| | | | at USBIP_CMD_SUBMIT.seqnum field |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x30 | n | | URB data bytes. For ISO transfers the padding |
> -| | | | between each ISO packets is not transmitted. |
> -+-----------+--------+------------+---------------------------------------------------+
> ++-----------+--------+---------------------------------------------------+
> +| Offset | Length | Description |
> ++===========+========+===================================================+
> +| 0 | 20 | usbip_header_basic, 'command' shall be 0x00000002 |
> ++-----------+--------+---------------------------------------------------+
> +| 0x14 | 4 | unlink_seqnum, of the SUBMIT request to unlink |
> ++-----------+--------+---------------------------------------------------+
> +| 0x18 | 24 | padding, shall be set to 0 |
> ++-----------+--------+---------------------------------------------------+
>
> USBIP_RET_UNLINK:
> Reply for URB unlink
>
> -+-----------+--------+------------+---------------------------------------------------+
> -| Offset | Length | Value | Description |
> -+===========+========+============+===================================================+
> -| 0 | 4 | 0x00000004 | command: reply for the URB unlink command |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 4 | 4 | | seqnum: the unlinked URB sequence number |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 8 | 4 | | devid |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0xC | 4 | | direction: |
> -| | | | |
> -| | | | - 0: USBIP_DIR_OUT |
> -| | | | - 1: USBIP_DIR_IN |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x10 | 4 | | ep: endpoint number |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x14 | 4 | | status: This is the value contained in the |
> -| | | | urb->status in the URB completition handler. |
completion
> -| | | | |
> -| | | | FIXME: |
> -| | | | a better explanation needed. |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x30 | n | | URB data bytes. For ISO transfers the padding |
> -| | | | between each ISO packets is not transmitted. |
> -+-----------+--------+------------+---------------------------------------------------+
> ++-----------+--------+---------------------------------------------------+
> +| Offset | Length | Description |
> ++===========+========+===================================================+
> +| 0 | 20 | usbip_header_basic, 'command' shall be 0x00000004 |
> ++-----------+--------+---------------------------------------------------+
> +| 0x14 | 4 | status: This is similar to that status of |
to the status of
> +| | | USBIP_RET_SUBMIT (share the same memory offset) |
offset).
> +| | | When UNLINK is successful, status is -ECONNRESET; |
> +| | | when USBIP_CMD_UNLINK is after USBIP_RET_SUBMIT |
> +| | | status is 0 |> ++-----------+--------+---------------------------------------------------+
> +| 0x18 | 24 | padding, shall be set to 0 |
> ++-----------+--------+---------------------------------------------------+
> +
> +EXAMPLE
> +=======
HTH.
--
~Randy
Powered by blists - more mailing lists