linux-kernel - Re: [PATCH v2 13/14] iommufd: Introduce iommufd_object_alloc

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: <aG3+s8rzcjeKazG8@yilunxu-OptiPlex-7050>
Date: Wed, 9 Jul 2025 13:31:31 +0800
From: Xu Yilun <yilun.xu@...ux.intel.com>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: jgg@...dia.com, kevin.tian@...el.com, will@...nel.org,
	robin.murphy@....com, joro@...tes.org, praan@...gle.com,
	yi.l.liu@...el.com, peterz@...radead.org, jsnitsel@...hat.com,
	linux-arm-kernel@...ts.infradead.org, iommu@...ts.linux.dev,
	linux-kernel@...r.kernel.org, patches@...ts.linux.dev,
	baolu.lu@...ux.intel.com
Subject: Re: [PATCH v2 13/14] iommufd: Introduce iommufd_object_alloc_ucmd
 helper

> @@ -61,6 +61,24 @@ struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,
>  	return ERR_PTR(rc);
>  }
>  
> +struct iommufd_object *_iommufd_object_alloc_ucmd(struct iommufd_ucmd *ucmd,
> +						  size_t size,
> +						  enum iommufd_object_type type)
> +{
> +	struct iommufd_object *new_obj;
> +
> +	/* Something is coded wrong if this is hit */
> +	if (WARN_ON(ucmd->new_obj))
> +		return ERR_PTR(-EBUSY);
> +
> +	new_obj = _iommufd_object_alloc(ucmd->ictx, size, type);
> +	if (IS_ERR(new_obj))
> +		return new_obj;
> +
> +	ucmd->new_obj = new_obj;
> +	return new_obj;
> +}
> +
>  /*
>   * Allow concurrent access to the object.
>   *
> @@ -448,6 +466,13 @@ static long iommufd_fops_ioctl(struct file *filp, unsigned int cmd,
>  	if (ret)
>  		return ret;
>  	ret = op->execute(&ucmd);
> +
> +	if (ucmd.new_obj) {
> +		if (ret)
> +			iommufd_object_abort_and_destroy(ictx, ucmd.new_obj);

Sorry I didn't follow this thread before and maybe missed something.

According to 70eadc7fc7ef, abort op is for the object that can assume
the caller is holding the lock. But here is for no locking, so calling
iommufd_object_abort_and_destroy() is quite confusing.

Is it better we change to:

	if (ret) {
		iommufd_object_ops[obj->type].destroy(obj);
		iommufd_object_abort(ictx, obj);
	}

Also explicitely assert iommufd_object_alloc_ucmd() and abort can't be
used at the same time.

in _iommufd_object_alloc_ucmd():

	if (WARN_ON(iommufd_object_ops[type].abort))
		return ERR_PTR(-EFAULT);

Thanks,
Yilun

> +		else
> +			iommufd_object_finalize(ictx, ucmd.new_obj);
> +	}
>  	return ret;
>  }
>  
> -- 
> 2.43.0
> 
>