On Wed, Nov 24, 2021 at 7:33 PM Halil Pasic <pasic@xxxxxxxxxxxxx> wrote: > > On Wed, 24 Nov 2021 10:33:28 +0800 > Jason Wang <jasowang@xxxxxxxxxx> wrote: > > > > > > Let's see how far we can get. But yes, maybe we were too aggressive in > > > > > breaking things by default, a warning might be a better choice for a > > > > > couple of cycles. > > > > > > Ok, considering we saw the issues with balloons I think I can post a > > > patch to use warn instead. I wonder if we need to taint the kernel in > > > this case. > > > > Rethink this, consider we still have some time, I tend to convert the > > drivers to validate the length by themselves. Does this make sense? > > I do find value in doing the validation in a single place for every > driver. This is really a common concern. But I think, not breaking > what used to work before is a good idea. So I would opt for producing > a warning, but otherwise preserving old behavior unless there is an > explicit opt-in for something more strict. Yes, I totally agree with you after more thought and discussion. > > BTW AFAIU if we detect a problem here, there are basically two > cases: > (1) Either the device is over-reporting what it has written, or > (2) we have a memory corruption in the guest because the device has > written beyond the end of the provided buffer. This can be because > (2.1) the driver provided a smaller buffer than mandated by the spec, > or > (2.2) the device is broken. > > Case (1) is relatively harmless, and I believe a warning for it is more > than appropriate. Whoever sees the warning should push for a fixed device > if possible. Yes. > > Case (2) is nasty. What would be the sanest course of action if we were > reasonably sure we have have case (2.2)? I think that's why a per driver validation is more preferable. The driver can choose the comfortable action, e.g for networking it may just drop the packets. > > Maybe we can detect case (2) with a canary. I.e. artificially extend > the buffer with an extra descriptor that has a poisoned buffer, and > check if the value of that poisoned buffer is different than poison. I'm > not sure it is worth the effort though in production. This might work but it might cause performance overhead. I still think doing the validation per driver is better, the driver can choose to fix the used length and taint the kernel anyway. Thanks > > Regards, > Halil > _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
