diff mbox

[2/3] blockdev: Fix NULL pointer dereference in blockdev-add

Message ID 1394120669-4675-3-git-send-email-kwolf@redhat.com
State New
Headers show

Commit Message

Kevin Wolf March 6, 2014, 3:44 p.m. UTC
If aio=native, we check that cache.direct is set as well. If however
cache wasn't specified at all, qemu just segfaulted.

The old condition didn't make any sense anyway because it effectively
only checked for the default cache mode case, but not for an explicitly
set cache.direct=off mode.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Benoît Canet March 6, 2014, 4:06 p.m. UTC | #1
The Thursday 06 Mar 2014 à 16:44:28 (+0100), Kevin Wolf wrote :
> If aio=native, we check that cache.direct is set as well. If however
> cache wasn't specified at all, qemu just segfaulted.
> 
> The old condition didn't make any sense anyway because it effectively
> only checked for the default cache mode case, but not for an explicitly
> set cache.direct=off mode.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 561cb81..c3422a1 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2283,8 +2283,10 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
>       *
>       * For now, simply forbidding the combination for all drivers will do. */
>      if (options->has_aio && options->aio == BLOCKDEV_AIO_OPTIONS_NATIVE) {
> -        bool direct = options->cache->has_direct && options->cache->direct;
> -        if (!options->has_cache && !direct) {
> +        bool direct = options->has_cache &&
> +                      options->cache->has_direct &&
> +                      options->cache->direct;
> +        if (!direct) {
>              error_setg(errp, "aio=native requires cache.direct=true");
>              goto fail;
>          }
> -- 
> 1.8.1.4
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Eric Blake March 6, 2014, 4:22 p.m. UTC | #2
On 03/06/2014 08:44 AM, Kevin Wolf wrote:
> If aio=native, we check that cache.direct is set as well. If however
> cache wasn't specified at all, qemu just segfaulted.
> 
> The old condition didn't make any sense anyway because it effectively
> only checked for the default cache mode case, but not for an explicitly
> set cache.direct=off mode.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox

Patch

diff --git a/blockdev.c b/blockdev.c
index 561cb81..c3422a1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2283,8 +2283,10 @@  void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
      *
      * For now, simply forbidding the combination for all drivers will do. */
     if (options->has_aio && options->aio == BLOCKDEV_AIO_OPTIONS_NATIVE) {
-        bool direct = options->cache->has_direct && options->cache->direct;
-        if (!options->has_cache && !direct) {
+        bool direct = options->has_cache &&
+                      options->cache->has_direct &&
+                      options->cache->direct;
+        if (!direct) {
             error_setg(errp, "aio=native requires cache.direct=true");
             goto fail;
         }