[Qemu-devel] [PATCH for 2.10 16/35] usb/dev-mtp: fix use of uninitialized values

Philippe Mathieu-Daudé posted 35 patches 8 years, 3 months ago
There is a newer version of this series
[Qemu-devel] [PATCH for 2.10 16/35] usb/dev-mtp: fix use of uninitialized values
Posted by Philippe Mathieu-Daudé 8 years, 3 months ago
hw/usb/dev-mtp.c:1212:13: warning: 2nd function call argument is an uninitialized value
        o = usb_mtp_object_lookup(s, c->argv[0]);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/usb/dev-mtp.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 94c2e94f10..6dfece9ea9 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1209,7 +1209,9 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
         }
         break;
     case CMD_GET_OBJECT_INFO:
-        o = usb_mtp_object_lookup(s, c->argv[0]);
+        if (c->argc > 0) {
+            o = usb_mtp_object_lookup(s, c->argv[0]);
+        }
         if (o == NULL) {
             usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
                                  c->trans, 0, 0, 0);
@@ -1218,7 +1220,9 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
         data_in = usb_mtp_get_object_info(s, c, o);
         break;
     case CMD_GET_OBJECT:
-        o = usb_mtp_object_lookup(s, c->argv[0]);
+        if (c->argc > 0) {
+            o = usb_mtp_object_lookup(s, c->argv[0]);
+        }
         if (o == NULL) {
             usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
                                  c->trans, 0, 0, 0);
@@ -1237,7 +1241,9 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
         }
         break;
     case CMD_GET_PARTIAL_OBJECT:
-        o = usb_mtp_object_lookup(s, c->argv[0]);
+        if (c->argc > 0) {
+            o = usb_mtp_object_lookup(s, c->argv[0]);
+        }
         if (o == NULL) {
             usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
                                  c->trans, 0, 0, 0);
@@ -1281,7 +1287,9 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
         }
         break;
     case CMD_GET_OBJECT_PROP_VALUE:
-        o = usb_mtp_object_lookup(s, c->argv[0]);
+        if (c->argc > 0) {
+            o = usb_mtp_object_lookup(s, c->argv[0]);
+        }
         if (o == NULL) {
             usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
                                  c->trans, 0, 0, 0);
-- 
2.13.3


Re: [Qemu-devel] [PATCH for 2.10 16/35] usb/dev-mtp: fix use of uninitialized values
Posted by Gerd Hoffmann 8 years, 3 months ago
     case CMD_GET_OBJECT_INFO:
> -        o = usb_mtp_object_lookup(s, c->argv[0]);
> +        if (c->argc > 0) {
> +            o = usb_mtp_object_lookup(s, c->argv[0]);
> +        }

How about zero-initializing c->argv instead?

cheers,
  Gerd


Re: [Qemu-devel] [PATCH for 2.10 16/35] usb/dev-mtp: fix use of uninitialized values
Posted by Philippe Mathieu-Daudé 8 years, 3 months ago
On 07/25/2017 09:34 AM, Gerd Hoffmann wrote:
>       case CMD_GET_OBJECT_INFO:
>> -        o = usb_mtp_object_lookup(s, c->argv[0]);
>> +        if (c->argc > 0) {
>> +            o = usb_mtp_object_lookup(s, c->argv[0]);
>> +        }
> 
> How about zero-initializing c->argv instead?

I checked the MTP specs rev. 1.1 and I understand the case argc == 0 
fits in "Invalid Parameter" section (F.2.30, code 0x201d).

So the correct patch is to queue a RES_INVALID_PARAMETER result.

I'll send another patch but since this require heavy testing this is 
probably 2.11 material now.

Regards,

Phil.