linux-kernel - Re: [PATCH v1 11/12] 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: <aEyxhKOSlvP23/81@nvidia.com>
Date: Fri, 13 Jun 2025 16:17:24 -0700
From: Nicolin Chen <nicolinc@...dia.com>
To: Jason Gunthorpe <jgg@...dia.com>
CC: <kevin.tian@...el.com>, <will@...nel.org>, <robin.murphy@....com>,
	<joro@...tes.org>, <ddutile@...hat.com>, <yi.l.liu@...el.com>,
	<peterz@...radead.org>, <jsnitsel@...hat.com>, <praan@...gle.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 v1 11/12] iommufd: Introduce iommufd_object_alloc_ucmd
 helper

On Fri, Jun 13, 2025 at 10:58:17AM -0300, Jason Gunthorpe wrote:
> On Mon, Jun 09, 2025 at 10:13:34AM -0700, Nicolin Chen wrote:
> 
> > +#define iommufd_object_alloc_ucmd(ucmd, ptr, type) \
> > +	__iommufd_object_alloc_ucmd(ucmd, ptr, type, obj)
> 
> Lets add a comment here and on the normal iommufd_object_alloc
> explaining that this function automatically calls finalize and abort,
> the non ucmd version requires the caller to do so.

Yes.

@@ -231,6 +231,11 @@ iommufd_object_put_and_try_destroy(struct iommufd_ctx *ictx,
        iommufd_object_remove(ictx, obj, obj->id, 0);
 }

+/*
+ * Callers of these normal object allocators must call iommufd_object_finalize()
+ * to finalize the object, or call iommufd_object_abort_and_destroy() to revert
+ * the allocation.
+ */
 struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,
                                             size_t size,
                                             enum iommufd_object_type type);
@@ -247,6 +252,10 @@ struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,
 #define iommufd_object_alloc(ictx, ptr, type) \
        __iommufd_object_alloc(ictx, ptr, type, obj)

+/*
+ * Callers of these _ucmd allocators should not call iommufd_object_finalize()
+ * or iommufd_object_abort_and_destroy(), as the core automatically does that.
+ */
 struct iommufd_object *
 _iommufd_object_alloc_ucmd(struct iommufd_ucmd *ucmd, size_t size,
                           enum iommufd_object_type type);

> > +struct iommufd_object *_iommufd_object_alloc_ucmd(struct iommufd_ucmd *ucmd,
> > +						  size_t size,
> > +						  enum iommufd_object_type type)
> > +{
> > +	struct iommufd_object *new_obj;
> > +
> > +	if (ucmd->new_obj)
> 
> WARN_ON? Something is coded wrong if we hit this right?

Yes.

@@ -67,7 +67,8 @@ struct iommufd_object *_iommufd_object_alloc_ucmd(struct iommufd_ucmd *ucmd,
 {
        struct iommufd_object *new_obj;

-       if (ucmd->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);

Thanks
Nicolin