| Message ID | 174138547078.1626291.2681320775917569071.stgit@dwillia2-xfh.jf.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | cxl/fwctl: Cleanup unused fwctl_dev from cxl_features_state | expand |
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; >
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
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;
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(-)