[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 5/5] usb-mtp: Advertise SendObjectInfo for write
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PULL 5/5] usb-mtp: Advertise SendObjectInfo for write support |
Date: |
Fri, 27 Apr 2018 14:35:29 +0100 |
On 27 February 2018 at 08:39, Gerd Hoffmann <address@hidden> wrote:
> From: Bandan Das <address@hidden>
>
> This patch implements a dummy ObjectInfo structure so that
> it's easy to typecast the incoming data. If the metadata is
> valid, write_pending is set. Also, the incoming filename
> is utf-16, so, instead of depending on external libraries, just
> implement a simple function to get the filename
>
> Signed-off-by: Bandan Das <address@hidden>
> Message-id: address@hidden
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---
> hw/usb/dev-mtp.c | 136
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 134 insertions(+), 2 deletions(-)
Hi; Coverity points out a pointer use-after-NULL-check in this
code (CID1390592):
> + case CMD_SEND_OBJECT_INFO:
> + /* Return error if store is read-only */
> + if (!FLAG_SET(s, MTP_FLAG_WRITABLE)) {
> + usb_mtp_queue_result(s, RES_STORE_READ_ONLY,
> + c->trans, 0, 0, 0, 0);
> + } else if (c->argv[0] && (c->argv[0] != QEMU_STORAGE_ID)) {
> + /* First parameter points to storage id or is 0 */
> + usb_mtp_queue_result(s, RES_STORE_NOT_AVAILABLE, c->trans,
> + 0, 0, 0, 0);
> + } else if (c->argv[1] && !c->argv[0]) {
> + /* If second parameter is specified, first must also be
> specified */
> + usb_mtp_queue_result(s, RES_DESTINATION_UNSUPPORTED, c->trans,
> + 0, 0, 0, 0);
> + } else {
> + uint32_t handle = c->argv[1];
> + if (handle == 0xFFFFFFFF || handle == 0) {
> + /* root object */
> + o = QTAILQ_FIRST(&s->objects);
> + } else {
> + o = usb_mtp_object_lookup(s, handle);
> + }
> + if (o == NULL) {
Here we do a NULL check on 'o'...
> + usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE, c->trans,
> + 0, 0, 0, 0);
> + }
> + if (o->format != FMT_ASSOCIATION) {
...but here we dereference 'o', which will crash if it is NULL.
> + usb_mtp_queue_result(s, RES_INVALID_PARENT_OBJECT, c->trans,
> + 0, 0, 0, 0);
> + }
> + }
> + if (o) {
> + s->dataset.parent_handle = o->handle;
> + }
> + s->data_out = usb_mtp_data_alloc(c);
> + return;
thanks
-- PMM