diff mbox series

[2/7] cxl/region: Fix cxl_region leak, cleanup targets at region delete

Message ID 166752183055.947915.17681995648556534844.stgit@dwillia2-xfh.jf.intel.com
State Accepted
Commit 9ef13807cf9ab9a5e32dd26070637de998e5fc07
Headers show
Series CXL region creation fixes for 6.1 | expand

Commit Message

Dan Williams Nov. 4, 2022, 12:30 a.m. UTC
When a region is deleted any targets that have been previously assigned
to that region hold references to it. Trigger those references to
drop by detaching all targets at unregister_region() time.

Otherwise that region object will leak as userspace has lost the ability
to detach targets once region sysfs is torn down.

Cc: <stable@vger.kernel.org>
Fixes: b9686e8c8e39 ("cxl/region: Enable the assignment of endpoint decoders to regions")
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/core/region.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Verma, Vishal L Nov. 4, 2022, 5:39 a.m. UTC | #1
On Thu, 2022-11-03 at 17:30 -0700, Dan Williams wrote:
> When a region is deleted any targets that have been previously assigned
> to that region hold references to it. Trigger those references to
> drop by detaching all targets at unregister_region() time.
> 
> Otherwise that region object will leak as userspace has lost the ability
> to detach targets once region sysfs is torn down.
> 
> Cc: <stable@vger.kernel.org>
> Fixes: b9686e8c8e39 ("cxl/region: Enable the assignment of endpoint decoders to regions")
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/core/region.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)

Looks good,

Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>

> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index d26ca7a6beae..c52465e09f26 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1557,8 +1557,19 @@ static struct cxl_region *to_cxl_region(struct device *dev)
>  static void unregister_region(void *dev)
>  {
>         struct cxl_region *cxlr = to_cxl_region(dev);
> +       struct cxl_region_params *p = &cxlr->params;
> +       int i;
>  
>         device_del(dev);
> +
> +       /*
> +        * Now that region sysfs is shutdown, the parameter block is now
> +        * read-only, so no need to hold the region rwsem to access the
> +        * region parameters.
> +        */
> +       for (i = 0; i < p->interleave_ways; i++)
> +               detach_target(cxlr, i);
> +
>         cxl_region_iomem_release(cxlr);
>         put_device(dev);
>  }
>
Dave Jiang Nov. 4, 2022, 9:38 p.m. UTC | #2
On 11/3/2022 5:30 PM, Dan Williams wrote:
> When a region is deleted any targets that have been previously assigned
> to that region hold references to it. Trigger those references to
> drop by detaching all targets at unregister_region() time.
> 
> Otherwise that region object will leak as userspace has lost the ability
> to detach targets once region sysfs is torn down.
> 
> Cc: <stable@vger.kernel.org>
> Fixes: b9686e8c8e39 ("cxl/region: Enable the assignment of endpoint decoders to regions")
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>   drivers/cxl/core/region.c |   11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index d26ca7a6beae..c52465e09f26 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1557,8 +1557,19 @@ static struct cxl_region *to_cxl_region(struct device *dev)
>   static void unregister_region(void *dev)
>   {
>   	struct cxl_region *cxlr = to_cxl_region(dev);
> +	struct cxl_region_params *p = &cxlr->params;
> +	int i;
>   
>   	device_del(dev);
> +
> +	/*
> +	 * Now that region sysfs is shutdown, the parameter block is now
> +	 * read-only, so no need to hold the region rwsem to access the
> +	 * region parameters.
> +	 */
> +	for (i = 0; i < p->interleave_ways; i++)
> +		detach_target(cxlr, i);
> +
>   	cxl_region_iomem_release(cxlr);
>   	put_device(dev);
>   }
>
diff mbox series

Patch

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index d26ca7a6beae..c52465e09f26 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -1557,8 +1557,19 @@  static struct cxl_region *to_cxl_region(struct device *dev)
 static void unregister_region(void *dev)
 {
 	struct cxl_region *cxlr = to_cxl_region(dev);
+	struct cxl_region_params *p = &cxlr->params;
+	int i;
 
 	device_del(dev);
+
+	/*
+	 * Now that region sysfs is shutdown, the parameter block is now
+	 * read-only, so no need to hold the region rwsem to access the
+	 * region parameters.
+	 */
+	for (i = 0; i < p->interleave_ways; i++)
+		detach_target(cxlr, i);
+
 	cxl_region_iomem_release(cxlr);
 	put_device(dev);
 }