[PATCH 0/7] ATAPI CDROM passthrough v5

[Ian Jackson]

I've picked up Alex's patch series for this. Changes since v4:
* rebased to current tip as of approximately yesterday
* passthrough of firmware update is enabled by default
* fixed some build issues

This series breaks the cris-softmmu build.

I think I've fixed it up. It builds for me, anyway. If it now
doesn't build do please tell me the error message and I'l fix it.

Also, I think Paul and I both requested that fw upgrade not be
disabled by default.

As previously discussed I think this is a mistake, but it's a decision
for qemu upstream to make so I have changed this.

I would also suggest that you only expose this as an option
through qdev properties instead of a new command line option as it
should be controllable on a per-device basis.

The reason to disable it is not to prevent the guest breaking the
hardware. It is to prevent the guest escaping the containment
entirely, which it can probably do if firmware updates are allowed.
This seems to me to be a general property of the guest, rather than of
the device. So I think disabling it in one place is better.

Patches follow.

Thanks,
Ian.

[Ian Jackson]

Post by Ian Jackson
I think I've fixed it up. It builds for me, anyway. If it now
doesn't build do please tell me the error message and I'l fix it.

I did have to do this, to fix an unrelated `may be used initialised'
warning. I'm not sure if it's the right fix.

Ian.

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 4da916c..2efd21c 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -99,7 +99,7 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
const char *opts)
{
PCIDevice *dev;
- DriveInfo *dinfo;
+ DriveInfo *dinfo=0;
int type = -1;
char buf[128];

[Christoph Hellwig]

Post by Ian Jackson
* rebased to current tip as of approximately yesterday
* passthrough of firmware update is enabled by default
* fixed some build issues

If you resend patch from someone else the normal convention is to put
a

From: Original Author <***@foobar.blah>

into the patch body so that it's obvious who the author is.

[Ian Jackson]

Post by Christoph Hellwig
If you resend patch from someone else the normal convention is to put
a
into the patch body so that it's obvious who the author is.

OK, I will do that next time.

(I thought the Signed-Off-By would be sufficient but I'm happy to do
whatever people think best.)

Ian.

[Anthony Liguori]

Post by Ian Jackson

I would also suggest that you only expose this as an option
through qdev properties instead of a new command line option as it
should be controllable on a per-device basis.

The reason to disable it is not to prevent the guest breaking the
hardware. It is to prevent the guest escaping the containment
entirely, which it can probably do if firmware updates are allowed.
This seems to me to be a general property of the guest, rather than of
the device. So I think disabling it in one place is better.

If you go back to the original thread, the argument against this was
that some devices abuse other atapi commands to do firmware updates so
you cannot 100% reliably contain this.

But more importantly, and the reason I originally requested this, having
a global option bakes knowledge of atapi pass through into vl.c. Making
it a qdev property means vl.c does not need explicit knowledge of this
mechanism.

I think this is an important change to make for merging.

Regards,

Anthony Liguori

Post by Ian Jackson
Patches follow.
Thanks,
Ian.

[Bique Alexandre]

Post by Ian Jackson
* rebased to current tip as of approximately yesterday
* passthrough of firmware update is enabled by default
* fixed some build issues

This series breaks the cris-softmmu build.

I think I've fixed it up. It builds for me, anyway. If it now
doesn't build do please tell me the error message and I'l fix it.

Also, I think Paul and I both requested that fw upgrade not be
disabled by default.

As previously discussed I think this is a mistake, but it's a decision
for qemu upstream to make so I have changed this.

I would also suggest that you only expose this as an option
through qdev properties instead of a new command line option as it
should be controllable on a per-device basis.

The reason to disable it is not to prevent the guest breaking the
hardware. It is to prevent the guest escaping the containment
entirely, which it can probably do if firmware updates are allowed.
This seems to me to be a general property of the guest, rather than of
the device. So I think disabling it in one place is better.
Patches follow.

Hi Ian,

Are you going to continue to work on this patch queue or do you want me to
finish the job ?

Thanks a lot.

[Carl-Daniel Hailfinger]

Post by Ian Jackson

Also, I think Paul and I both requested that fw upgrade not be
disabled by default.

As previously discussed I think this is a mistake, but it's a decision
for qemu upstream to make so I have changed this.

Anyone up for writing a security advisory about this?

Regards,
Carl-Daniel

[Anthony Liguori]

Post by Ian Jackson

Also, I think Paul and I both requested that fw upgrade not be
disabled by default.

As previously discussed I think this is a mistake, but it's a decision
for qemu upstream to make so I have changed this.

Anyone up for writing a security advisory about this?\

Eh?

If you do hardware passthrough, the guest can mess up the device. This
is always going to be true and it's a security problem IMHO to make the
user think anything other than that.

Regards,

Anthony Liguori

Regards,
Carl-Daniel

[Carl-Daniel Hailfinger]

Post by Anthony Liguori

Post by Carl-Daniel Hailfinger

Post by Ian Jackson

Also, I think Paul and I both requested that fw upgrade not be
disabled by default.

As previously discussed I think this is a mistake, but it's a decision
for qemu upstream to make so I have changed this.

Anyone up for writing a security advisory about this?

Eh?
If you do hardware passthrough, the guest can mess up the device.
This is always going to be true and it's a security problem IMHO to
make the user think anything other than that.

The guest can also mess up other devices with the help of specially
crafted firmware. So even if the user does not care about the effects on
a particular device, a firmware upgrade might affect other devices
(which are not used by Qemu in any way) as well. As a result, this is
essentially a "break out of qemu or DoS the machine under certain
conditions" feature. If that particular side effect / feature is
documented, users who read the documentation won't get any nasty surprises.
If that's what you intended to say, I apologize for the misunderstanding.

Regards,
Carl-Daniel

Post by Anthony Liguori
Regards,
Anthony Liguori

[Anthony Liguori]

Post by Carl-Daniel Hailfinger
The guest can also mess up other devices with the help of specially
crafted firmware. So even if the user does not care about the effects on
a particular device, a firmware upgrade might affect other devices
(which are not used by Qemu in any way) as well.

Please be more specific. How is this any different than PCI passthrough
with VT-d or USB passthrough?

Post by Carl-Daniel Hailfinger
As a result, this is
essentially a "break out of qemu or DoS the machine under certain
conditions" feature. If that particular side effect / feature is
documented, users who read the documentation won't get any nasty surprises.

A user will get a really nasty surprise if they think they can use a
flag or rely on QEMU to prevent a VM from doing something nasty with a
device. If they have this feeling of security, they're likely to chmod
the device to allow unprivileged users to access it.

But how a device handles ATAPI commands is totally up to the device. If
you issue the wrong sequence, I'm sure there are devices out there that
totally hose themselves. Are you absolutely confident that every ATAPI
device out there is completely safe against hostile code provided that
you simply prevent the FW update commands? I'm certainly not.

Regards,

Anthony Liguori

[Alexander Graf]

Post by Carl-Daniel Hailfinger
The guest can also mess up other devices with the help of specially
crafted firmware. So even if the user does not care about the effects on
a particular device, a firmware upgrade might affect other devices
(which are not used by Qemu in any way) as well.

Please be more specific. How is this any different than PCI passthrough with VT-d or USB passthrough?

Post by Carl-Daniel Hailfinger
As a result, this is
essentially a "break out of qemu or DoS the machine under certain
conditions" feature. If that particular side effect / feature is
documented, users who read the documentation won't get any nasty surprises.

A user will get a really nasty surprise if they think they can use a flag or rely on QEMU to prevent a VM from doing something nasty with a device. If they have this feeling of security, they're likely to chmod the device to allow unprivileged users to access it.
But how a device handles ATAPI commands is totally up to the device. If you issue the wrong sequence, I'm sure there are devices out there that totally hose themselves. Are you absolutely confident that every ATAPI device out there is completely safe against hostile code provided that you simply prevent the FW update commands? I'm certainly not.

Ping?


Alex

[Anthony Liguori]

A user will get a really nasty surprise if they think they can use a flag or rely on QEMU to prevent a VM from doing something nasty with a device. If they have this feeling of security, they're likely to chmod the device to allow unprivileged users to access it.
But how a device handles ATAPI commands is totally up to the device. If you issue the wrong sequence, I'm sure there are devices out there that totally hose themselves. Are you absolutely confident that every ATAPI device out there is completely safe against hostile code provided that you simply prevent the FW update commands? I'm certainly not.

Ping?

Who are you pinging?

Regards,

Anthony Liguori

Alex

[Alexander Graf]

Post by Anthony Liguori

A user will get a really nasty surprise if they think they can use a flag or rely on QEMU to prevent a VM from doing something nasty with a device. If they have this feeling of security, they're likely to chmod the device to allow unprivileged users to access it.
But how a device handles ATAPI commands is totally up to the device. If you issue the wrong sequence, I'm sure there are devices out there that totally hose themselves. Are you absolutely confident that every ATAPI device out there is completely safe against hostile code provided that you simply prevent the FW update commands? I'm certainly not.

Ping?

Who are you pinging?

Mostly Ian. I haven't seen any follow-up on this discussion and would like to know why and if there's still plans to upstream this code :).

Alex

[Michal Suchanek]

Post by Alexander Graf

Post by Anthony Liguori

A user will get a really nasty surprise if they think they can use a flag or rely on QEMU to prevent a VM from doing something nasty with a device.  If they have this feeling of security, they're likely to chmod the device to allow unprivileged users to access it.
But how a device handles ATAPI commands is totally up to the device.  If you issue the wrong sequence, I'm sure there are devices out there that totally hose themselves.  Are you absolutely confident that every ATAPI device out there is completely safe against hostile code provided that you simply prevent the FW update commands?  I'm certainly not.

Ping?

Who are you pinging?

Mostly Ian. I haven't seen any follow-up on this discussion and would like to know why and if there's still plans to upstream this code :).

Why is allowing ATAPI passthrough such a problem?

Sure if your boot drive is on the same IDE cable as the device you may
have issues but other than that the device may just stop working if it
is not designed to handle incorrect command gracefully (ie it is
broken).

I am sure there are devices that also break under issuing correct
commands or commands that look vaguely sane. Eg. there are CD-ROMs
that would lock up the whole system when you boot certain vintage of
Linux (not tested with current Linux due to lack of old hardware) on a
machine with the Intel BX chipset and one of these CD-ROMs attached
over IDE cable.

However, assuming random hardware breakage you cannot allow anything.

Perhaps the ATAPI passthrough should be designed to allow any commands
and some command profiles could be selected to allow for some
sane/conservative subset, burning, LightScribe, LabelFlash, disc
***@tto, FW upgrade, ..

It would be nice if these subsets were defined in a configuration file
so that people can create their own 'default' combination or just
install a new set when a new fancy feature comes out.

Thanks

Michal