On Thu, May 27, 2021 at 04:06:20PM -0300, Jason Gunthorpe wrote: > On Thu, May 27, 2021 at 02:53:42PM +1000, David Gibson wrote: > > > > > If the physical device had a bug which meant the mdevs *weren't* > > > > properly isolated from each other, then those mdevs would share a > > > > group, and you *would* care about it. Depending on how the isolation > > > > failed the mdevs might or might not also share a group with the parent > > > > physical device. > > > > > > That isn't a real scenario.. mdevs that can't be isolated just > > > wouldn't be useful to exist > > > > Really? So what do you do when you discover some mdevs you thought > > were isolated actually aren't due to a hardware bug? Drop support > > from the driver entirely? In which case what do you say to the people > > who understandably complain "but... we had all the mdevs in one guest > > anyway, we don't care if they're not isolated"? > > I've never said to eliminate groups entirely. > > What I'm saying is that all the cases we have for mdev today do not > require groups, but are forced to create a fake group anyhow just to > satisfy the odd VFIO requirement to have a group FD. > > If some future mdev needs groups then sure, add the appropriate group > stuff. > > But that doesn't effect the decision to have a VFIO group FD, or not. > > > > > It ensures that they're parked at the moment the group moves from > > > > kernel to userspace ownership, but it can't prevent dpdk from > > > > accessing and unparking those devices via peer to peer DMA. > > > > > > Right, and adding all this group stuff did nothing to alert the poor > > > admin that is running DPDK to this risk. > > > > Didn't it? Seems to me the admin that in order to give the group to > > DPDK, the admin had to find and unbind all the things in it... so is > > therefore aware that they're giving everything in it to DPDK. > > Again, I've never said the *group* should be removed. I'm only > concerned about the *group FD* Ok, that wasn't really clear to me. I still wouldn't say the group for mdevs is a fiction though.. rather that the group device used for (no internal IOMMU case) mdevs is just plain wrong. > When the admin found and unbound they didn't use the *group FD* in any > way. No, they are likely to have changed permissions on the group device node as part of the process, though. > > > You put the same security labels you'd put on the group to the devices > > > that consitute the group. It is only more tricky in the sense that the > > > script that would have to do this will need to do more than ID the > > > group to label but also ID the device members of the group and label > > > their char nodes. > > > > Well, I guess, if you take the view that root is allowed to break the > > kernel. I tend to prefer that although root can obviously break the > > kernel if they intend do, we should make it hard to do by accident - > > which in this case would mean the kernel *enforcing* that the devices > > in the group have the same security labels, which I can't really see > > how to do without an exposed group. > > How is this "break the kernel"? It has nothing to do with the > kernel. Security labels are a user space concern. *thinks*... yeah, ok, that was much too strong an assertion. What I was thinking of is the fact that this means that guarantees you'd normally expect the kernel to enforce can be obviated by bad configuration: chown-ing a device to root doesn't actually protect it if there's another device in the same group exposed to other users. But I guess you could say the same about, say, an unauthenticated nbd export of a root-owned block device, so I guess that's not something the kernel can reasonably enforce. Ok.. you might be finally convincing me, somewhat. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
