From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yosuke Iwamatsu Subject: Re: ioemu: Re-enable guest boot with blktap devices ( Weekly VMX status report. Xen: #18577 & Xen0: #696) Date: Wed, 15 Oct 2008 08:57:58 +0900 Message-ID: <48F53206.70207@ab.jp.nec.com> References: <48F41A4D.1020608@ab.jp.nec.com> <76780B19A496DC4B80439008DAD7076C01ACDF710F@PDSMSX501.ccr.corp.intel.com> <18676.31008.919860.890968@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <18676.31008.919860.890968@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Ian Jackson Cc: haicheng.li@intel.com, xen-devel@lists.xensource.com, Keir Fraser List-Id: xen-devel@lists.xenproject.org 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 > 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 > > 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; >