[Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Daniel P. Berrangé]

Two previous attempts to fix this due to GCC 9 highlighting
unaligned data access. My attempt:

  https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html

And a previous one:

  https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
  https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html

There are a number of bugs in the USB MTP usb_mtp_write_metadata
method handling the filename character set conversion.

The 2nd patch in this series is a security flaw fix since the
code was not correctly validating guest provided data length.

I've been unable to figure out how to exercise the codepath that
calls usb_mtp_write_metadata. At a guess, it looks like something
that should be called when writing to a file from a guest, but the
GNOME GVFS MTP driver doesn't provide write support. Using the
command line MTP tools "mtp-sendfile" command results in an
protocol error

    # mtp-sendfile foo eek.txt
    libmtp version: 1.1.14

    Device 0 (VID=46f4 and PID=0004) is UNKNOWN in libmtp v1.1.14.
    Please report this VID/PID and the device model to the libmtp development team
    PTP_ERROR_IO: failed to open session, trying again after resetting USB interface
    LIBMTP libusb: Attempt to reset device
    Sending foo to eek.txt
    type: , 44
    Sending file...

    Error sending file.
    Error 2: PTP Layer error 02ff: send_file_object_info(): Could not send object info.
    Error 2: Error 02ff: PTP I/O Error
    ERROR: Could not close session!

And QEMU tracing show unexpected requests

    26582@1555340076151600935 usb_mtp_command dev 4, code 0x9803, trans 0x18, args 0x11, 0xdc04, 0x0, 0x0, 0x0
    26582@1555340076151619955 usb_mtp_xfer dev 4, ep 2, 20/20
    26582@1555340076154138556 usb_mtp_data_in dev 4, trans 0x18, len 8
    26582@1555340076154150689 usb_mtp_xfer dev 4, ep 1, 20/512
    26582@1555340076156654311 usb_mtp_success dev 4, trans 0x18, args 0x0, 0x0
    26582@1555340076156667764 usb_mtp_xfer dev 4, ep 1, 12/512
    26582@1555340076159215930 usb_mtp_command dev 4, code 0x100c, trans 0x19, args 0x10001, 0xc, 0x0, 0x0, 0x0
    26582@1555340076159229610 usb_mtp_xfer dev 4, ep 2, 20/20
    26582@1555340076164166196 usb_mtp_stall dev 4, reason: awaiting data-out
    26582@1555340076167156367 usb_mtp_stall dev 4, reason: transaction inflight
    26582@1555340076170108336 usb_mtp_stall dev 4, reason: unknown control request
    26582@1555340076172606798 usb_mtp_stall dev 4, reason: unknown control request

Perhaps a Windows guest can exercise this, but I don't have a modern
Windows install with MTP support.

Thus this series is merely compile tested.

Daniel P. Berrangé (3):
  usb-mtp: fix string length for filename when writing metadata
  usb-mtp: fix bounds check for guest provided filename
  usb-mtp: fix alignment of access of ObjectInfo filename field

 hw/usb/dev-mtp.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Daniel P. Berrangé]

Two previous attempts to fix this due to GCC 9 highlighting
unaligned data access. My attempt:

  https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html

And a previous one:

  https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
  https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html

There are a number of bugs in the USB MTP usb_mtp_write_metadata
method handling the filename character set conversion.

The 2nd patch in this series is a security flaw fix since the
code was not correctly validating guest provided data length.

I've been unable to figure out how to exercise the codepath that
calls usb_mtp_write_metadata. At a guess, it looks like something
that should be called when writing to a file from a guest, but the
GNOME GVFS MTP driver doesn't provide write support. Using the
command line MTP tools "mtp-sendfile" command results in an
protocol error

    # mtp-sendfile foo eek.txt
    libmtp version: 1.1.14

    Device 0 (VID=46f4 and PID=0004) is UNKNOWN in libmtp v1.1.14.
    Please report this VID/PID and the device model to the libmtp development team
    PTP_ERROR_IO: failed to open session, trying again after resetting USB interface
    LIBMTP libusb: Attempt to reset device
    Sending foo to eek.txt
    type: , 44
    Sending file...

    Error sending file.
    Error 2: PTP Layer error 02ff: send_file_object_info(): Could not send object info.
    Error 2: Error 02ff: PTP I/O Error
    ERROR: Could not close session!

And QEMU tracing show unexpected requests

    26582@1555340076151600935 usb_mtp_command dev 4, code 0x9803, trans 0x18, args 0x11, 0xdc04, 0x0, 0x0, 0x0
    26582@1555340076151619955 usb_mtp_xfer dev 4, ep 2, 20/20
    26582@1555340076154138556 usb_mtp_data_in dev 4, trans 0x18, len 8
    26582@1555340076154150689 usb_mtp_xfer dev 4, ep 1, 20/512
    26582@1555340076156654311 usb_mtp_success dev 4, trans 0x18, args 0x0, 0x0
    26582@1555340076156667764 usb_mtp_xfer dev 4, ep 1, 12/512
    26582@1555340076159215930 usb_mtp_command dev 4, code 0x100c, trans 0x19, args 0x10001, 0xc, 0x0, 0x0, 0x0
    26582@1555340076159229610 usb_mtp_xfer dev 4, ep 2, 20/20
    26582@1555340076164166196 usb_mtp_stall dev 4, reason: awaiting data-out
    26582@1555340076167156367 usb_mtp_stall dev 4, reason: transaction inflight
    26582@1555340076170108336 usb_mtp_stall dev 4, reason: unknown control request
    26582@1555340076172606798 usb_mtp_stall dev 4, reason: unknown control request

Perhaps a Windows guest can exercise this, but I don't have a modern
Windows install with MTP support.

Thus this series is merely compile tested.

Daniel P. Berrangé (3):
  usb-mtp: fix string length for filename when writing metadata
  usb-mtp: fix bounds check for guest provided filename
  usb-mtp: fix alignment of access of ObjectInfo filename field

 hw/usb/dev-mtp.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH 1/3] usb-mtp: fix string length for filename when writing metadata

[Daniel P. Berrangé]

The ObjectInfo 'length' field provides the length of the
wide character string filename. This is then converted to
a multi-byte character string. This may have a different
byte count to the wide character string. We should use the
C string length of the multi-byte string instead.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hw/usb/dev-mtp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index ebf210fbf8..838cd74da6 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1714,7 +1714,7 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
         return;
     }
 
-    o = usb_mtp_object_lookup_name(p, filename, dataset->length);
+    o = usb_mtp_object_lookup_name(p, filename, -1);
     if (o != NULL) {
         next_handle = o->handle;
     }
-- 
2.20.1

[Qemu-devel] [PATCH 1/3] usb-mtp: fix string length for filename when writing metadata

[Daniel P. Berrangé]

The ObjectInfo 'length' field provides the length of the
wide character string filename. This is then converted to
a multi-byte character string. This may have a different
byte count to the wide character string. We should use the
C string length of the multi-byte string instead.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hw/usb/dev-mtp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index ebf210fbf8..838cd74da6 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1714,7 +1714,7 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
         return;
     }
 
-    o = usb_mtp_object_lookup_name(p, filename, dataset->length);
+    o = usb_mtp_object_lookup_name(p, filename, -1);
     if (o != NULL) {
         next_handle = o->handle;
     }
-- 
2.20.1

[Qemu-devel] [PATCH 2/3] usb-mtp: fix bounds check for guest provided filename

[Daniel P. Berrangé]

The ObjectInfo struct has a variable length array containing the UTF-16
encoded filename. The number of characters of trailing data is given by
the 'length' field in the struct and this must be validated against the
size of the data packet received from the guest.

Since the data is UTF-16, we must convert the byte count we have to a
character count before validating. This must take care to truncate if
a malicious guest sent an odd number of bytes.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hw/usb/dev-mtp.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 838cd74da6..6b7d1296e4 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1699,12 +1699,19 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
     MTPObject *o;
     MTPObject *p = usb_mtp_object_lookup(s, s->dataset.parent_handle);
     uint32_t next_handle = s->next_handle;
+    size_t filename_chars = dlen - offsetof(ObjectInfo, filename);
+
+    /*
+     * filename is utf-16. We're intentionally doing
+     * integer division to truncate if malicious guest
+     * sent an odd number of bytes.
+     */
+    filename_chars /= 2;
 
     assert(!s->write_pending);
     assert(p != NULL);
 
-    filename = utf16_to_str(MIN(dataset->length,
-                                dlen - offsetof(ObjectInfo, filename)),
+    filename = utf16_to_str(MIN(dataset->length, filename_chars),
                             dataset->filename);
 
     if (strchr(filename, '/')) {
-- 
2.20.1

[Qemu-devel] [PATCH 2/3] usb-mtp: fix bounds check for guest provided filename

[Daniel P. Berrangé]

The ObjectInfo struct has a variable length array containing the UTF-16
encoded filename. The number of characters of trailing data is given by
the 'length' field in the struct and this must be validated against the
size of the data packet received from the guest.

Since the data is UTF-16, we must convert the byte count we have to a
character count before validating. This must take care to truncate if
a malicious guest sent an odd number of bytes.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hw/usb/dev-mtp.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 838cd74da6..6b7d1296e4 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1699,12 +1699,19 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
     MTPObject *o;
     MTPObject *p = usb_mtp_object_lookup(s, s->dataset.parent_handle);
     uint32_t next_handle = s->next_handle;
+    size_t filename_chars = dlen - offsetof(ObjectInfo, filename);
+
+    /*
+     * filename is utf-16. We're intentionally doing
+     * integer division to truncate if malicious guest
+     * sent an odd number of bytes.
+     */
+    filename_chars /= 2;
 
     assert(!s->write_pending);
     assert(p != NULL);
 
-    filename = utf16_to_str(MIN(dataset->length,
-                                dlen - offsetof(ObjectInfo, filename)),
+    filename = utf16_to_str(MIN(dataset->length, filename_chars),
                             dataset->filename);
 
     if (strchr(filename, '/')) {
-- 
2.20.1

[Qemu-devel] [PATCH 3/3] usb-mtp: fix alignment of access of ObjectInfo filename field

[Daniel P. Berrangé]

The ObjectInfo struct's "filename" field is following a uint8_t
field in a packed struct and thus has bad alignment for a 16-bit
field. Switch the field to to uint8_t and use the helper function
for accessing unaligned 16-bit data.

Note that although the MTP spec specifies big endian, when transported
over the USB protocol, data is little endian.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hw/usb/dev-mtp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 6b7d1296e4..963449ec7d 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -226,7 +226,7 @@ typedef struct {
     uint32_t assoc_desc;
     uint32_t seq_no; /*unused*/
     uint8_t length; /*part of filename field*/
-    uint16_t filename[0];
+    uint8_t filename[0]; /* UTF-16 encoded */
     char date_created[0]; /*unused*/
     char date_modified[0]; /*unused*/
     char keywords[0]; /*unused*/
@@ -1551,7 +1551,7 @@ static void usb_mtp_cancel_packet(USBDevice *dev, USBPacket *p)
     fprintf(stderr, "%s\n", __func__);
 }
 
-static char *utf16_to_str(uint8_t len, uint16_t *arr)
+static char *utf16_to_str(uint8_t len, uint8_t *str16)
 {
     wchar_t *wstr = g_new0(wchar_t, len + 1);
     int count, dlen;
@@ -1559,7 +1559,7 @@ static char *utf16_to_str(uint8_t len, uint16_t *arr)
 
     for (count = 0; count < len; count++) {
         /* FIXME: not working for surrogate pairs */
-        wstr[count] = (wchar_t)arr[count];
+        wstr[count] = lduw_le_p(str16 + (count * 2));
     }
     wstr[count] = 0;
 
-- 
2.20.1

[Qemu-devel] [PATCH 3/3] usb-mtp: fix alignment of access of ObjectInfo filename field

[Daniel P. Berrangé]

The ObjectInfo struct's "filename" field is following a uint8_t
field in a packed struct and thus has bad alignment for a 16-bit
field. Switch the field to to uint8_t and use the helper function
for accessing unaligned 16-bit data.

Note that although the MTP spec specifies big endian, when transported
over the USB protocol, data is little endian.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hw/usb/dev-mtp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 6b7d1296e4..963449ec7d 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -226,7 +226,7 @@ typedef struct {
     uint32_t assoc_desc;
     uint32_t seq_no; /*unused*/
     uint8_t length; /*part of filename field*/
-    uint16_t filename[0];
+    uint8_t filename[0]; /* UTF-16 encoded */
     char date_created[0]; /*unused*/
     char date_modified[0]; /*unused*/
     char keywords[0]; /*unused*/
@@ -1551,7 +1551,7 @@ static void usb_mtp_cancel_packet(USBDevice *dev, USBPacket *p)
     fprintf(stderr, "%s\n", __func__);
 }
 
-static char *utf16_to_str(uint8_t len, uint16_t *arr)
+static char *utf16_to_str(uint8_t len, uint8_t *str16)
 {
     wchar_t *wstr = g_new0(wchar_t, len + 1);
     int count, dlen;
@@ -1559,7 +1559,7 @@ static char *utf16_to_str(uint8_t len, uint16_t *arr)
 
     for (count = 0; count < len; count++) {
         /* FIXME: not working for surrogate pairs */
-        wstr[count] = (wchar_t)arr[count];
+        wstr[count] = lduw_le_p(str16 + (count * 2));
     }
     wstr[count] = 0;
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Bandan Das]

Daniel P. Berrangé <berrange@redhat.com> writes:

> Two previous attempts to fix this due to GCC 9 highlighting
> unaligned data access. My attempt:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
>
> And a previous one:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html
>
> There are a number of bugs in the USB MTP usb_mtp_write_metadata
> method handling the filename character set conversion.
>
> The 2nd patch in this series is a security flaw fix since the
> code was not correctly validating guest provided data length.
>
> I've been unable to figure out how to exercise the codepath that
> calls usb_mtp_write_metadata. At a guess, it looks like something
> that should be called when writing to a file from a guest, but the
> GNOME GVFS MTP driver doesn't provide write support. Using the
> command line MTP tools "mtp-sendfile" command results in an
> protocol error
>
>     # mtp-sendfile foo eek.txt
>     libmtp version: 1.1.14
>

The store is read only by default. Are you trying something like:
 -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?

BTW, I also found a bug introduced by a recent commit which will
return an incomplete transfer for smaller file sizes.


>     Device 0 (VID=46f4 and PID=0004) is UNKNOWN in libmtp v1.1.14.
>     Please report this VID/PID and the device model to the libmtp development team
>     PTP_ERROR_IO: failed to open session, trying again after resetting USB interface
>     LIBMTP libusb: Attempt to reset device
>     Sending foo to eek.txt
>     type: , 44
>     Sending file...
>
>     Error sending file.
>     Error 2: PTP Layer error 02ff: send_file_object_info(): Could not send object info.
>     Error 2: Error 02ff: PTP I/O Error
>     ERROR: Could not close session!
>
> And QEMU tracing show unexpected requests
>
>     26582@1555340076151600935 usb_mtp_command dev 4, code 0x9803, trans 0x18, args 0x11, 0xdc04, 0x0, 0x0, 0x0
>     26582@1555340076151619955 usb_mtp_xfer dev 4, ep 2, 20/20
>     26582@1555340076154138556 usb_mtp_data_in dev 4, trans 0x18, len 8
>     26582@1555340076154150689 usb_mtp_xfer dev 4, ep 1, 20/512
>     26582@1555340076156654311 usb_mtp_success dev 4, trans 0x18, args 0x0, 0x0
>     26582@1555340076156667764 usb_mtp_xfer dev 4, ep 1, 12/512
>     26582@1555340076159215930 usb_mtp_command dev 4, code 0x100c, trans 0x19, args 0x10001, 0xc, 0x0, 0x0, 0x0
>     26582@1555340076159229610 usb_mtp_xfer dev 4, ep 2, 20/20
>     26582@1555340076164166196 usb_mtp_stall dev 4, reason: awaiting data-out
>     26582@1555340076167156367 usb_mtp_stall dev 4, reason: transaction inflight
>     26582@1555340076170108336 usb_mtp_stall dev 4, reason: unknown control request
>     26582@1555340076172606798 usb_mtp_stall dev 4, reason: unknown control request
>
> Perhaps a Windows guest can exercise this, but I don't have a modern
> Windows install with MTP support.
>
> Thus this series is merely compile tested.
>
> Daniel P. Berrangé (3):
>   usb-mtp: fix string length for filename when writing metadata
>   usb-mtp: fix bounds check for guest provided filename
>   usb-mtp: fix alignment of access of ObjectInfo filename field
>
>  hw/usb/dev-mtp.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Bandan Das]

Daniel P. Berrangé <berrange@redhat.com> writes:

> Two previous attempts to fix this due to GCC 9 highlighting
> unaligned data access. My attempt:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
>
> And a previous one:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html
>
> There are a number of bugs in the USB MTP usb_mtp_write_metadata
> method handling the filename character set conversion.
>
> The 2nd patch in this series is a security flaw fix since the
> code was not correctly validating guest provided data length.
>
> I've been unable to figure out how to exercise the codepath that
> calls usb_mtp_write_metadata. At a guess, it looks like something
> that should be called when writing to a file from a guest, but the
> GNOME GVFS MTP driver doesn't provide write support. Using the
> command line MTP tools "mtp-sendfile" command results in an
> protocol error
>
>     # mtp-sendfile foo eek.txt
>     libmtp version: 1.1.14
>

The store is read only by default. Are you trying something like:
 -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?

BTW, I also found a bug introduced by a recent commit which will
return an incomplete transfer for smaller file sizes.


>     Device 0 (VID=46f4 and PID=0004) is UNKNOWN in libmtp v1.1.14.
>     Please report this VID/PID and the device model to the libmtp development team
>     PTP_ERROR_IO: failed to open session, trying again after resetting USB interface
>     LIBMTP libusb: Attempt to reset device
>     Sending foo to eek.txt
>     type: , 44
>     Sending file...
>
>     Error sending file.
>     Error 2: PTP Layer error 02ff: send_file_object_info(): Could not send object info.
>     Error 2: Error 02ff: PTP I/O Error
>     ERROR: Could not close session!
>
> And QEMU tracing show unexpected requests
>
>     26582@1555340076151600935 usb_mtp_command dev 4, code 0x9803, trans 0x18, args 0x11, 0xdc04, 0x0, 0x0, 0x0
>     26582@1555340076151619955 usb_mtp_xfer dev 4, ep 2, 20/20
>     26582@1555340076154138556 usb_mtp_data_in dev 4, trans 0x18, len 8
>     26582@1555340076154150689 usb_mtp_xfer dev 4, ep 1, 20/512
>     26582@1555340076156654311 usb_mtp_success dev 4, trans 0x18, args 0x0, 0x0
>     26582@1555340076156667764 usb_mtp_xfer dev 4, ep 1, 12/512
>     26582@1555340076159215930 usb_mtp_command dev 4, code 0x100c, trans 0x19, args 0x10001, 0xc, 0x0, 0x0, 0x0
>     26582@1555340076159229610 usb_mtp_xfer dev 4, ep 2, 20/20
>     26582@1555340076164166196 usb_mtp_stall dev 4, reason: awaiting data-out
>     26582@1555340076167156367 usb_mtp_stall dev 4, reason: transaction inflight
>     26582@1555340076170108336 usb_mtp_stall dev 4, reason: unknown control request
>     26582@1555340076172606798 usb_mtp_stall dev 4, reason: unknown control request
>
> Perhaps a Windows guest can exercise this, but I don't have a modern
> Windows install with MTP support.
>
> Thus this series is merely compile tested.
>
> Daniel P. Berrangé (3):
>   usb-mtp: fix string length for filename when writing metadata
>   usb-mtp: fix bounds check for guest provided filename
>   usb-mtp: fix alignment of access of ObjectInfo filename field
>
>  hw/usb/dev-mtp.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Daniel P. Berrangé]

On Mon, Apr 15, 2019 at 12:52:41PM -0400, Bandan Das wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > Two previous attempts to fix this due to GCC 9 highlighting
> > unaligned data access. My attempt:
> >
> >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
> >
> > And a previous one:
> >
> >   https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
> >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html
> >
> > There are a number of bugs in the USB MTP usb_mtp_write_metadata
> > method handling the filename character set conversion.
> >
> > The 2nd patch in this series is a security flaw fix since the
> > code was not correctly validating guest provided data length.
> >
> > I've been unable to figure out how to exercise the codepath that
> > calls usb_mtp_write_metadata. At a guess, it looks like something
> > that should be called when writing to a file from a guest, but the
> > GNOME GVFS MTP driver doesn't provide write support. Using the
> > command line MTP tools "mtp-sendfile" command results in an
> > protocol error
> >
> >     # mtp-sendfile foo eek.txt
> >     libmtp version: 1.1.14
> >
> 
> The store is read only by default. Are you trying something like:
>  -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?

Ah ha, I didn't realize I had to enable write support explicitly. Will
retry with that.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Daniel P. Berrangé]

On Mon, Apr 15, 2019 at 12:52:41PM -0400, Bandan Das wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > Two previous attempts to fix this due to GCC 9 highlighting
> > unaligned data access. My attempt:
> >
> >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
> >
> > And a previous one:
> >
> >   https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
> >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html
> >
> > There are a number of bugs in the USB MTP usb_mtp_write_metadata
> > method handling the filename character set conversion.
> >
> > The 2nd patch in this series is a security flaw fix since the
> > code was not correctly validating guest provided data length.
> >
> > I've been unable to figure out how to exercise the codepath that
> > calls usb_mtp_write_metadata. At a guess, it looks like something
> > that should be called when writing to a file from a guest, but the
> > GNOME GVFS MTP driver doesn't provide write support. Using the
> > command line MTP tools "mtp-sendfile" command results in an
> > protocol error
> >
> >     # mtp-sendfile foo eek.txt
> >     libmtp version: 1.1.14
> >
> 
> The store is read only by default. Are you trying something like:
>  -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?

Ah ha, I didn't realize I had to enable write support explicitly. Will
retry with that.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/3] usb-mtp: fix string length for filename when writing metadata

[Bandan Das]

Daniel P. Berrangé <berrange@redhat.com> writes:

> The ObjectInfo 'length' field provides the length of the
> wide character string filename. This is then converted to
> a multi-byte character string. This may have a different
> byte count to the wide character string. We should use the
> C string length of the multi-byte string instead.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  hw/usb/dev-mtp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index ebf210fbf8..838cd74da6 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1714,7 +1714,7 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
>          return;
>      }
>  
> -    o = usb_mtp_object_lookup_name(p, filename, dataset->length);
> +    o = usb_mtp_object_lookup_name(p, filename, -1);

Nit: Might as well just remove the "-1" argument and unconditionally use
strlen in usb_mtp_object_lookup_name

Bandan

>      if (o != NULL) {
>          next_handle = o->handle;
>      }

Re: [Qemu-devel] [PATCH 1/3] usb-mtp: fix string length for filename when writing metadata

[Bandan Das]

Daniel P. Berrangé <berrange@redhat.com> writes:

> The ObjectInfo 'length' field provides the length of the
> wide character string filename. This is then converted to
> a multi-byte character string. This may have a different
> byte count to the wide character string. We should use the
> C string length of the multi-byte string instead.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  hw/usb/dev-mtp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index ebf210fbf8..838cd74da6 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1714,7 +1714,7 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
>          return;
>      }
>  
> -    o = usb_mtp_object_lookup_name(p, filename, dataset->length);
> +    o = usb_mtp_object_lookup_name(p, filename, -1);

Nit: Might as well just remove the "-1" argument and unconditionally use
strlen in usb_mtp_object_lookup_name

Bandan

>      if (o != NULL) {
>          next_handle = o->handle;
>      }

Re: [Qemu-devel] [PATCH for-4.0? 0/3] usb-mtp: fix ObjectInfo request handling

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

On 4/15/19 10:45 AM, Daniel P. Berrangé wrote:
> Two previous attempts to fix this due to GCC 9 highlighting
> unaligned data access. My attempt:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
> 
> And a previous one:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html
> 
> There are a number of bugs in the USB MTP usb_mtp_write_metadata
> method handling the filename character set conversion.
> 
> The 2nd patch in this series is a security flaw fix since the
> code was not correctly validating guest provided data length.

Given that this is a security flaw, I've added this series to
https://wiki.qemu.org/Planning/4.0 in case you're hoping to get it in -rc4.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH for-4.0? 0/3] usb-mtp: fix ObjectInfo request handling

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

On 4/15/19 10:45 AM, Daniel P. Berrangé wrote:
> Two previous attempts to fix this due to GCC 9 highlighting
> unaligned data access. My attempt:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
> 
> And a previous one:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html
> 
> There are a number of bugs in the USB MTP usb_mtp_write_metadata
> method handling the filename character set conversion.
> 
> The 2nd patch in this series is a security flaw fix since the
> code was not correctly validating guest provided data length.

Given that this is a security flaw, I've added this series to
https://wiki.qemu.org/Planning/4.0 in case you're hoping to get it in -rc4.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH for-4.0? 0/3] usb-mtp: fix ObjectInfo request handling

[Peter Maydell]

On Mon, 15 Apr 2019 at 18:10, Eric Blake <eblake@redhat.com> wrote:
> On 4/15/19 10:45 AM, Daniel P. Berrangé wrote:
> > The 2nd patch in this series is a security flaw fix since the
> > code was not correctly validating guest provided data length.
>
> Given that this is a security flaw, I've added this series to
> https://wiki.qemu.org/Planning/4.0 in case you're hoping to get it in -rc4.

What are the consequences of the flaw ? IIRC it's only one
extra byte read?

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.0? 0/3] usb-mtp: fix ObjectInfo request handling

[Peter Maydell]

On Mon, 15 Apr 2019 at 18:10, Eric Blake <eblake@redhat.com> wrote:
> On 4/15/19 10:45 AM, Daniel P. Berrangé wrote:
> > The 2nd patch in this series is a security flaw fix since the
> > code was not correctly validating guest provided data length.
>
> Given that this is a security flaw, I've added this series to
> https://wiki.qemu.org/Planning/4.0 in case you're hoping to get it in -rc4.

What are the consequences of the flaw ? IIRC it's only one
extra byte read?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Daniel P. Berrangé]

On Mon, Apr 15, 2019 at 05:54:26PM +0100, Daniel P. Berrangé wrote:
> On Mon, Apr 15, 2019 at 12:52:41PM -0400, Bandan Das wrote:
> > Daniel P. Berrangé <berrange@redhat.com> writes:
> > 
> > > Two previous attempts to fix this due to GCC 9 highlighting
> > > unaligned data access. My attempt:
> > >
> > >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
> > >
> > > And a previous one:
> > >
> > >   https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
> > >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html
> > >
> > > There are a number of bugs in the USB MTP usb_mtp_write_metadata
> > > method handling the filename character set conversion.
> > >
> > > The 2nd patch in this series is a security flaw fix since the
> > > code was not correctly validating guest provided data length.
> > >
> > > I've been unable to figure out how to exercise the codepath that
> > > calls usb_mtp_write_metadata. At a guess, it looks like something
> > > that should be called when writing to a file from a guest, but the
> > > GNOME GVFS MTP driver doesn't provide write support. Using the
> > > command line MTP tools "mtp-sendfile" command results in an
> > > protocol error
> > >
> > >     # mtp-sendfile foo eek.txt
> > >     libmtp version: 1.1.14
> > >
> > 
> > The store is read only by default. Are you trying something like:
> >  -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?
> 
> Ah ha, I didn't realize I had to enable write support explicitly. Will
> retry with that.

Even after setting  readonly=false, I still can't get "mtp-sendfile"
to succeed in a guest.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Daniel P. Berrangé]

On Mon, Apr 15, 2019 at 05:54:26PM +0100, Daniel P. Berrangé wrote:
> On Mon, Apr 15, 2019 at 12:52:41PM -0400, Bandan Das wrote:
> > Daniel P. Berrangé <berrange@redhat.com> writes:
> > 
> > > Two previous attempts to fix this due to GCC 9 highlighting
> > > unaligned data access. My attempt:
> > >
> > >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
> > >
> > > And a previous one:
> > >
> > >   https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
> > >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html
> > >
> > > There are a number of bugs in the USB MTP usb_mtp_write_metadata
> > > method handling the filename character set conversion.
> > >
> > > The 2nd patch in this series is a security flaw fix since the
> > > code was not correctly validating guest provided data length.
> > >
> > > I've been unable to figure out how to exercise the codepath that
> > > calls usb_mtp_write_metadata. At a guess, it looks like something
> > > that should be called when writing to a file from a guest, but the
> > > GNOME GVFS MTP driver doesn't provide write support. Using the
> > > command line MTP tools "mtp-sendfile" command results in an
> > > protocol error
> > >
> > >     # mtp-sendfile foo eek.txt
> > >     libmtp version: 1.1.14
> > >
> > 
> > The store is read only by default. Are you trying something like:
> >  -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?
> 
> Ah ha, I didn't realize I had to enable write support explicitly. Will
> retry with that.

Even after setting  readonly=false, I still can't get "mtp-sendfile"
to succeed in a guest.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH for-4.0? 0/3] usb-mtp: fix ObjectInfo request handling

[Daniel P. Berrangé]

On Mon, Apr 15, 2019 at 06:18:01PM +0100, Peter Maydell wrote:
> On Mon, 15 Apr 2019 at 18:10, Eric Blake <eblake@redhat.com> wrote:
> > On 4/15/19 10:45 AM, Daniel P. Berrangé wrote:
> > > The 2nd patch in this series is a security flaw fix since the
> > > code was not correctly validating guest provided data length.
> >
> > Given that this is a security flaw, I've added this series to
> > https://wiki.qemu.org/Planning/4.0 in case you're hoping to get it in -rc4.
> 
> What are the consequences of the flaw ? IIRC it's only one
> extra byte read?

No, it is arbitrary extra bytes read.  We have a USB packet N bytes in
length, where N is supposed to match

  sizeof(ObjectInfo) + (ObjectInfo.length * 2)

but we checked it against

  sizeof(ObjectInfo) + ObjectInfo.length

As a result a malicious value for ObjectInfo.length can make QEMU can
read  ObjectInfo.length too many bytes when converting the string from
UTF16 to UTF8.

IOW, the returned UTF-8 string will end with whatever data is next on
the heap. This in turn creates a filename on disk with this random
data. I don't think this is a serious problem though, because if you
have enabled write support you've already given the guest ermission
to create arbitrary named files, so I don't see what they gain by
trying to exploit this. We already outlaw "/" which is the main danger
point in guest provided filenames.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH for-4.0? 0/3] usb-mtp: fix ObjectInfo request handling

[Daniel P. Berrangé]

On Mon, Apr 15, 2019 at 06:18:01PM +0100, Peter Maydell wrote:
> On Mon, 15 Apr 2019 at 18:10, Eric Blake <eblake@redhat.com> wrote:
> > On 4/15/19 10:45 AM, Daniel P. Berrangé wrote:
> > > The 2nd patch in this series is a security flaw fix since the
> > > code was not correctly validating guest provided data length.
> >
> > Given that this is a security flaw, I've added this series to
> > https://wiki.qemu.org/Planning/4.0 in case you're hoping to get it in -rc4.
> 
> What are the consequences of the flaw ? IIRC it's only one
> extra byte read?

No, it is arbitrary extra bytes read.  We have a USB packet N bytes in
length, where N is supposed to match

  sizeof(ObjectInfo) + (ObjectInfo.length * 2)

but we checked it against

  sizeof(ObjectInfo) + ObjectInfo.length

As a result a malicious value for ObjectInfo.length can make QEMU can
read  ObjectInfo.length too many bytes when converting the string from
UTF16 to UTF8.

IOW, the returned UTF-8 string will end with whatever data is next on
the heap. This in turn creates a filename on disk with this random
data. I don't think this is a serious problem though, because if you
have enabled write support you've already given the guest ermission
to create arbitrary named files, so I don't see what they gain by
trying to exploit this. We already outlaw "/" which is the main danger
point in guest provided filenames.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Peter Maydell]

On Mon, 15 Apr 2019 at 16:45, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Two previous attempts to fix this due to GCC 9 highlighting
> unaligned data access. My attempt:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
>
> And a previous one:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html
>
> There are a number of bugs in the USB MTP usb_mtp_write_metadata
> method handling the filename character set conversion.
>
> The 2nd patch in this series is a security flaw fix since the
> code was not correctly validating guest provided data length.

Given that we don't seem to be confident in this fix just now,
and this is a read-only buffer overrun in a not-commonly-used
feature that only happens if you explicitly enable write support,
my current thought is that we should not try to put this into 4.0
(but instead treat it as we would a security issue that had
occurred after we released 4.0).

Opinions? Maybe we should just apply patch 2/3 for 4.0 ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Peter Maydell]

On Mon, 15 Apr 2019 at 16:45, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Two previous attempts to fix this due to GCC 9 highlighting
> unaligned data access. My attempt:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
>
> And a previous one:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html
>
> There are a number of bugs in the USB MTP usb_mtp_write_metadata
> method handling the filename character set conversion.
>
> The 2nd patch in this series is a security flaw fix since the
> code was not correctly validating guest provided data length.

Given that we don't seem to be confident in this fix just now,
and this is a read-only buffer overrun in a not-commonly-used
feature that only happens if you explicitly enable write support,
my current thought is that we should not try to put this into 4.0
(but instead treat it as we would a security issue that had
occurred after we released 4.0).

Opinions? Maybe we should just apply patch 2/3 for 4.0 ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Bandan Das]

Daniel P. Berrangé <berrange@redhat.com> writes:
...
>> > The store is read only by default. Are you trying something like:
>> >  -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?
>> 
>> Ah ha, I didn't realize I had to enable write support explicitly. Will
>> retry with that.
>
> Even after setting  readonly=false, I still can't get "mtp-sendfile"
> to succeed in a guest.
>
I posted a patch for a bug introduced by a recent commit that made smaller
file sizes return back with a incomplete file transfer.

https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg02552.html

> Regards,
> Daniel

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Bandan Das]

Daniel P. Berrangé <berrange@redhat.com> writes:
...
>> > The store is read only by default. Are you trying something like:
>> >  -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?
>> 
>> Ah ha, I didn't realize I had to enable write support explicitly. Will
>> retry with that.
>
> Even after setting  readonly=false, I still can't get "mtp-sendfile"
> to succeed in a guest.
>
I posted a patch for a bug introduced by a recent commit that made smaller
file sizes return back with a incomplete file transfer.

https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg02552.html

> Regards,
> Daniel

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Daniel P. Berrangé]

On Tue, Apr 16, 2019 at 12:10:16PM -0400, Bandan Das wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> ...
> >> > The store is read only by default. Are you trying something like:
> >> >  -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?
> >> 
> >> Ah ha, I didn't realize I had to enable write support explicitly. Will
> >> retry with that.
> >
> > Even after setting  readonly=false, I still can't get "mtp-sendfile"
> > to succeed in a guest.
> >
> I posted a patch for a bug introduced by a recent commit that made smaller
> file sizes return back with a incomplete file transfer.
> 
> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg02552.html

Yes, I applied that and didn't see any difference in behaviour

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Daniel P. Berrangé]

On Tue, Apr 16, 2019 at 12:10:16PM -0400, Bandan Das wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> ...
> >> > The store is read only by default. Are you trying something like:
> >> >  -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?
> >> 
> >> Ah ha, I didn't realize I had to enable write support explicitly. Will
> >> retry with that.
> >
> > Even after setting  readonly=false, I still can't get "mtp-sendfile"
> > to succeed in a guest.
> >
> I posted a patch for a bug introduced by a recent commit that made smaller
> file sizes return back with a incomplete file transfer.
> 
> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg02552.html

Yes, I applied that and didn't see any difference in behaviour

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Bandan Das]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Apr 16, 2019 at 12:10:16PM -0400, Bandan Das wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> ...
>> >> > The store is read only by default. Are you trying something like:
>> >> >  -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?
>> >> 
>> >> Ah ha, I didn't realize I had to enable write support explicitly. Will
>> >> retry with that.
>> >
>> > Even after setting  readonly=false, I still can't get "mtp-sendfile"
>> > to succeed in a guest.
>> >
>> I posted a patch for a bug introduced by a recent commit that made smaller
>> file sizes return back with a incomplete file transfer.
>> 
>> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg02552.html
>
> Yes, I applied that and didn't see any difference in behaviour
>

Just noticed the error message you posted:

    Error sending file.
    Error 2: PTP Layer error 02ff: send_file_object_info(): Could not send object info.
    Error 2: Error 02ff: PTP I/O Error
    ERROR: Could not close session!

I can't find usb-mtp sending a "I/O error" on an error condition
for the objectinfo phase. It might be libmtp or even the command itself
failing for some reason. For incomplete transfer, I just checked, it's
spitting out the error message correctly as INCOMPLETE_FILE_TRANSFER.

With libmtp version 1.13 on a FC24 guest, here's the output:

$ mtp-sendfile test.txt test.img
libmtp version: 1.1.13

Device 0 (VID=46f4 and PID=0004) is UNKNOWN in libmtp v1.1.13.
Please report this VID/PID and the device model to the libmtp development team
ignoring libusb_claim_interface() = -6PTP_ERROR_IO: failed to open session, trying again after resetting USB interface
LIBMTP libusb: Attempt to reset device
Sending test.txt to test.img
type: txt, 44
Sending file...
Progress: 322 of 322 (100%)
New file ID: 7

What guest is this ? I can try to reproduce.

> Regards,
> Daniel

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Bandan Das]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Apr 16, 2019 at 12:10:16PM -0400, Bandan Das wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> ...
>> >> > The store is read only by default. Are you trying something like:
>> >> >  -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?
>> >> 
>> >> Ah ha, I didn't realize I had to enable write support explicitly. Will
>> >> retry with that.
>> >
>> > Even after setting  readonly=false, I still can't get "mtp-sendfile"
>> > to succeed in a guest.
>> >
>> I posted a patch for a bug introduced by a recent commit that made smaller
>> file sizes return back with a incomplete file transfer.
>> 
>> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg02552.html
>
> Yes, I applied that and didn't see any difference in behaviour
>

Just noticed the error message you posted:

    Error sending file.
    Error 2: PTP Layer error 02ff: send_file_object_info(): Could not send object info.
    Error 2: Error 02ff: PTP I/O Error
    ERROR: Could not close session!

I can't find usb-mtp sending a "I/O error" on an error condition
for the objectinfo phase. It might be libmtp or even the command itself
failing for some reason. For incomplete transfer, I just checked, it's
spitting out the error message correctly as INCOMPLETE_FILE_TRANSFER.

With libmtp version 1.13 on a FC24 guest, here's the output:

$ mtp-sendfile test.txt test.img
libmtp version: 1.1.13

Device 0 (VID=46f4 and PID=0004) is UNKNOWN in libmtp v1.1.13.
Please report this VID/PID and the device model to the libmtp development team
ignoring libusb_claim_interface() = -6PTP_ERROR_IO: failed to open session, trying again after resetting USB interface
LIBMTP libusb: Attempt to reset device
Sending test.txt to test.img
type: txt, 44
Sending file...
Progress: 322 of 322 (100%)
New file ID: 7

What guest is this ? I can try to reproduce.

> Regards,
> Daniel

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Daniel P. Berrangé]

On Tue, Apr 16, 2019 at 12:45:04PM -0400, Bandan Das wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Tue, Apr 16, 2019 at 12:10:16PM -0400, Bandan Das wrote:
> >> Daniel P. Berrangé <berrange@redhat.com> writes:
> >> ...
> >> >> > The store is read only by default. Are you trying something like:
> >> >> >  -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?
> >> >> 
> >> >> Ah ha, I didn't realize I had to enable write support explicitly. Will
> >> >> retry with that.
> >> >
> >> > Even after setting  readonly=false, I still can't get "mtp-sendfile"
> >> > to succeed in a guest.
> >> >
> >> I posted a patch for a bug introduced by a recent commit that made smaller
> >> file sizes return back with a incomplete file transfer.
> >> 
> >> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg02552.html
> >
> > Yes, I applied that and didn't see any difference in behaviour
> >
> 
> Just noticed the error message you posted:
> 
>     Error sending file.
>     Error 2: PTP Layer error 02ff: send_file_object_info(): Could not send object info.
>     Error 2: Error 02ff: PTP I/O Error
>     ERROR: Could not close session!
> 
> I can't find usb-mtp sending a "I/O error" on an error condition
> for the objectinfo phase. It might be libmtp or even the command itself
> failing for some reason. For incomplete transfer, I just checked, it's
> spitting out the error message correctly as INCOMPLETE_FILE_TRANSFER.
> 
> With libmtp version 1.13 on a FC24 guest, here's the output:
> 
> $ mtp-sendfile test.txt test.img
> libmtp version: 1.1.13
> 
> Device 0 (VID=46f4 and PID=0004) is UNKNOWN in libmtp v1.1.13.
> Please report this VID/PID and the device model to the libmtp development team
> ignoring libusb_claim_interface() = -6PTP_ERROR_IO: failed to open session, trying again after resetting USB interface
> LIBMTP libusb: Attempt to reset device
> Sending test.txt to test.img
> type: txt, 44
> Sending file...
> Progress: 322 of 322 (100%)
> New file ID: 7
> 
> What guest is this ? I can try to reproduce.

This was Fedora 26, so technically it is end of life, but then so
is your F24 guest :-)

Should really check with a modern guest like Fedora 29 or rawhide.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Daniel P. Berrangé]

On Tue, Apr 16, 2019 at 12:45:04PM -0400, Bandan Das wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Tue, Apr 16, 2019 at 12:10:16PM -0400, Bandan Das wrote:
> >> Daniel P. Berrangé <berrange@redhat.com> writes:
> >> ...
> >> >> > The store is read only by default. Are you trying something like:
> >> >> >  -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?
> >> >> 
> >> >> Ah ha, I didn't realize I had to enable write support explicitly. Will
> >> >> retry with that.
> >> >
> >> > Even after setting  readonly=false, I still can't get "mtp-sendfile"
> >> > to succeed in a guest.
> >> >
> >> I posted a patch for a bug introduced by a recent commit that made smaller
> >> file sizes return back with a incomplete file transfer.
> >> 
> >> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg02552.html
> >
> > Yes, I applied that and didn't see any difference in behaviour
> >
> 
> Just noticed the error message you posted:
> 
>     Error sending file.
>     Error 2: PTP Layer error 02ff: send_file_object_info(): Could not send object info.
>     Error 2: Error 02ff: PTP I/O Error
>     ERROR: Could not close session!
> 
> I can't find usb-mtp sending a "I/O error" on an error condition
> for the objectinfo phase. It might be libmtp or even the command itself
> failing for some reason. For incomplete transfer, I just checked, it's
> spitting out the error message correctly as INCOMPLETE_FILE_TRANSFER.
> 
> With libmtp version 1.13 on a FC24 guest, here's the output:
> 
> $ mtp-sendfile test.txt test.img
> libmtp version: 1.1.13
> 
> Device 0 (VID=46f4 and PID=0004) is UNKNOWN in libmtp v1.1.13.
> Please report this VID/PID and the device model to the libmtp development team
> ignoring libusb_claim_interface() = -6PTP_ERROR_IO: failed to open session, trying again after resetting USB interface
> LIBMTP libusb: Attempt to reset device
> Sending test.txt to test.img
> type: txt, 44
> Sending file...
> Progress: 322 of 322 (100%)
> New file ID: 7
> 
> What guest is this ? I can try to reproduce.

This was Fedora 26, so technically it is end of life, but then so
is your F24 guest :-)

Should really check with a modern guest like Fedora 29 or rawhide.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Bandan Das]

Daniel P. Berrangé <berrange@redhat.com> writes:

>> I can't find usb-mtp sending a "I/O error" on an error condition
>> for the objectinfo phase. It might be libmtp or even the command itself
>> failing for some reason. For incomplete transfer, I just checked, it's
>> spitting out the error message correctly as INCOMPLETE_FILE_TRANSFER.
>> 
>> With libmtp version 1.13 on a FC24 guest, here's the output:
>> 
>> $ mtp-sendfile test.txt test.img
>> libmtp version: 1.1.13
>> 
>> Device 0 (VID=46f4 and PID=0004) is UNKNOWN in libmtp v1.1.13.
>> Please report this VID/PID and the device model to the libmtp development team
>> ignoring libusb_claim_interface() = -6PTP_ERROR_IO: failed to open session, trying again after resetting USB interface
>> LIBMTP libusb: Attempt to reset device
>> Sending test.txt to test.img
>> type: txt, 44
>> Sending file...
>> Progress: 322 of 322 (100%)
>> New file ID: 7
>> 
>> What guest is this ? I can try to reproduce.
>
> This was Fedora 26, so technically it is end of life, but then so
> is your F24 guest :-)
>
> Should really check with a modern guest like Fedora 29 or rawhide.
>
I had a F28 guest and here, mtp-sendfile appears to succeed but no file is written
However, nautilus works fine, it doesn't appear like an issue with libmtp
or Qemu.

> Regards,
> Daniel

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Bandan Das]

Daniel P. Berrangé <berrange@redhat.com> writes:

>> I can't find usb-mtp sending a "I/O error" on an error condition
>> for the objectinfo phase. It might be libmtp or even the command itself
>> failing for some reason. For incomplete transfer, I just checked, it's
>> spitting out the error message correctly as INCOMPLETE_FILE_TRANSFER.
>> 
>> With libmtp version 1.13 on a FC24 guest, here's the output:
>> 
>> $ mtp-sendfile test.txt test.img
>> libmtp version: 1.1.13
>> 
>> Device 0 (VID=46f4 and PID=0004) is UNKNOWN in libmtp v1.1.13.
>> Please report this VID/PID and the device model to the libmtp development team
>> ignoring libusb_claim_interface() = -6PTP_ERROR_IO: failed to open session, trying again after resetting USB interface
>> LIBMTP libusb: Attempt to reset device
>> Sending test.txt to test.img
>> type: txt, 44
>> Sending file...
>> Progress: 322 of 322 (100%)
>> New file ID: 7
>> 
>> What guest is this ? I can try to reproduce.
>
> This was Fedora 26, so technically it is end of life, but then so
> is your F24 guest :-)
>
> Should really check with a modern guest like Fedora 29 or rawhide.
>
I had a F28 guest and here, mtp-sendfile appears to succeed but no file is written
However, nautilus works fine, it doesn't appear like an issue with libmtp
or Qemu.

> Regards,
> Daniel

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Peter Maydell]

On Tue, 16 Apr 2019 at 14:35, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 15 Apr 2019 at 16:45, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > Two previous attempts to fix this due to GCC 9 highlighting
> > unaligned data access. My attempt:
> >
> >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
> >
> > And a previous one:
> >
> >   https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
> >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html
> >
> > There are a number of bugs in the USB MTP usb_mtp_write_metadata
> > method handling the filename character set conversion.
> >
> > The 2nd patch in this series is a security flaw fix since the
> > code was not correctly validating guest provided data length.
>
> Given that we don't seem to be confident in this fix just now,
> and this is a read-only buffer overrun in a not-commonly-used
> feature that only happens if you explicitly enable write support,
> my current thought is that we should not try to put this into 4.0
> (but instead treat it as we would a security issue that had
> occurred after we released 4.0).
>
> Opinions? Maybe we should just apply patch 2/3 for 4.0 ?

Having thought a bit more I think I'd definitely like to apply
just patch 2 for 4.0. Could people try to test that and confirm
that it at least does not make the feature behave any worse?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Peter Maydell]

On Tue, 16 Apr 2019 at 14:35, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 15 Apr 2019 at 16:45, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > Two previous attempts to fix this due to GCC 9 highlighting
> > unaligned data access. My attempt:
> >
> >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
> >
> > And a previous one:
> >
> >   https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
> >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html
> >
> > There are a number of bugs in the USB MTP usb_mtp_write_metadata
> > method handling the filename character set conversion.
> >
> > The 2nd patch in this series is a security flaw fix since the
> > code was not correctly validating guest provided data length.
>
> Given that we don't seem to be confident in this fix just now,
> and this is a read-only buffer overrun in a not-commonly-used
> feature that only happens if you explicitly enable write support,
> my current thought is that we should not try to put this into 4.0
> (but instead treat it as we would a security issue that had
> occurred after we released 4.0).
>
> Opinions? Maybe we should just apply patch 2/3 for 4.0 ?

Having thought a bit more I think I'd definitely like to apply
just patch 2 for 4.0. Could people try to test that and confirm
that it at least does not make the feature behave any worse?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Peter Maydell]

On Tue, 16 Apr 2019 at 18:27, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 16 Apr 2019 at 14:35, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Mon, 15 Apr 2019 at 16:45, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > Two previous attempts to fix this due to GCC 9 highlighting
> > > unaligned data access. My attempt:
> > >
> > >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
> > >
> > > And a previous one:
> > >
> > >   https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
> > >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html
> > >
> > > There are a number of bugs in the USB MTP usb_mtp_write_metadata
> > > method handling the filename character set conversion.
> > >
> > > The 2nd patch in this series is a security flaw fix since the
> > > code was not correctly validating guest provided data length.
> >
> > Given that we don't seem to be confident in this fix just now,
> > and this is a read-only buffer overrun in a not-commonly-used
> > feature that only happens if you explicitly enable write support,
> > my current thought is that we should not try to put this into 4.0
> > (but instead treat it as we would a security issue that had
> > occurred after we released 4.0).
> >
> > Opinions? Maybe we should just apply patch 2/3 for 4.0 ?
>
> Having thought a bit more I think I'd definitely like to apply
> just patch 2 for 4.0. Could people try to test that and confirm
> that it at least does not make the feature behave any worse?

I've done a tentative merge test of patch 2, which is OK.
I'd like to push that either today or tomorrow (uk time):
objections?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Peter Maydell]

On Tue, 16 Apr 2019 at 18:27, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 16 Apr 2019 at 14:35, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Mon, 15 Apr 2019 at 16:45, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > Two previous attempts to fix this due to GCC 9 highlighting
> > > unaligned data access. My attempt:
> > >
> > >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
> > >
> > > And a previous one:
> > >
> > >   https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
> > >   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg00162.html
> > >
> > > There are a number of bugs in the USB MTP usb_mtp_write_metadata
> > > method handling the filename character set conversion.
> > >
> > > The 2nd patch in this series is a security flaw fix since the
> > > code was not correctly validating guest provided data length.
> >
> > Given that we don't seem to be confident in this fix just now,
> > and this is a read-only buffer overrun in a not-commonly-used
> > feature that only happens if you explicitly enable write support,
> > my current thought is that we should not try to put this into 4.0
> > (but instead treat it as we would a security issue that had
> > occurred after we released 4.0).
> >
> > Opinions? Maybe we should just apply patch 2/3 for 4.0 ?
>
> Having thought a bit more I think I'd definitely like to apply
> just patch 2 for 4.0. Could people try to test that and confirm
> that it at least does not make the feature behave any worse?

I've done a tentative merge test of patch 2, which is OK.
I'd like to push that either today or tomorrow (uk time):
objections?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/3] usb-mtp: fix bounds check for guest provided filename

[Bandan Das]

Daniel P. Berrangé <berrange@redhat.com> writes:

> The ObjectInfo struct has a variable length array containing the UTF-16
> encoded filename. The number of characters of trailing data is given by
> the 'length' field in the struct and this must be validated against the
> size of the data packet received from the guest.
>
> Since the data is UTF-16, we must convert the byte count we have to a
> character count before validating. This must take care to truncate if
> a malicious guest sent an odd number of bytes.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  hw/usb/dev-mtp.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 838cd74da6..6b7d1296e4 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1699,12 +1699,19 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
>      MTPObject *o;
>      MTPObject *p = usb_mtp_object_lookup(s, s->dataset.parent_handle);
>      uint32_t next_handle = s->next_handle;
> +    size_t filename_chars = dlen - offsetof(ObjectInfo, filename);
> +
> +    /*
> +     * filename is utf-16. We're intentionally doing
> +     * integer division to truncate if malicious guest
> +     * sent an odd number of bytes.
> +     */
> +    filename_chars /= 2;
>  
>      assert(!s->write_pending);
>      assert(p != NULL);
>  
> -    filename = utf16_to_str(MIN(dataset->length,
> -                                dlen - offsetof(ObjectInfo, filename)),
> +    filename = utf16_to_str(MIN(dataset->length, filename_chars),
>                              dataset->filename);
>  
>      if (strchr(filename, '/')) {

Reviewed-by: Bandan Das <bsd@redhat.com>

Re: [Qemu-devel] [PATCH 2/3] usb-mtp: fix bounds check for guest provided filename

[Bandan Das]

Daniel P. Berrangé <berrange@redhat.com> writes:

> The ObjectInfo struct has a variable length array containing the UTF-16
> encoded filename. The number of characters of trailing data is given by
> the 'length' field in the struct and this must be validated against the
> size of the data packet received from the guest.
>
> Since the data is UTF-16, we must convert the byte count we have to a
> character count before validating. This must take care to truncate if
> a malicious guest sent an odd number of bytes.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  hw/usb/dev-mtp.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 838cd74da6..6b7d1296e4 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1699,12 +1699,19 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
>      MTPObject *o;
>      MTPObject *p = usb_mtp_object_lookup(s, s->dataset.parent_handle);
>      uint32_t next_handle = s->next_handle;
> +    size_t filename_chars = dlen - offsetof(ObjectInfo, filename);
> +
> +    /*
> +     * filename is utf-16. We're intentionally doing
> +     * integer division to truncate if malicious guest
> +     * sent an odd number of bytes.
> +     */
> +    filename_chars /= 2;
>  
>      assert(!s->write_pending);
>      assert(p != NULL);
>  
> -    filename = utf16_to_str(MIN(dataset->length,
> -                                dlen - offsetof(ObjectInfo, filename)),
> +    filename = utf16_to_str(MIN(dataset->length, filename_chars),
>                              dataset->filename);
>  
>      if (strchr(filename, '/')) {

Reviewed-by: Bandan Das <bsd@redhat.com>

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Peter Maydell]

On Tue, 16 Apr 2019 at 20:33, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 16 Apr 2019 at 18:27, Peter Maydell <peter.maydell@linaro.org> wrote:
> > Having thought a bit more I think I'd definitely like to apply
> > just patch 2 for 4.0. Could people try to test that and confirm
> > that it at least does not make the feature behave any worse?
>
> I've done a tentative merge test of patch 2, which is OK.
> I'd like to push that either today or tomorrow (uk time):
> objections?

...now pushed.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Peter Maydell]

On Tue, 16 Apr 2019 at 20:33, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 16 Apr 2019 at 18:27, Peter Maydell <peter.maydell@linaro.org> wrote:
> > Having thought a bit more I think I'd definitely like to apply
> > just patch 2 for 4.0. Could people try to test that and confirm
> > that it at least does not make the feature behave any worse?
>
> I've done a tentative merge test of patch 2, which is OK.
> I'd like to push that either today or tomorrow (uk time):
> objections?

...now pushed.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Gerd Hoffmann]

On Tue, Apr 16, 2019 at 11:27:06PM +0100, Peter Maydell wrote:
> On Tue, 16 Apr 2019 at 20:33, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Tue, 16 Apr 2019 at 18:27, Peter Maydell <peter.maydell@linaro.org> wrote:
> > > Having thought a bit more I think I'd definitely like to apply
> > > just patch 2 for 4.0. Could people try to test that and confirm
> > > that it at least does not make the feature behave any worse?
> >
> > I've done a tentative merge test of patch 2, which is OK.
> > I'd like to push that either today or tomorrow (uk time):
> > objections?
> 
> ...now pushed.

Added the other two to the (4.1) usb queue.

thanks,
  Gerd

Re: [Qemu-devel] [PATCH 0/3] usb-mtp: fix ObjectInfo request handling

[Gerd Hoffmann]

On Tue, Apr 16, 2019 at 11:27:06PM +0100, Peter Maydell wrote:
> On Tue, 16 Apr 2019 at 20:33, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Tue, 16 Apr 2019 at 18:27, Peter Maydell <peter.maydell@linaro.org> wrote:
> > > Having thought a bit more I think I'd definitely like to apply
> > > just patch 2 for 4.0. Could people try to test that and confirm
> > > that it at least does not make the feature behave any worse?
> >
> > I've done a tentative merge test of patch 2, which is OK.
> > I'd like to push that either today or tomorrow (uk time):
> > objections?
> 
> ...now pushed.

Added the other two to the (4.1) usb queue.

thanks,
  Gerd