Re: [Qemu-devel] [PATCH] ssh: Don't crash if either host or path is not specified.

[Gonglei (Arei)]

> Subject: [Qemu-devel] [PATCH] ssh: Don't crash if either host or path is not
> specified.
> 
> $ ./qemu-img create -f qcow2 overlay \
>     -b 'json: { "file.driver":"ssh",
>                 "file.host":"localhost",
>                 "file.host_key_check":"no" }'
> qemu-img: qobject/qdict.c:193: qdict_get_obj: Assertion `obj != ((void *)0)'
> failed.
> Aborted
> 
> A similar crash also happens if the file.host field is omitted.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1147343
> 
> Bug found and reported by Jun Li.
> 
> Signed-off-by: Richard W.M. Jones <address@hidden>
> ---
>  block/ssh.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/block/ssh.c b/block/ssh.c
> index cd2fd75..54009c0 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -517,6 +517,11 @@ static int connect_to_ssh(BDRVSSHState *s, QDict
> *options,
>      const char *host, *user, *path, *host_key_check;
>      int port;
> 
> +    if (!qdict_haskey(options, "host")) {
> +        ret = -EINVAL;
> +        error_setg_errno(errp, errno, "No hostname was specified");

I don't think errno is useful (will be a random value) here.
You just need use error_setg(), and return ret directly. :)

> +        goto err;
> +    }
>      host = qdict_get_str(options, "host");
> 
>      if (qdict_haskey(options, "port")) {
> @@ -525,6 +530,11 @@ static int connect_to_ssh(BDRVSSHState *s, QDict
> *options,
>          port = 22;
>      }
> 
> +    if (!qdict_haskey(options, "path")) {
> +        ret = -EINVAL;
> +        error_setg_errno(errp, errno, "No path was specified");

Here, too.

> +        goto err;
> +    }
>      path = qdict_get_str(options, "path");
> 
>      if (qdict_haskey(options, "user")) {
> --
> 2.0.4
> 

BTW, you'd better CC corresponding maintainer in the next version.
You can get the maintainer by "./script/get_maintainer.pl" script.

Best regards,
-Gonglei