diff mbox

[for,2.10,17/35] usb/dev-mtp: fix use of uninitialized values

Message ID 20170724182751.18261-18-f4bug@amsat.org
State New
Headers show

Commit Message

Philippe Mathieu-Daudé July 24, 2017, 6:27 p.m. UTC 
Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/usb/dev-mtp.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

Comments

Philippe Mathieu-Daudé July 26, 2017, 11:26 p.m. UTC | #1 
On 07/24/2017 03:27 PM, Philippe Mathieu-Daudé wrote:
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   hw/usb/dev-mtp.c | 36 +++++++++++++++++++++---------------
>   1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 6dfece9ea9..ad64495f05 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1134,7 +1134,7 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
>                                    c->trans, 1, s->session, 0);
>               return;
>           }
> -        if (c->argv[0] == 0) {
> +        if (c->argc == 0 || c->argv[0] == 0) {
>               usb_mtp_queue_result(s, RES_INVALID_PARAMETER,
>                                    c->trans, 0, 0, 0);

^ This is OK,

but part below is incorrect, after reading the MTP specs 1.1 I 
understood the correct code to return is RES_INVALID_PARAMETER.

>               return;
> @@ -1162,8 +1162,9 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
>           data_in = usb_mtp_get_storage_ids(s, c);
>           break;
>       case CMD_GET_STORAGE_INFO:
> -        if (c->argv[0] != QEMU_STORAGE_ID &&
> -            c->argv[0] != 0xffffffff) {
> +        if (c->argc == 0 ||
> +           (c->argv[0] != QEMU_STORAGE_ID &&
> +            c->argv[0] != 0xffffffff)) {
>               usb_mtp_queue_result(s, RES_INVALID_STORAGE_ID,
>                                    c->trans, 0, 0, 0);
>               return;
> @@ -1172,22 +1173,25 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
>           break;
>       case CMD_GET_NUM_OBJECTS:
>       case CMD_GET_OBJECT_HANDLES:
> -        if (c->argv[0] != QEMU_STORAGE_ID &&
> -            c->argv[0] != 0xffffffff) {
> +        if (c->argc == 0 ||
> +           (c->argv[0] != QEMU_STORAGE_ID &&
> +            c->argv[0] != 0xffffffff)) {
>               usb_mtp_queue_result(s, RES_INVALID_STORAGE_ID,
>                                    c->trans, 0, 0, 0);
>               return;
>           }
> -        if (c->argv[1] != 0x00000000) {
> +        if (c->argc > 1 && c->argv[1] != 0x00000000) {
>               usb_mtp_queue_result(s, RES_SPEC_BY_FORMAT_UNSUPPORTED,
>                                    c->trans, 0, 0, 0);
>               return;
>           }
> -        if (c->argv[2] == 0x00000000 ||
> -            c->argv[2] == 0xffffffff) {
> -            o = QTAILQ_FIRST(&s->objects);
> -        } else {
> -            o = usb_mtp_object_lookup(s, c->argv[2]);
> +        if (c->argc > 2) {
> +            if (c->argv[2] == 0x00000000 ||
> +                c->argv[2] == 0xffffffff) {
> +                o = QTAILQ_FIRST(&s->objects);
> +            } else {
> +                o = usb_mtp_object_lookup(s, c->argv[2]);
> +            }
>           }
>           if (o == NULL) {
>               usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
> @@ -1264,8 +1268,9 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
>           res0 = data_in->length;
>           break;
>       case CMD_GET_OBJECT_PROPS_SUPPORTED:
> -        if (c->argv[0] != FMT_UNDEFINED_OBJECT &&
> -            c->argv[0] != FMT_ASSOCIATION) {
> +        if (c->argc == 0 ||
> +           (c->argv[0] != FMT_UNDEFINED_OBJECT &&
> +            c->argv[0] != FMT_ASSOCIATION)) {
>               usb_mtp_queue_result(s, RES_INVALID_OBJECT_FORMAT_CODE,
>                                    c->trans, 0, 0, 0);
>               return;
> @@ -1273,8 +1278,9 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
>           data_in = usb_mtp_get_object_props_supported(s, c);
>           break;
>       case CMD_GET_OBJECT_PROP_DESC:
> -        if (c->argv[1] != FMT_UNDEFINED_OBJECT &&
> -            c->argv[1] != FMT_ASSOCIATION) {
> +        if (c->argc > 1 &&
> +           (c->argv[1] != FMT_UNDEFINED_OBJECT &&
> +            c->argv[1] != FMT_ASSOCIATION)) {
>               usb_mtp_queue_result(s, RES_INVALID_OBJECT_FORMAT_CODE,
>                                    c->trans, 0, 0, 0);
>               return;
>
diff mbox

Patch

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 6dfece9ea9..ad64495f05 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1134,7 +1134,7 @@  static void usb_mtp_command(MTPState *s, MTPControl *c)
                                  c->trans, 1, s->session, 0);
             return;
         }
-        if (c->argv[0] == 0) {
+        if (c->argc == 0 || c->argv[0] == 0) {
             usb_mtp_queue_result(s, RES_INVALID_PARAMETER,
                                  c->trans, 0, 0, 0);
             return;
@@ -1162,8 +1162,9 @@  static void usb_mtp_command(MTPState *s, MTPControl *c)
         data_in = usb_mtp_get_storage_ids(s, c);
         break;
     case CMD_GET_STORAGE_INFO:
-        if (c->argv[0] != QEMU_STORAGE_ID &&
-            c->argv[0] != 0xffffffff) {
+        if (c->argc == 0 ||
+           (c->argv[0] != QEMU_STORAGE_ID &&
+            c->argv[0] != 0xffffffff)) {
             usb_mtp_queue_result(s, RES_INVALID_STORAGE_ID,
                                  c->trans, 0, 0, 0);
             return;
@@ -1172,22 +1173,25 @@  static void usb_mtp_command(MTPState *s, MTPControl *c)
         break;
     case CMD_GET_NUM_OBJECTS:
     case CMD_GET_OBJECT_HANDLES:
-        if (c->argv[0] != QEMU_STORAGE_ID &&
-            c->argv[0] != 0xffffffff) {
+        if (c->argc == 0 ||
+           (c->argv[0] != QEMU_STORAGE_ID &&
+            c->argv[0] != 0xffffffff)) {
             usb_mtp_queue_result(s, RES_INVALID_STORAGE_ID,
                                  c->trans, 0, 0, 0);
             return;
         }
-        if (c->argv[1] != 0x00000000) {
+        if (c->argc > 1 && c->argv[1] != 0x00000000) {
             usb_mtp_queue_result(s, RES_SPEC_BY_FORMAT_UNSUPPORTED,
                                  c->trans, 0, 0, 0);
             return;
         }
-        if (c->argv[2] == 0x00000000 ||
-            c->argv[2] == 0xffffffff) {
-            o = QTAILQ_FIRST(&s->objects);
-        } else {
-            o = usb_mtp_object_lookup(s, c->argv[2]);
+        if (c->argc > 2) {
+            if (c->argv[2] == 0x00000000 ||
+                c->argv[2] == 0xffffffff) {
+                o = QTAILQ_FIRST(&s->objects);
+            } else {
+                o = usb_mtp_object_lookup(s, c->argv[2]);
+            }
         }
         if (o == NULL) {
             usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
@@ -1264,8 +1268,9 @@  static void usb_mtp_command(MTPState *s, MTPControl *c)
         res0 = data_in->length;
         break;
     case CMD_GET_OBJECT_PROPS_SUPPORTED:
-        if (c->argv[0] != FMT_UNDEFINED_OBJECT &&
-            c->argv[0] != FMT_ASSOCIATION) {
+        if (c->argc == 0 ||
+           (c->argv[0] != FMT_UNDEFINED_OBJECT &&
+            c->argv[0] != FMT_ASSOCIATION)) {
             usb_mtp_queue_result(s, RES_INVALID_OBJECT_FORMAT_CODE,
                                  c->trans, 0, 0, 0);
             return;
@@ -1273,8 +1278,9 @@  static void usb_mtp_command(MTPState *s, MTPControl *c)
         data_in = usb_mtp_get_object_props_supported(s, c);
         break;
     case CMD_GET_OBJECT_PROP_DESC:
-        if (c->argv[1] != FMT_UNDEFINED_OBJECT &&
-            c->argv[1] != FMT_ASSOCIATION) {
+        if (c->argc > 1 &&
+           (c->argv[1] != FMT_UNDEFINED_OBJECT &&
+            c->argv[1] != FMT_ASSOCIATION)) {
             usb_mtp_queue_result(s, RES_INVALID_OBJECT_FORMAT_CODE,
                                  c->trans, 0, 0, 0);
             return;