On Mon, Sep 02, 2024 at 09:16:54PM -0300, Jason Gunthorpe wrote: > On Mon, Sep 02, 2024 at 10:11:16AM +0000, Mostafa Saleh wrote: > > > > What is the harm? Does exposing IDR data to userspace in any way > > > compromise the security or integrity of the system? > > > > > > I think no - how could it? > > > > I don’t see a clear harm or exploit with exposing IDRs, but IMHO we > > should deal with userspace with the least privilege principle and > > only expose what user space cares about (with sanitised IDRs or > > through another mechanism) > > If the information is harmless then why hide it? We expose all kinds > of stuff to userspace, like most of the PCI config space for > instance. I think we need a reason. > > Any sanitization in the kernel will complicate everything because we > will get it wrong. > > Let's not make things complicated without reasons. Intel and AMD are > exposing their IDR equivalents in this manner as well. > > > For example, KVM doesn’t allow reading reading the CPU system > > registers to know if SVE(or other features) is supported but hides > > that by a CAP in KVM_CHECK_EXTENSION > > Do you know why? > I am not really sure, but I believe it’s a useful abstraction > > > As the comments says, the VMM should not just blindly forward this to > > > a guest! > > > > I don't think the kernel should trust userspace. > > There is no trust. If the VMM blindly forwards the IDRS then the VMM > will find its VM's have issues. It is a functional bug, just as if the > VMM puts random garbage in its vIDRS. > > The onl purpose of this interface is to provide information about the > physical hardware to the VMM. > > > > The VMM needs to make its own IDR to reflect its own vSMMU > > > capabilities. It can refer to the kernel IDR if it needs to. > > > > > > So, if the kernel is going to limit it, what criteria would you > > > propose the kernel use? > > > > I agree that the VMM would create a virtual IDR for guest, but that > > doesn't have to be directly based on the physical one (same as CPU). > > No one said it should be. In fact the comment explicitly says not to > do that. > > The VMM is expected to read out of the physical IDR any information > that effects data structures that are under direct guest control. > > For instance anything that effects the CD on downwards. So page sizes, > IAS limits, etc etc etc. Anything that effects assigned invalidation > queues. Anything that impacts errata the VM needs to be aware of. > > If you sanitize it then you will hide information that someone will > need at some point, then we have go an unsanitize it, then add feature > flags.. It is a pain. I don’t have a very strong opinion to sanitise the IDRs (specifically many of those are documented anyway per IP), but at least we should have some clear requirement for what userspace needs, I am just concerned that userspace can misuse some of the features leading to a strange UAPI. Thanks, Mostafa > > Jason
