Re: ioemu: Re-enable guest boot with blktap devices ( Weekly VMX status report. Xen: #18577 & Xen0: #696)

[Yosuke Iwamatsu <y-iwamatsu@ab.jp.nec.com>]

Ian Jackson wrote:
> Yosuke Iwamatsu writes ("ioemu: Re-enable guest boot with blktap devices ([Xen-devel] Weekly VMX status report. Xen: #18577 & Xen0: #696)"):
>> Bug #1367 looks caused by the xenstore path checking code recently added
>> to ioemu-remote. I saw the same problem and found that the code didn't
>> consider the case blktap devices were attached to the guest. The patch
>> below should avoid the problem.
> 
> Thanks for the report.  Could you try the attached patch instead and
> let me know whether it fixes the problem ?

It looks fine to me.

-- Yosuke

> 
> Keir Fraser writes ("Re: ioemu: Re-enable guest boot with blktap devices ([Xen-devel] Weekly VMX status report. Xen: #18577 & Xen0: #696)"):
>> Alternatively could just check a shorter path prefix (just /local/domain/0/
>> would be sufficient I think). Full path checking is obviously inherently a
>> bit more fragile.
> 
> Well, yes, but this is a security check and I think those are really
> supposed to be brittle.  So I'd prefer to make us check that the path
> is definitely completely of an expected form.
> 
> Ian.
> 
> commit 629adb3f5244169731ff18b16ae919641d81ad76
> Author: Ian Jackson <ian.jackson@eu.citrix.com>
> Date:   Tue Oct 14 11:46:53 2008 +0100
> 
>     Fix blktap device backend patch check
>     
>     Regarding http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=1367,
>     it appears that the expected backend path check is too strict for'
>     blktap devices.  Therefore if the devtype is `vbd' we allow the
>     backend to be `tap'.
>     
>     Thanks to report and inspiration from Yosuke Iwamatsu.
>     
>     Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> diff --git a/xenstore.c b/xenstore.c
> index f5aa8a7..5cd5063 100644
> --- a/xenstore.c
> +++ b/xenstore.c
> @@ -191,17 +191,36 @@ static void xenstore_get_backend_path(char **backend, const char *devtype,
>      backend_dompath = xs_get_domain_path(xsh, domid_backend);
>      if (!backend_dompath) goto out;
>      
> -    if (pasprintf(&expected_backend, "%s/backend/%s/%lu/%s",
> -                  backend_dompath, devtype, frontend_domid, inst_danger)
> -        == -1) goto out;
> +    const char *expected_devtypes[3];
> +    const char **expected_devtype = expected_devtypes;
> +
> +    *expected_devtype++ = devtype;
> +    if (!strcmp(devtype, "vbd")) *expected_devtype++ = "tap";
> +    *expected_devtype = 0;
> +    assert(expected_devtype <
> +           expected_devtypes + ARRAY_SIZE(expected_devtypes));
> +
> +    for (expected_devtype = expected_devtypes;
> +         *expected_devtype;
> +         expected_devtype++) {
> +    
> +        if (pasprintf(&expected_backend, "%s/backend/%s/%lu/%s",
> +                      backend_dompath, *expected_devtype,
> +                      frontend_domid, inst_danger)
> +            == -1) goto out;
>  
> -    if (strcmp(bpath, expected_backend)) {
> -        fprintf(stderr, "frontend `%s' expected backend `%s' got `%s',"
> -                " ignoring\n", frontend_path, expected_backend, bpath);
> -        errno = EINVAL;
> -        goto out;
> +        if (!strcmp(bpath, expected_backend))
> +            goto found;
>      }
>  
> +    fprintf(stderr, "frontend `%s' devtype `%s' expected backend `%s'"
> +            " got `%s', ignoring\n",
> +            frontend_path, devtype, expected_backend, bpath);
> +    errno = EINVAL;
> +    goto out;
> +
> + found:
> +
>      if (pasprintf(&backend_frontend_path, "%s/frontend", bpath)
>          == -1) goto out;
>