[PATCH] cxl/fwctl: Cleanup unused fwctl_dev from cxl_features_state

[PATCH] cxl/fwctl: Cleanup unused fwctl_dev from cxl_features_state

[Dan Williams]

A review of devm_cxl_setup_fwctl() indicates a potential a use after
free condition for the ->fwctl_dev pointer given it is not cleared after
devm_add_action_or_reset() might have released it. However, nothing uses
it, so just delete it.

If it ever comes back it should be returned by devm_cxl_setup_fwctl() so
that users can always assume the pointer is either valid or NULL.

Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Li Ming <ming.li@zohomail.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---

base-commit: da0dd17604d4c70080497091c762a790b0871eff

 drivers/cxl/core/features.c |    2 --
 include/cxl/features.h      |    2 --
 2 files changed, 4 deletions(-)

diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index caf92e9cea21..f4daefe3180e 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -700,8 +700,6 @@ int devm_cxl_setup_fwctl(struct cxl_memdev *cxlmd)
 	if (rc)
 		return rc;
 
-	cxlfs->fwctl_dev = fwctl_dev;
-
 	return devm_add_action_or_reset(&cxlmd->dev, free_memdev_fwctl,
 					no_free_ptr(fwctl_dev));
 }
diff --git a/include/cxl/features.h b/include/cxl/features.h
index ead63573b0b4..fbfdc601bd85 100644
--- a/include/cxl/features.h
+++ b/include/cxl/features.h
@@ -53,7 +53,6 @@ enum cxl_features_capability {
  * @entries: CXl feature entry context
  *	@num_features: total Features supported by the device
  *	@ent: Flex array of Feature detail entries from the device
- * @fwctl_dev: Firmware Control device
  */
 struct cxl_features_state {
 	struct cxl_dev_state *cxlds;
@@ -62,7 +61,6 @@ struct cxl_features_state {
 		int num_user_features;
 		struct cxl_feat_entry ent[] __counted_by(num_features);
 	} *entries;
-	struct fwctl_device *fwctl_dev;
 };
 
 struct cxl_mailbox;

Re: [PATCH] cxl/fwctl: Cleanup unused fwctl_dev from cxl_features_state

[Dave Jiang]

On 3/7/25 3:11 PM, Dan Williams wrote:
> A review of devm_cxl_setup_fwctl() indicates a potential a use after
> free condition for the ->fwctl_dev pointer given it is not cleared after
> devm_add_action_or_reset() might have released it. However, nothing uses
> it, so just delete it.
> 
> If it ever comes back it should be returned by devm_cxl_setup_fwctl() so
> that users can always assume the pointer is either valid or NULL.
> 
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Li Ming <ming.li@zohomail.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> 
> base-commit: da0dd17604d4c70080497091c762a790b0871eff
> 
>  drivers/cxl/core/features.c |    2 --
>  include/cxl/features.h      |    2 --
>  2 files changed, 4 deletions(-)
> 
> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index caf92e9cea21..f4daefe3180e 100644
> --- a/drivers/cxl/core/features.c
> +++ b/drivers/cxl/core/features.c
> @@ -700,8 +700,6 @@ int devm_cxl_setup_fwctl(struct cxl_memdev *cxlmd)
>  	if (rc)
>  		return rc;
>  
> -	cxlfs->fwctl_dev = fwctl_dev;
> -
>  	return devm_add_action_or_reset(&cxlmd->dev, free_memdev_fwctl,
>  					no_free_ptr(fwctl_dev));
>  }
> diff --git a/include/cxl/features.h b/include/cxl/features.h
> index ead63573b0b4..fbfdc601bd85 100644
> --- a/include/cxl/features.h
> +++ b/include/cxl/features.h
> @@ -53,7 +53,6 @@ enum cxl_features_capability {
>   * @entries: CXl feature entry context
>   *	@num_features: total Features supported by the device
>   *	@ent: Flex array of Feature detail entries from the device
> - * @fwctl_dev: Firmware Control device
>   */
>  struct cxl_features_state {
>  	struct cxl_dev_state *cxlds;
> @@ -62,7 +61,6 @@ struct cxl_features_state {
>  		int num_user_features;
>  		struct cxl_feat_entry ent[] __counted_by(num_features);
>  	} *entries;
> -	struct fwctl_device *fwctl_dev;
>  };
>  
>  struct cxl_mailbox;
> 

Re: [PATCH] cxl/fwctl: Cleanup unused fwctl_dev from cxl_features_state

[Jason Gunthorpe]

On Fri, Mar 07, 2025 at 03:26:02PM -0700, Dave Jiang wrote:
> 
> 
> On 3/7/25 3:11 PM, Dan Williams wrote:
> > A review of devm_cxl_setup_fwctl() indicates a potential a use after
> > free condition for the ->fwctl_dev pointer given it is not cleared after
> > devm_add_action_or_reset() might have released it. However, nothing uses
> > it, so just delete it.
> > 
> > If it ever comes back it should be returned by devm_cxl_setup_fwctl() so
> > that users can always assume the pointer is either valid or NULL.
> > 
> > Cc: Dave Jiang <dave.jiang@intel.com>
> > Cc: Li Ming <ming.li@zohomail.com>
> > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Jason Gunthorpe <jgg@nvidia.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> 
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>

I can squash this into the v8

Thanks,
Jason