[libvirt] [PATCH] qemu: Put format=raw onto cmd line for SCSI passthrough
https://bugzilla.redhat.com/show_bug.cgi?id=1632833 When doing a SCSI passthrough we don't put format= onto the command line. This causes qemu to probe the format automatically which ends up in a warning in the domain log and possible qemu disabling writes to the first block (according to the warning message). Based-on-work-of: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 2 +- tests/qemuxml2argvdata/hostdev-scsi-lsi.args | 2 +- tests/qemuxml2argvdata/hostdev-scsi-readonly.args | 2 +- tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.args | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 269276f2f9..1ff593c657 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4841,7 +4841,7 @@ qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, } else { if (!(source = qemuBuildSCSIHostHostdevDrvStr(dev))) goto error; - virBufferAsprintf(&buf, "file=/dev/%s,if=none", source); + virBufferAsprintf(&buf, "file=/dev/%s,if=none,format=raw", source); } VIR_FREE(source); diff --git a/tests/qemuxml2argvdata/hostdev-scsi-lsi.args b/tests/qemuxml2argvdata/hostdev-scsi-lsi.args index d05e2a8bf8..f2048fe920 100644 --- a/tests/qemuxml2argvdata/hostdev-scsi-lsi.args +++ b/tests/qemuxml2argvdata/hostdev-scsi-lsi.args @@ -25,6 +25,6 @@ server,nowait \ -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,\ bootindex=1 \ --drive file=/dev/sg0,if=none,id=drive-hostdev0 \ +-drive file=/dev/sg0,if=none,format=raw,id=drive-hostdev0 \ -device scsi-generic,bus=scsi0.0,scsi-id=7,drive=drive-hostdev0,id=hostdev0 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/hostdev-scsi-readonly.args b/tests/qemuxml2argvdata/hostdev-scsi-readonly.args index c6336ca441..0d5a0d327d 100644 --- a/tests/qemuxml2argvdata/hostdev-scsi-readonly.args +++ b/tests/qemuxml2argvdata/hostdev-scsi-readonly.args @@ -25,7 +25,7 @@ server,nowait \ -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,\ bootindex=1 \ --drive file=/dev/sg0,if=none,id=drive-hostdev0,readonly=on \ +-drive file=/dev/sg0,if=none,format=raw,id=drive-hostdev0,readonly=on \ -device scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=8,\ drive=drive-hostdev0,id=hostdev0 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.args b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.args index 4bf4ce7f82..13a1e9fe95 100644 --- a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.args +++ b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.args @@ -25,7 +25,7 @@ server,nowait \ -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,\ bootindex=1 \ --drive file=/dev/sg0,if=none,id=drive-hostdev0 \ +-drive file=/dev/sg0,if=none,format=raw,id=drive-hostdev0 \ -device scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=8,\ drive=drive-hostdev0,id=hostdev0 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 -- 2.18.1
On Fri, Oct 12, 2018 at 01:14:51PM +0200, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1632833
When doing a SCSI passthrough we don't put format= onto the command line. This causes qemu to probe the format automatically which ends up in a warning in the domain log and possible qemu disabling writes to the first block (according to the warning message).
If the warning message is correct, this should have been reported as a security bug to libvirt and given a CVE. On the other hand if the warning from QEMU isn't correct, then QEMU shouldn't have printed the warning about it being dangerous. So something is missing here either way.
Based-on-work-of: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 2 +- tests/qemuxml2argvdata/hostdev-scsi-lsi.args | 2 +- tests/qemuxml2argvdata/hostdev-scsi-readonly.args | 2 +- tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.args | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 269276f2f9..1ff593c657 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4841,7 +4841,7 @@ qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, } else { if (!(source = qemuBuildSCSIHostHostdevDrvStr(dev))) goto error; - virBufferAsprintf(&buf, "file=/dev/%s,if=none", source); + virBufferAsprintf(&buf, "file=/dev/%s,if=none,format=raw", source); } VIR_FREE(source);
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-lsi.args b/tests/qemuxml2argvdata/hostdev-scsi-lsi.args index d05e2a8bf8..f2048fe920 100644 --- a/tests/qemuxml2argvdata/hostdev-scsi-lsi.args +++ b/tests/qemuxml2argvdata/hostdev-scsi-lsi.args @@ -25,6 +25,6 @@ server,nowait \ -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,\ bootindex=1 \ --drive file=/dev/sg0,if=none,id=drive-hostdev0 \ +-drive file=/dev/sg0,if=none,format=raw,id=drive-hostdev0 \ -device scsi-generic,bus=scsi0.0,scsi-id=7,drive=drive-hostdev0,id=hostdev0 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/hostdev-scsi-readonly.args b/tests/qemuxml2argvdata/hostdev-scsi-readonly.args index c6336ca441..0d5a0d327d 100644 --- a/tests/qemuxml2argvdata/hostdev-scsi-readonly.args +++ b/tests/qemuxml2argvdata/hostdev-scsi-readonly.args @@ -25,7 +25,7 @@ server,nowait \ -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,\ bootindex=1 \ --drive file=/dev/sg0,if=none,id=drive-hostdev0,readonly=on \ +-drive file=/dev/sg0,if=none,format=raw,id=drive-hostdev0,readonly=on \ -device scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=8,\ drive=drive-hostdev0,id=hostdev0 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.args b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.args index 4bf4ce7f82..13a1e9fe95 100644 --- a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.args +++ b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.args @@ -25,7 +25,7 @@ server,nowait \ -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,\ bootindex=1 \ --drive file=/dev/sg0,if=none,id=drive-hostdev0 \ +-drive file=/dev/sg0,if=none,format=raw,id=drive-hostdev0 \ -device scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=8,\ drive=drive-hostdev0,id=hostdev0 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 -- 2.18.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 10/12/2018 02:17 PM, Daniel P. Berrangé wrote:
On Fri, Oct 12, 2018 at 01:14:51PM +0200, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1632833
When doing a SCSI passthrough we don't put format= onto the command line. This causes qemu to probe the format automatically which ends up in a warning in the domain log and possible qemu disabling writes to the first block (according to the warning message).
If the warning message is correct, this should have been reported as a security bug to libvirt and given a CVE.
Why is that? It the message is correct, qemu would prevent from writing to the first block. No harm there.
On the other hand if the warning from QEMU isn't correct, then QEMU shouldn't have printed the warning about it being dangerous.
In my testing I was able to write to the first block. Therefore, IMO qemu is throwing incorrect warning message.
So something is missing here either way.
Sure, but that doesn't invalidate my patch, does it? Michal
On Fri, Oct 12, 2018 at 02:27:26PM +0200, Michal Privoznik wrote:
On 10/12/2018 02:17 PM, Daniel P. Berrangé wrote:
On Fri, Oct 12, 2018 at 01:14:51PM +0200, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1632833
When doing a SCSI passthrough we don't put format= onto the command line. This causes qemu to probe the format automatically which ends up in a warning in the domain log and possible qemu disabling writes to the first block (according to the warning message).
If the warning message is correct, this should have been reported as a security bug to libvirt and given a CVE.
Why is that? It the message is correct, qemu would prevent from writing to the first block. No harm there.
Only QEMU >= 2.3.0 has that protection, so this is not something we can rely to avoid calling it a CVE. It just means distros when QEMU >=2.3.0 would not be affected by the CVE.
On the other hand if the warning from QEMU isn't correct, then QEMU shouldn't have printed the warning about it being dangerous.
In my testing I was able to write to the first block. Therefore, IMO qemu is throwing incorrect warning message.
So something is missing here either way.
Sure, but that doesn't invalidate my patch, does it?
Only the commit message - if this is a security flaw, we must be more explicit about it in the commit. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Fri, Oct 12, 2018 at 01:17:42PM +0100, Daniel P. Berrangé wrote:
On Fri, Oct 12, 2018 at 01:14:51PM +0200, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1632833
When doing a SCSI passthrough we don't put format= onto the command line. This causes qemu to probe the format automatically which ends up in a warning in the domain log and possible qemu disabling writes to the first block (according to the warning message).
If the warning message is correct, this should have been reported as a security bug to libvirt and given a CVE.
On the other hand if the warning from QEMU isn't correct, then QEMU shouldn't have printed the warning about it being dangerous.
So something is missing here either way.
I used 'modprobe scsi_debug' to create a fake SCSI device which appears as "scsi_host6" in my host OS, and has a /dev/sg3 and /dev/sdd device nodes. When I use this XML: <hostdev mode='subsystem' type='scsi' managed='no' sgio='filtered' rawio='yes'> <source> <adapter name='scsi_host6'/> <address bus='0' target='0' unit='0'/> </source> <alias name='hostdev0'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </hostdev> libvirt spawns QEMU pointing to /dev/sg3 -drive file=/dev/sg3,if=none,id=drive-hostdev0 Inside the guest, I can successfully run qemu-img create -f qcow2 /dev/sda 4M and on the host OS this now appears visible in the host # qemu-img info /dev/sdd image: /dev/sdd file format: qcow2 virtual size: 6.0M (6291456 bytes) disk size: 0 cluster_size: 65536 Format specific information: compat: 1.1 lazy refcounts: false refcount bits: 16 corrupt: false this will *not* have an effect on this QEMU binary if it reboots, however, because QEMU does not appear to actually trigger format probing when used via the "scsi-generic" device type. Attempting to run qemu-img info against the /dev/sg3 device in the host fails as you can't read from an sg device directly. Presumably this is why QEMU won't do format probing either. Thus I don't think there is any security flaw here. The QEMU warning appears bogus. So to shut QEMU up: Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé -
Michal Privoznik