On Mon, Aug 07, 2023 at 08:12:37PM -0700, Nicolin Chen wrote: > On Mon, Aug 07, 2023 at 03:08:29PM +0000, Liu, Yi L wrote: > > > > From: Liu, Yi L <yi.l.liu@xxxxxxxxx> > > > > Sent: Monday, July 24, 2023 7:14 PM > > > > > > > > +static int intel_nested_cache_invalidate_user(struct iommu_domain > > > > *domain, > > > > + void *user_data) > > > > +{ > > > > + struct iommu_hwpt_vtd_s1_invalidate_desc *req = user_data; > > > > + struct iommu_hwpt_vtd_s1_invalidate *inv_info = user_data; > > > > + struct dmar_domain *dmar_domain = to_dmar_domain(domain); > > > > + unsigned int entry_size = inv_info->entry_size; > > > > + u64 uptr = inv_info->inv_data_uptr; > > > > + u64 nr_uptr = inv_info->entry_nr_uptr; > > > > + struct device_domain_info *info; > > > > + u32 entry_nr, index; > > > > + unsigned long flags; > > > > + int ret = 0; > > > > + > > > > + if (get_user(entry_nr, (uint32_t __user *)u64_to_user_ptr(nr_uptr))) > > > > + return -EFAULT; > > > > + > > > > + for (index = 0; index < entry_nr; index++) { > > > > + ret = copy_struct_from_user(req, sizeof(*req), > > > > + u64_to_user_ptr(uptr + index * > > > > entry_size), > > > > + entry_size); > > > > > > If continuing this direction then the driver should also check minsz etc. > > > for struct iommu_hwpt_vtd_s1_invalidate and iommu_hwpt_vtd_s1_invalidate_desc > > > since they are uAPI and subject to change. > > > > Then needs to define size in the uapi data structure, and copy size first and > > check minsz before going forward. How about the structures for hwpt alloc > > like struct iommu_hwpt_vtd_s1? Should check minsz for them as well? > > Assuming that every uAPI data structure needs a min_size, we can > either add a structure holding all min_sizes like iommufd main.c > or have another xx_min_len in iommu_/domain_ops. If driver is doing the copy it is OK that driver does the min_size check too Jason