qemu-io-cmds: assert that we don't have .perm requested in no-blk case

[PATCH] qemu-io-cmds: assert that we don't have .perm requested in no-blk case

Posted by Vladimir Sementsov-Ogievskiy 2 years, 11 months ago

Coverity things blk may be NULL. It's a false-positive, as described in
a new comment.

Fixes: Coverity CID 1453194
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qemu-io-cmds.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 998b67186d..3b7cceecbf 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -92,9 +92,19 @@ static int command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
         return -EINVAL;
     }
 
-    /* Request additional permissions if necessary for this command. The caller
+    /*
+     * Request additional permissions if necessary for this command. The caller
      * is responsible for restoring the original permissions afterwards if this
-     * is what it wants. */
+     * is what it wants.
+     *
+     * Coverity things that blk may be NULL in the following if condition. It's
+     * not so: in init_check_command() we fail if blk is NULL for command with
+     * both CMD_FLAG_GLOBAL and CMD_NOFILE_OK flags unset. And in
+     * qemuio_add_command() we assert that command with non-zero .perm field
+     * doesn't set this flags. So, the following assertion is to silence
+     * Coverity:
+     */
+    assert(blk || !ct->perm);
     if (ct->perm && blk_is_available(blk)) {
         uint64_t orig_perm, orig_shared_perm;
         blk_get_perm(blk, &orig_perm, &orig_shared_perm);
-- 
2.29.2

Re: [PATCH] qemu-io-cmds: assert that we don't have .perm requested in no-blk case

Posted by Kevin Wolf 2 years, 11 months ago

Am 19.05.2021 um 11:05 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Coverity things blk may be NULL. It's a false-positive, as described in

s/things/thinks/

> a new comment.
> 
> Fixes: Coverity CID 1453194
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  qemu-io-cmds.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 998b67186d..3b7cceecbf 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -92,9 +92,19 @@ static int command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
>          return -EINVAL;
>      }
>  
> -    /* Request additional permissions if necessary for this command. The caller
> +    /*
> +     * Request additional permissions if necessary for this command. The caller
>       * is responsible for restoring the original permissions afterwards if this
> -     * is what it wants. */
> +     * is what it wants.
> +     *
> +     * Coverity things that blk may be NULL in the following if condition. It's

And here.

> +     * not so: in init_check_command() we fail if blk is NULL for command with
> +     * both CMD_FLAG_GLOBAL and CMD_NOFILE_OK flags unset. And in
> +     * qemuio_add_command() we assert that command with non-zero .perm field
> +     * doesn't set this flags. So, the following assertion is to silence
> +     * Coverity:
> +     */
> +    assert(blk || !ct->perm);
>      if (ct->perm && blk_is_available(blk)) {
>          uint64_t orig_perm, orig_shared_perm;
>          blk_get_perm(blk, &orig_perm, &orig_shared_perm);

Thanks, applied with the typo fixed up.

Kevin