On Mon, Jul 25, 2011 at 02:14:28PM -0600, Alex Williamson wrote: > On Sun, 2011-07-24 at 11:36 +0300, Michael S. Tsirkin wrote: > > On Thu, Jul 21, 2011 at 11:02:53AM -0600, Alex Williamson wrote: > > > This is crazy, why would we only test this for PCI_CAP_ID_EXP? If the > > > test is going to go in device-assignment, we need to wrap > > > pci_add_capability and do it for all callers. However, maybe we can get > > > MST to take it in pci_add_capability() if we make the test more > > > complete... something like: > > > > > > if ((pos < 256 && size > 256 - pos) || > > > (pci_config_size() > 256 && pos > 256 && > > > size > pci_config_size() - pos)) { > > > ... badness > > > > > > Thanks, > > > > > > Alex > > > > We expect device assignment to be able to corrupt > > guest memory but not qemu memory. So we must validate > > Gee, thanks. Ugh, sorry, not device assignment per se. I really meant an assigned device controlled by a malicious guest. > > whatever we get from the device, and I think this > > validation belongs in device-assignment.c: > > > > IOW I think input should be validated where it's > > input, while we still know it's untrusted, > > instead of relying on core to validate parameters. > > > > Makes sense? > > No, not really. Why should the core "trust" anything? We generally don't validate most function parameters. We trust callers to a level because they can corrupt our memory anyway. > This is a basic, simply sanity test, why push it out to the leaf > driver when pushing it > into the core provides the sanity test for everyone? Is it beneath an > emulated driver to pass such a test? Thanks, > > Alex It's easy to add this to the core but please don't rely on this: functions validating parameters is a debugging aid. Validating untrusted input from a guest-controlled device is a security feature. -- MST -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html