| 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
| ||
|
Date: Wed, 25 Oct 2023 18:19:46 +0800
From: Yi Liu <yi.l.liu@...el.com>
To: Jason Gunthorpe <jgg@...dia.com>
CC: <joro@...tes.org>, <alex.williamson@...hat.com>,
<kevin.tian@...el.com>, <robin.murphy@....com>,
<baolu.lu@...ux.intel.com>, <cohuck@...hat.com>,
<eric.auger@...hat.com>, <nicolinc@...dia.com>,
<kvm@...r.kernel.org>, <mjrosato@...ux.ibm.com>,
<chao.p.peng@...ux.intel.com>, <yi.y.sun@...ux.intel.com>,
<peterx@...hat.com>, <jasowang@...hat.com>,
<shameerali.kolothum.thodi@...wei.com>, <lulu@...hat.com>,
<suravee.suthikulpanit@....com>, <iommu@...ts.linux.dev>,
<linux-kernel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
<zhenzhong.duan@...el.com>, <joao.m.martins@...cle.com>
Subject: Re: [PATCH v6 07/10] iommufd: Add a nested HW pagetable object
On 2023/10/25 01:30, Jason Gunthorpe wrote:
> On Tue, Oct 24, 2023 at 02:18:10PM -0300, Jason Gunthorpe wrote:
>> On Tue, Oct 24, 2023 at 08:06:06AM -0700, Yi Liu wrote:
>>> @@ -195,6 +279,10 @@ int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd)
>>> if (pt_obj->type == IOMMUFD_OBJ_IOAS) {
>>> struct iommufd_hwpt_paging *hwpt_paging;
>>>
>>> + if (cmd->data_type != IOMMU_HWPT_DATA_NONE) {
>>> + rc = -EINVAL;
>>> + goto out_put_pt;
>>> + }
>>> ioas = container_of(pt_obj, struct iommufd_ioas, obj);
>>> mutex_lock(&ioas->mutex);
>>> hwpt_paging = iommufd_hwpt_paging_alloc(ucmd->ictx, ioas, idev,
>>
>> ?? What is this?
>>
>> Ah something went wrong earlier in "iommu: Pass in parent domain with
>> user_data to domain_alloc_user op"
>
> Bah, I got confused because that had half the uapi, so in this pathc
>
>> Once we added the user_data we should flow it through to the op
>> always.
>
> Like this:
ack.
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> index 92859b907bb93c..a3382811af8a81 100644
> --- a/drivers/iommu/iommufd/device.c
> +++ b/drivers/iommu/iommufd/device.c
> @@ -586,8 +586,8 @@ iommufd_device_auto_get_domain(struct iommufd_device *idev,
> goto out_unlock;
> }
>
> - hwpt_paging = iommufd_hwpt_paging_alloc(idev->ictx, ioas, idev,
> - 0, immediate_attach);
> + hwpt_paging = iommufd_hwpt_paging_alloc(idev->ictx, ioas, idev, 0,
> + immediate_attach, NULL);
> if (IS_ERR(hwpt_paging)) {
> destroy_hwpt = ERR_CAST(hwpt_paging);
> goto out_unlock;
> diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
> index cfd85df693d7b2..324a6d73f032ee 100644
> --- a/drivers/iommu/iommufd/hw_pagetable.c
> +++ b/drivers/iommu/iommufd/hw_pagetable.c
> @@ -96,7 +96,8 @@ iommufd_hwpt_paging_enforce_cc(struct iommufd_hwpt_paging *hwpt_paging)
> struct iommufd_hwpt_paging *
> iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
> struct iommufd_device *idev, u32 flags,
> - bool immediate_attach)
> + bool immediate_attach,
> + const struct iommu_user_data *user_data)
> {
> const u32 valid_flags = IOMMU_HWPT_ALLOC_NEST_PARENT |
> IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
> @@ -107,7 +108,7 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
>
> lockdep_assert_held(&ioas->mutex);
>
> - if (flags && !ops->domain_alloc_user)
> + if ((flags || user_data) && !ops->domain_alloc_user)
> return ERR_PTR(-EOPNOTSUPP);
> if (flags & ~valid_flags)
> return ERR_PTR(-EOPNOTSUPP);
> @@ -127,7 +128,7 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
>
> if (ops->domain_alloc_user) {
> hwpt->domain = ops->domain_alloc_user(idev->dev, flags,
> - NULL, NULL);
> + NULL, user_data);
> if (IS_ERR(hwpt->domain)) {
> rc = PTR_ERR(hwpt->domain);
> hwpt->domain = NULL;
> @@ -210,8 +211,7 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx,
> struct iommufd_hw_pagetable *hwpt;
> int rc;
>
> - if (flags || user_data->type == IOMMU_HWPT_DATA_NONE ||
> - !ops->domain_alloc_user)
> + if (flags || !user_data->len || !ops->domain_alloc_user)
> return ERR_PTR(-EOPNOTSUPP);
> if (parent->auto_domain || !parent->nest_parent)
> return ERR_PTR(-EINVAL);
> @@ -249,6 +249,11 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx,
> int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd)
> {
> struct iommu_hwpt_alloc *cmd = ucmd->cmd;
> + const struct iommu_user_data user_data = {
> + .type = cmd->data_type,
> + .uptr = u64_to_user_ptr(cmd->data_uptr),
> + .len = cmd->data_len,
> + };
> struct iommufd_hw_pagetable *hwpt;
> struct iommufd_ioas *ioas = NULL;
> struct iommufd_object *pt_obj;
> @@ -273,25 +278,17 @@ int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd)
> if (pt_obj->type == IOMMUFD_OBJ_IOAS) {
> struct iommufd_hwpt_paging *hwpt_paging;
>
> - if (cmd->data_type != IOMMU_HWPT_DATA_NONE) {
> - rc = -EINVAL;
> - goto out_put_pt;
> - }
it can be covered by iommu driver when checking user data pointer. hence
remove above if.
> ioas = container_of(pt_obj, struct iommufd_ioas, obj);
> mutex_lock(&ioas->mutex);
> - hwpt_paging = iommufd_hwpt_paging_alloc(ucmd->ictx, ioas, idev,
> - cmd->flags, false);
> + hwpt_paging = iommufd_hwpt_paging_alloc(
> + ucmd->ictx, ioas, idev, cmd->flags, false,
> + user_data.len ? &user_data : NULL);
> if (IS_ERR(hwpt_paging)) {
> rc = PTR_ERR(hwpt_paging);
> goto out_unlock;
> }
> hwpt = &hwpt_paging->common;
> } else if (pt_obj->type == IOMMUFD_OBJ_HWPT_PAGING) {
> - const struct iommu_user_data user_data = {
> - .type = cmd->data_type,
> - .uptr = u64_to_user_ptr(cmd->data_uptr),
> - .len = cmd->data_len,
> - };
> struct iommufd_hwpt_nested *hwpt_nested;
> struct iommufd_hwpt_paging *parent;
>
> diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
> index 6fdad56893af4d..24e5a36fc875e0 100644
> --- a/drivers/iommu/iommufd/iommufd_private.h
> +++ b/drivers/iommu/iommufd/iommufd_private.h
> @@ -290,7 +290,8 @@ int iommufd_hwpt_get_dirty_bitmap(struct iommufd_ucmd *ucmd);
> struct iommufd_hwpt_paging *
> iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
> struct iommufd_device *idev, u32 flags,
> - bool immediate_attach);
> + bool immediate_attach,
> + const struct iommu_user_data *user_data);
> int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
> struct iommufd_device *idev);
> struct iommufd_hw_pagetable *
--
Regards,
Yi Liu
Powered by blists - more mailing lists