lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b1663110e1ca4301834f403270357bea@huawei.com>
Date: Mon, 15 Jan 2024 17:44:13 +0000
From: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@...wei.com>
To: Jason Gunthorpe <jgg@...pe.ca>
CC: Lu Baolu <baolu.lu@...ux.intel.com>, Kevin Tian <kevin.tian@...el.com>,
	Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>, Robin Murphy
	<robin.murphy@....com>, Jean-Philippe Brucker <jean-philippe@...aro.org>,
	Nicolin Chen <nicolinc@...dia.com>, Yi Liu <yi.l.liu@...el.com>, Jacob Pan
	<jacob.jun.pan@...ux.intel.com>, "iommu@...ts.linux.dev"
	<iommu@...ts.linux.dev>, "linux-kselftest@...r.kernel.org"
	<linux-kselftest@...r.kernel.org>,
	"virtualization@...ts.linux-foundation.org"
	<virtualization@...ts.linux-foundation.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 4/6] iommufd: Deliver fault messages to user space



> -----Original Message-----
> From: Jason Gunthorpe <jgg@...pe.ca>
> Sent: Monday, January 15, 2024 4:47 PM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@...wei.com>
> Cc: Lu Baolu <baolu.lu@...ux.intel.com>; Kevin Tian <kevin.tian@...el.com>;
> Joerg Roedel <joro@...tes.org>; Will Deacon <will@...nel.org>; Robin
> Murphy <robin.murphy@....com>; Jean-Philippe Brucker <jean-
> philippe@...aro.org>; Nicolin Chen <nicolinc@...dia.com>; Yi Liu
> <yi.l.liu@...el.com>; Jacob Pan <jacob.jun.pan@...ux.intel.com>;
> iommu@...ts.linux.dev; linux-kselftest@...r.kernel.org;
> virtualization@...ts.linux-foundation.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH v2 4/6] iommufd: Deliver fault messages to user space
> 
> On Fri, Jan 12, 2024 at 05:46:13PM +0000, Shameerali Kolothum Thodi wrote:
> >
> >
> > > -----Original Message-----
> > > From: Lu Baolu <baolu.lu@...ux.intel.com>
> > > Sent: Thursday, October 26, 2023 3:49 AM
> > > To: Jason Gunthorpe <jgg@...pe.ca>; Kevin Tian <kevin.tian@...el.com>;
> Joerg
> > > Roedel <joro@...tes.org>; Will Deacon <will@...nel.org>; Robin
> Murphy
> > > <robin.murphy@....com>; Jean-Philippe Brucker <jean-
> philippe@...aro.org>;
> > > Nicolin Chen <nicolinc@...dia.com>; Yi Liu <yi.l.liu@...el.com>; Jacob
> Pan
> > > <jacob.jun.pan@...ux.intel.com>
> > > Cc: iommu@...ts.linux.dev; linux-kselftest@...r.kernel.org;
> > > virtualization@...ts.linux-foundation.org; linux-kernel@...r.kernel.org;
> Lu
> > > Baolu <baolu.lu@...ux.intel.com>
> > > Subject: [PATCH v2 4/6] iommufd: Deliver fault messages to user space
> > >
> > [...]
> >
> > Hi,
> >
> > > +static ssize_t hwpt_fault_fops_write(struct file *filep,
> > > +				     const char __user *buf,
> > > +				     size_t count, loff_t *ppos)
> > > +{
> > > +	size_t response_size = sizeof(struct iommu_hwpt_page_response);
> > > +	struct hw_pgtable_fault *fault = filep->private_data;
> > > +	struct iommu_hwpt_page_response response;
> > > +	struct iommufd_hw_pagetable *hwpt;
> > > +	struct iopf_group *iter, *group;
> > > +	struct iommufd_device *idev;
> > > +	size_t done = 0;
> > > +	int rc = 0;
> > > +
> > > +	if (*ppos || count % response_size)
> > > +		return -ESPIPE;
> > > +
> > > +	mutex_lock(&fault->mutex);
> > > +	while (!list_empty(&fault->response) && count > done) {
> > > +		rc = copy_from_user(&response, buf + done, response_size);
> > > +		if (rc)
> > > +			break;
> > > +
> > > +		/* Get the device that this response targets at. */
> > > +		idev = container_of(iommufd_get_object(fault->ictx,
> > > +						       response.dev_id,
> > > +						       IOMMUFD_OBJ_DEVICE),
> > > +				    struct iommufd_device, obj);
> > > +		if (IS_ERR(idev)) {
> > > +			rc = PTR_ERR(idev);
> > > +			break;
> > > +		}
> > > +
> > > +		/*
> > > +		 * Get the hw page table that this response was generated
> for.
> > > +		 * It must match the one stored in the fault data.
> > > +		 */
> > > +		hwpt = container_of(iommufd_get_object(fault->ictx,
> > > +						       response.hwpt_id,
> > > +
> > > IOMMUFD_OBJ_HW_PAGETABLE),
> > > +				    struct iommufd_hw_pagetable, obj);
> > > +		if (IS_ERR(hwpt)) {
> > > +			iommufd_put_object(&idev->obj);
> > > +			rc = PTR_ERR(hwpt);
> > > +			break;
> > > +		}
> > > +
> > > +		if (hwpt != fault->hwpt) {
> > > +			rc = -EINVAL;
> > > +			goto put_obj;
> > > +		}
> > > +
> > > +		group = NULL;
> > > +		list_for_each_entry(iter, &fault->response, node) {
> > > +			if (response.grpid != iter->last_fault.fault.prm.grpid)
> > > +				continue;
> > > +
> > > +			if (idev->dev != iter->dev)
> > > +				continue;
> > > +
> > > +			if ((iter->last_fault.fault.prm.flags &
> > > +			     IOMMU_FAULT_PAGE_REQUEST_PASID_VALID)
> &&
> > > +			    response.pasid != iter->last_fault.fault.prm.pasid)
> > > +				continue;
> >
> > I am trying to get vSVA working with this series and got hit by the above
> check.
> > On ARM platforms, page responses to stall events(CMD_RESUME) do not
> have
> > an associated pasid.  I think, either we need to check here using
> > IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID or remove the check
> > as it will be eventually done in iommu_page_response().
> 
> That doesn't sound right..
> 
> The PASID is the only information we have for userspace to identify
> the domain that is being faulted. It cannot be optional on the request
> side.
> 

Yes, it is valid on the request side. But this is on the response side.

> If it is valid when userspace does read() then it should be valid when
> userspace does write() too.
> 
> It is the only way the kernel can actually match request and response
> here.

The kernel currently checks the pasid only if IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID
is set.

https://lore.kernel.org/linux-iommu/20200616144712.748818-1-jean-philippe@linaro.org/

> So, I think you have a userspace issue to not provide the right
> pasid??

This is not just ARM stall resume case, but for some PCI devices as well as per
the above commit log. So do we really need to track this in userspace ?

Thanks,
Shameer

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ