[libvirt] [PATCH] qemu: Be more selective when determining cdrom for taint messaging
Michal Privoznik
mprivozn at redhat.com
Wed Sep 6 12:17:34 UTC 2017
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 at redhat.com>
> ---
> 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
More information about the libvir-list
mailing list