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

[Bandan Das]

Daniel P. Berrangé <address@hidden> 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
>
>     address@hidden usb_mtp_command dev 4, code 0x9803, trans 0x18, args 0x11, 
> 0xdc04, 0x0, 0x0, 0x0
>     address@hidden usb_mtp_xfer dev 4, ep 2, 20/20
>     address@hidden usb_mtp_data_in dev 4, trans 0x18, len 8
>     address@hidden usb_mtp_xfer dev 4, ep 1, 20/512
>     address@hidden usb_mtp_success dev 4, trans 0x18, args 0x0, 0x0
>     address@hidden usb_mtp_xfer dev 4, ep 1, 12/512
>     address@hidden usb_mtp_command dev 4, code 0x100c, trans 0x19, args 
> 0x10001, 0xc, 0x0, 0x0, 0x0
>     address@hidden usb_mtp_xfer dev 4, ep 2, 20/20
>     address@hidden usb_mtp_stall dev 4, reason: awaiting data-out
>     address@hidden usb_mtp_stall dev 4, reason: transaction inflight
>     address@hidden usb_mtp_stall dev 4, reason: unknown control request
>     address@hidden 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(-)