On 09/05/2017 10:51 PM, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1471225 > > Commit id '99a2d6af2' was a bit too aggressive with determining whether > the provided path was a "physical" cd-rom in order to generate a taint > message due to the possibility of some guest and host trying to control > the tray. For cd-rom guest devices backed to some VIR_STORAGE_TYPE_FILE > storage, this wouldn't be a problem and as such it shouldn't be a problem > for guest devices using some sort of block device on the host such as > iSCSI, LVM, or a Disk pool would present. > > Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> > --- > src/libvirt_private.syms | 1 + > src/qemu/qemu_domain.c | 2 +- > src/util/virfile.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > src/util/virfile.h | 4 ++++ > 4 files changed, 54 insertions(+), 1 deletion(-) > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index f30a04b..0354568 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -1680,6 +1680,7 @@ virFileGetMountSubtree; > virFileHasSuffix; > virFileInData; > virFileIsAbsPath; > +virFileIsCDROM; > virFileIsDir; > virFileIsExecutable; > virFileIsLink; > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 9cff501..426c577 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -4807,7 +4807,7 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver, > > if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM && > virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_BLOCK && > - disk->src->path) > + disk->src->path && virFileIsCDROM(disk->src->path)) > qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_CDROM_PASSTHROUGH, > logCtxt); > > diff --git a/src/util/virfile.c b/src/util/virfile.c > index 2f28e83..4c31949 100644 > --- a/src/util/virfile.c > +++ b/src/util/virfile.c > @@ -4166,3 +4166,51 @@ virFileReadValueString(char **value, const char *format, ...) > VIR_FREE(str); > return ret; > } > + > + > +#if defined(__linux__) > + > +/* virFileIsCDROM > + * @path: Supplied path. > + * > + * Determine if the path is a CD-ROM path. Typically on Linux systems this > + * is either /dev/cdrom or /dev/sr0, so those are easy checks. Still if > + * someone is trying to be tricky, we can resolve the link to /dev/cdrom > + * and compare it to the resolved link of the supplied @path to compare > + * if they're the same. > + * > + * Returns true if the path is a CDROM, false otherwise. > + */ > +bool > +virFileIsCDROM(const char *path) > +{ > + bool ret = false; > + char *linkpath = NULL; > + char *cdrompath = NULL; > + > + if (STREQ(path, "/dev/cdrom") || STREQ(path, "/dev/sr0")) > + return true; What if I have two CDROMs? /dev/sr0 and /dev/sr1? I'm worried that name match is not sufficient and we need to lstat() and check if the major number (st.st_rdev) is 11 (0xb) (according to Documentation/admin-guide/devices.txt from kernel sources). And even that might be not enough :( > + > + if (virFileResolveLink(path, &linkpath) < 0 || > + virFileResolveLink("/dev/cdrom", &cdrompath) < 0) > + goto cleanup; This will only work for cases where /dev/cdrom points to the device in @path. However, if /dev/cdrom is pointing to /dev/sr0 and I'm calling this function over /dev/sr1 (because I have a machine with dozens of CDROMs) this function returns false which is obviously wrong. Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
