[PATCH-for-5.0] ui/input-linux: Do not ignore ioctl() return value

Philippe Mathieu-Daudé posted 1 patch 4 years ago
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test checkpatch passed
Test FreeBSD passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200322161219.17757-1-philmd@redhat.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
ui/input-linux.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
[PATCH-for-5.0] ui/input-linux: Do not ignore ioctl() return value
Posted by Philippe Mathieu-Daudé 4 years ago
Fix warnings reported by Clang static code analyzer:

    CC      ui/input-linux.o
      ui/input-linux.c:343:9: warning: Value stored to 'rc' is never read
          rc = ioctl(il->fd, EVIOCGBIT(EV_REL, sizeof(relmap)), &relmap);
          ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      ui/input-linux.c:351:9: warning: Value stored to 'rc' is never read
          rc = ioctl(il->fd, EVIOCGBIT(EV_ABS, sizeof(absmap)), &absmap);
          ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      ui/input-linux.c:354:13: warning: Value stored to 'rc' is never read
              rc = ioctl(il->fd, EVIOCGABS(ABS_X), &absinfo);
              ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      ui/input-linux.c:357:13: warning: Value stored to 'rc' is never read
              rc = ioctl(il->fd, EVIOCGABS(ABS_Y), &absinfo);
              ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      ui/input-linux.c:365:9: warning: Value stored to 'rc' is never read
          rc = ioctl(il->fd, EVIOCGBIT(EV_KEY, sizeof(keymap)), keymap);
          ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      ui/input-linux.c:366:9: warning: Value stored to 'rc' is never read
          rc = ioctl(il->fd, EVIOCGKEY(sizeof(keystate)), keystate);
          ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 ui/input-linux.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/ui/input-linux.c b/ui/input-linux.c
index a7b280b25b..ef37b14d6f 100644
--- a/ui/input-linux.c
+++ b/ui/input-linux.c
@@ -334,13 +334,15 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
 
     rc = ioctl(il->fd, EVIOCGBIT(0, sizeof(evtmap)), &evtmap);
     if (rc < 0) {
-        error_setg(errp, "%s: failed to read event bits", il->evdev);
-        goto err_close;
+        goto err_read_event_bits;
     }
 
     if (evtmap & (1 << EV_REL)) {
         relmap = 0;
         rc = ioctl(il->fd, EVIOCGBIT(EV_REL, sizeof(relmap)), &relmap);
+        if (rc < 0) {
+            goto err_read_event_bits;
+        }
         if (relmap & (1 << REL_X)) {
             il->has_rel_x = true;
         }
@@ -349,12 +351,25 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
     if (evtmap & (1 << EV_ABS)) {
         absmap = 0;
         rc = ioctl(il->fd, EVIOCGBIT(EV_ABS, sizeof(absmap)), &absmap);
+        if (rc < 0) {
+            goto err_read_event_bits;
+        }
         if (absmap & (1 << ABS_X)) {
             il->has_abs_x = true;
             rc = ioctl(il->fd, EVIOCGABS(ABS_X), &absinfo);
+            if (rc < 0) {
+                error_setg(errp, "%s: failed to get get absolute X value",
+                           il->evdev);
+                goto err_close;
+            }
             il->abs_x_min = absinfo.minimum;
             il->abs_x_max = absinfo.maximum;
             rc = ioctl(il->fd, EVIOCGABS(ABS_Y), &absinfo);
+            if (rc < 0) {
+                error_setg(errp, "%s: failed to get get absolute Y value",
+                           il->evdev);
+                goto err_close;
+            }
             il->abs_y_min = absinfo.minimum;
             il->abs_y_max = absinfo.maximum;
         }
@@ -363,7 +378,14 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
     if (evtmap & (1 << EV_KEY)) {
         memset(keymap, 0, sizeof(keymap));
         rc = ioctl(il->fd, EVIOCGBIT(EV_KEY, sizeof(keymap)), keymap);
+        if (rc < 0) {
+            goto err_read_event_bits;
+        }
         rc = ioctl(il->fd, EVIOCGKEY(sizeof(keystate)), keystate);
+        if (rc < 0) {
+            error_setg(errp, "%s: failed to get global key state", il->evdev);
+            goto err_close;
+        }
         for (i = 0; i < KEY_CNT; i++) {
             if (keymap[i / 8] & (1 << (i % 8))) {
                 if (linux_is_button(i)) {
@@ -390,6 +412,9 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
     il->initialized = true;
     return;
 
+err_read_event_bits:
+    error_setg(errp, "%s: failed to read event bits", il->evdev);
+
 err_close:
     close(il->fd);
     return;
-- 
2.21.1


Re: [PATCH-for-5.0] ui/input-linux: Do not ignore ioctl() return value
Posted by Darren Kenny 4 years ago
Philippe Mathieu-Daudé writes:

> Fix warnings reported by Clang static code analyzer:
>
>     CC      ui/input-linux.o
>       ui/input-linux.c:343:9: warning: Value stored to 'rc' is never read
>           rc = ioctl(il->fd, EVIOCGBIT(EV_REL, sizeof(relmap)), &relmap);
>           ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       ui/input-linux.c:351:9: warning: Value stored to 'rc' is never read
>           rc = ioctl(il->fd, EVIOCGBIT(EV_ABS, sizeof(absmap)), &absmap);
>           ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       ui/input-linux.c:354:13: warning: Value stored to 'rc' is never read
>               rc = ioctl(il->fd, EVIOCGABS(ABS_X), &absinfo);
>               ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       ui/input-linux.c:357:13: warning: Value stored to 'rc' is never read
>               rc = ioctl(il->fd, EVIOCGABS(ABS_Y), &absinfo);
>               ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       ui/input-linux.c:365:9: warning: Value stored to 'rc' is never read
>           rc = ioctl(il->fd, EVIOCGBIT(EV_KEY, sizeof(keymap)), keymap);
>           ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       ui/input-linux.c:366:9: warning: Value stored to 'rc' is never read
>           rc = ioctl(il->fd, EVIOCGKEY(sizeof(keystate)), keystate);
>           ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

> ---
>  ui/input-linux.c | 29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/ui/input-linux.c b/ui/input-linux.c
> index a7b280b25b..ef37b14d6f 100644
> --- a/ui/input-linux.c
> +++ b/ui/input-linux.c
> @@ -334,13 +334,15 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
>  
>      rc = ioctl(il->fd, EVIOCGBIT(0, sizeof(evtmap)), &evtmap);
>      if (rc < 0) {
> -        error_setg(errp, "%s: failed to read event bits", il->evdev);
> -        goto err_close;
> +        goto err_read_event_bits;
>      }
>  
>      if (evtmap & (1 << EV_REL)) {
>          relmap = 0;
>          rc = ioctl(il->fd, EVIOCGBIT(EV_REL, sizeof(relmap)), &relmap);
> +        if (rc < 0) {
> +            goto err_read_event_bits;
> +        }
>          if (relmap & (1 << REL_X)) {
>              il->has_rel_x = true;
>          }
> @@ -349,12 +351,25 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
>      if (evtmap & (1 << EV_ABS)) {
>          absmap = 0;
>          rc = ioctl(il->fd, EVIOCGBIT(EV_ABS, sizeof(absmap)), &absmap);
> +        if (rc < 0) {
> +            goto err_read_event_bits;
> +        }
>          if (absmap & (1 << ABS_X)) {
>              il->has_abs_x = true;
>              rc = ioctl(il->fd, EVIOCGABS(ABS_X), &absinfo);
> +            if (rc < 0) {
> +                error_setg(errp, "%s: failed to get get absolute X value",
> +                           il->evdev);
> +                goto err_close;
> +            }
>              il->abs_x_min = absinfo.minimum;
>              il->abs_x_max = absinfo.maximum;
>              rc = ioctl(il->fd, EVIOCGABS(ABS_Y), &absinfo);
> +            if (rc < 0) {
> +                error_setg(errp, "%s: failed to get get absolute Y value",
> +                           il->evdev);
> +                goto err_close;
> +            }
>              il->abs_y_min = absinfo.minimum;
>              il->abs_y_max = absinfo.maximum;
>          }
> @@ -363,7 +378,14 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
>      if (evtmap & (1 << EV_KEY)) {
>          memset(keymap, 0, sizeof(keymap));
>          rc = ioctl(il->fd, EVIOCGBIT(EV_KEY, sizeof(keymap)), keymap);
> +        if (rc < 0) {
> +            goto err_read_event_bits;
> +        }
>          rc = ioctl(il->fd, EVIOCGKEY(sizeof(keystate)), keystate);
> +        if (rc < 0) {
> +            error_setg(errp, "%s: failed to get global key state", il->evdev);
> +            goto err_close;
> +        }
>          for (i = 0; i < KEY_CNT; i++) {
>              if (keymap[i / 8] & (1 << (i % 8))) {
>                  if (linux_is_button(i)) {
> @@ -390,6 +412,9 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
>      il->initialized = true;
>      return;
>  
> +err_read_event_bits:
> +    error_setg(errp, "%s: failed to read event bits", il->evdev);
> +
>  err_close:
>      close(il->fd);
>      return;


Re: [PATCH-for-5.0] ui/input-linux: Do not ignore ioctl() return value
Posted by Philippe Mathieu-Daudé 4 years ago
On 3/22/20 5:12 PM, Philippe Mathieu-Daudé wrote:
> Fix warnings reported by Clang static code analyzer:
> 
>      CC      ui/input-linux.o
>        ui/input-linux.c:343:9: warning: Value stored to 'rc' is never read
>            rc = ioctl(il->fd, EVIOCGBIT(EV_REL, sizeof(relmap)), &relmap);
>            ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>        ui/input-linux.c:351:9: warning: Value stored to 'rc' is never read
>            rc = ioctl(il->fd, EVIOCGBIT(EV_ABS, sizeof(absmap)), &absmap);
>            ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>        ui/input-linux.c:354:13: warning: Value stored to 'rc' is never read
>                rc = ioctl(il->fd, EVIOCGABS(ABS_X), &absinfo);
>                ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>        ui/input-linux.c:357:13: warning: Value stored to 'rc' is never read
>                rc = ioctl(il->fd, EVIOCGABS(ABS_Y), &absinfo);
>                ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>        ui/input-linux.c:365:9: warning: Value stored to 'rc' is never read
>            rc = ioctl(il->fd, EVIOCGBIT(EV_KEY, sizeof(keymap)), keymap);
>            ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>        ui/input-linux.c:366:9: warning: Value stored to 'rc' is never read
>            rc = ioctl(il->fd, EVIOCGKEY(sizeof(keystate)), keystate);
>            ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   ui/input-linux.c | 29 +++++++++++++++++++++++++++--
>   1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/ui/input-linux.c b/ui/input-linux.c
> index a7b280b25b..ef37b14d6f 100644
> --- a/ui/input-linux.c
> +++ b/ui/input-linux.c
> @@ -334,13 +334,15 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
>   
>       rc = ioctl(il->fd, EVIOCGBIT(0, sizeof(evtmap)), &evtmap);
>       if (rc < 0) {
> -        error_setg(errp, "%s: failed to read event bits", il->evdev);
> -        goto err_close;
> +        goto err_read_event_bits;
>       }
>   
>       if (evtmap & (1 << EV_REL)) {
>           relmap = 0;
>           rc = ioctl(il->fd, EVIOCGBIT(EV_REL, sizeof(relmap)), &relmap);
> +        if (rc < 0) {
> +            goto err_read_event_bits;
> +        }
>           if (relmap & (1 << REL_X)) {
>               il->has_rel_x = true;
>           }
> @@ -349,12 +351,25 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
>       if (evtmap & (1 << EV_ABS)) {
>           absmap = 0;
>           rc = ioctl(il->fd, EVIOCGBIT(EV_ABS, sizeof(absmap)), &absmap);
> +        if (rc < 0) {
> +            goto err_read_event_bits;
> +        }
>           if (absmap & (1 << ABS_X)) {
>               il->has_abs_x = true;
>               rc = ioctl(il->fd, EVIOCGABS(ABS_X), &absinfo);
> +            if (rc < 0) {
> +                error_setg(errp, "%s: failed to get get absolute X value",
> +                           il->evdev);
> +                goto err_close;
> +            }
>               il->abs_x_min = absinfo.minimum;
>               il->abs_x_max = absinfo.maximum;
>               rc = ioctl(il->fd, EVIOCGABS(ABS_Y), &absinfo);
> +            if (rc < 0) {
> +                error_setg(errp, "%s: failed to get get absolute Y value",
> +                           il->evdev);
> +                goto err_close;
> +            }
>               il->abs_y_min = absinfo.minimum;
>               il->abs_y_max = absinfo.maximum;
>           }
> @@ -363,7 +378,14 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
>       if (evtmap & (1 << EV_KEY)) {
>           memset(keymap, 0, sizeof(keymap));
>           rc = ioctl(il->fd, EVIOCGBIT(EV_KEY, sizeof(keymap)), keymap);
> +        if (rc < 0) {
> +            goto err_read_event_bits;
> +        }
>           rc = ioctl(il->fd, EVIOCGKEY(sizeof(keystate)), keystate);
> +        if (rc < 0) {
> +            error_setg(errp, "%s: failed to get global key state", il->evdev);
> +            goto err_close;
> +        }
>           for (i = 0; i < KEY_CNT; i++) {
>               if (keymap[i / 8] & (1 << (i % 8))) {
>                   if (linux_is_button(i)) {
> @@ -390,6 +412,9 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
>       il->initialized = true;
>       return;
>   
> +err_read_event_bits:
> +    error_setg(errp, "%s: failed to read event bits", il->evdev);
> +
>   err_close:
>       close(il->fd);
>       return;
> 

I see this patch as been merged as commit 112c37a6a6e4, thanks!