Re: [Qemu-devel] [PATCH 1/1] block/nbd: fix segmentation fault when .desc is not null-terminated

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1821 bytes --]

On 01/05/2018 07:32 AM, Murilo Opsfelder Araujo wrote:
> The find_desc_by_name() from util/qemu-option.c relies on the .name not being
> NULL to call strcmp(). This check becomes unsafe when the list is not
> NULL-terminated, which is the case of nbd_runtime_opts in block/nbd.c, and can
> result in segmentation fault when strcmp() tries to access an invalid memory:

Thanks for the report and patch.  Adding qemu-stable in cc.

> 
> This patch fixes the segmentation fault in strcmp() by adding a NULL element at
> the end of nbd_runtime_opts.desc list, which is the common practice to most of
> other structs like runtime_opts in block/null.c. Thus, the desc[i].name != NULL
> check becomes safe because it will not evaluate to true when .desc list reached
> its end.
> 
> Reported-by: R. Nageswara Sastry <nasastry@in.ibm.com>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1727259
> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.vnet.ibm.com>

I'll update the commit message to add in the commit id that introduced
the problem, as well as check that other QemuOptsList do not have a
similar problem; I'm queueing this on the NBD tree and will submit a
pull request soon.

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

> ---
>  block/nbd.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/block/nbd.c b/block/nbd.c
> index a50d24b50a..8b8ba56cdd 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -388,6 +388,7 @@ static QemuOptsList nbd_runtime_opts = {
>              .type = QEMU_OPT_STRING,
>              .help = "ID of the TLS credentials to use",
>          },
> +        { /* end of list */ }
>      },
>  };
>  
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]