On Tue, Oct 29, 2024 at 03:55:58PM -0300, Jason Gunthorpe wrote: > On Tue, Oct 29, 2024 at 09:18:05AM -0700, Nicolin Chen wrote: > > On Tue, Oct 29, 2024 at 11:49:07AM -0300, Jason Gunthorpe wrote: > > > On Fri, Oct 25, 2024 at 04:49:43PM -0700, Nicolin Chen wrote: > > > > To support driver-allocated vIOMMU objects, it's suggested to call the > > > > allocator helper in IOMMU dirvers. However, there is no guarantee that > > > > drivers will all use it and allocate objects properly. > > > > > > > > Add a helper for iommufd core to verify if an unfinalized object is at > > > > least reserved in the ictx. > > > > > > I don't think we need this.. > > > > > > iommufd_object_finalize() already does: > > > > > > old = xa_store(&ictx->objects, obj->id, obj, GFP_KERNEL); > > > /* obj->id was returned from xa_alloc() so the xa_store() cannot fail */ > > > WARN_ON(old); > > > > It feels unsafe to carry on the iommufd_viommu_alloc_ioctl() until > > iommufd_object_finalize() as the function would touch the returned > > faulty viommu pointer? E.g. what if the viommu has an even smaller > > size than struct iommufd_viommu? > > This is Linux just because the output came from a driver doesn't mean > we have to validate it somehow. It is reasonable to be helpful and > detect driver bugs, but if the driver is buggy it is still OK to > crash. > > So you don't *have* to check any of this, if the driver didn't use the > right function to allocate the memory then it will go bad pretty fast. > > Improving the xa_store() is something that will detect more kinds of > bugs everywhere, so seems more worthwhile I see. Thanks! Nicolin
