mbox series

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

Message ID 20190415154503.6758-1-berrange@redhat.com (mailing list archive)
Headers show
Series usb-mtp: fix ObjectInfo request handling | expand

Message

Daniel P. Berrangé April 15, 2019, 3:45 p.m. UTC
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(-)

Comments

Bandan Das April 15, 2019, 4:52 p.m. UTC | #1
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(-)
Daniel P. Berrangé April 15, 2019, 4:54 p.m. UTC | #2
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
Eric Blake April 15, 2019, 5:09 p.m. UTC | #3
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.
Peter Maydell April 15, 2019, 5:18 p.m. UTC | #4
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
Daniel P. Berrangé April 16, 2019, 8:40 a.m. UTC | #5
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
Daniel P. Berrangé April 16, 2019, 8:48 a.m. UTC | #6
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
Peter Maydell April 16, 2019, 1:35 p.m. UTC | #7
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
Bandan Das April 16, 2019, 4:10 p.m. UTC | #8
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
Daniel P. Berrangé April 16, 2019, 4:12 p.m. UTC | #9
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
Bandan Das April 16, 2019, 4:45 p.m. UTC | #10
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
Daniel P. Berrangé April 16, 2019, 4:52 p.m. UTC | #11
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
Bandan Das April 16, 2019, 5:20 p.m. UTC | #12
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
Peter Maydell April 16, 2019, 5:27 p.m. UTC | #13
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
Peter Maydell April 16, 2019, 7:33 p.m. UTC | #14
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
Peter Maydell April 16, 2019, 10:27 p.m. UTC | #15
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
Gerd Hoffmann April 17, 2019, 8:27 a.m. UTC | #16
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