* [PATCH] cxl/region: Clarify pointers in unregister_region()
@ 2023-08-29 21:47 Ira Weiny
2023-08-30 11:06 ` Jonathan Cameron
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Ira Weiny @ 2023-08-29 21:47 UTC (permalink / raw)
To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
Vishal Verma, Dan Williams
Cc: linux-cxl, linux-kernel, Ben Widawsky, Ira Weiny
devm_add_action_or_reset() passes a void * data parameter to the
callback. In the case of CXL regions, unregister_region() is passed a
struct cxl_region *.
unregister_region() incorrectly interprets this as a struct device
pointer. Fortunately the device structure was the first member of
cxl_region. Therefore the code still works. However, should struct
cxl_region change the bug could be subtle.
Use the proper types in unregister_region() and extract the device
pointer correctly.
Fixes: 779dd20cfb56 ("cxl/region: Add region creation support")
Cc: Ben Widawsky <bwidawsk@kernel.org>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
drivers/cxl/core/region.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index e115ba382e04..471d305ef675 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2018,10 +2018,11 @@ static struct cxl_region *to_cxl_region(struct device *dev)
return container_of(dev, struct cxl_region, dev);
}
-static void unregister_region(void *dev)
+static void unregister_region(void *_cxlr)
{
- struct cxl_region *cxlr = to_cxl_region(dev);
+ struct cxl_region *cxlr = _cxlr;
struct cxl_region_params *p = &cxlr->params;
+ struct device *dev = &cxlr->dev;
int i;
device_del(dev);
---
base-commit: 1c59d383390f970b891b503b7f79b63a02db2ec5
change-id: 20230829-cxl-clarify-ptrs-f38255b7e52b
Best regards,
--
Ira Weiny <ira.weiny@intel.com>
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] cxl/region: Clarify pointers in unregister_region()
2023-08-29 21:47 [PATCH] cxl/region: Clarify pointers in unregister_region() Ira Weiny
@ 2023-08-30 11:06 ` Jonathan Cameron
2023-08-30 15:48 ` Davidlohr Bueso
2023-08-30 17:36 ` Dave Jiang
2 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2023-08-30 11:06 UTC (permalink / raw)
To: Ira Weiny
Cc: Davidlohr Bueso, Dave Jiang, Alison Schofield, Vishal Verma,
Dan Williams, linux-cxl, linux-kernel, Ben Widawsky
On Tue, 29 Aug 2023 14:47:46 -0700
Ira Weiny <ira.weiny@intel.com> wrote:
> devm_add_action_or_reset() passes a void * data parameter to the
> callback. In the case of CXL regions, unregister_region() is passed a
> struct cxl_region *.
>
> unregister_region() incorrectly interprets this as a struct device
> pointer. Fortunately the device structure was the first member of
> cxl_region. Therefore the code still works. However, should struct
> cxl_region change the bug could be subtle.
>
> Use the proper types in unregister_region() and extract the device
> pointer correctly.
>
> Fixes: 779dd20cfb56 ("cxl/region: Add region creation support")
> Cc: Ben Widawsky <bwidawsk@kernel.org>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> drivers/cxl/core/region.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e115ba382e04..471d305ef675 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2018,10 +2018,11 @@ static struct cxl_region *to_cxl_region(struct device *dev)
> return container_of(dev, struct cxl_region, dev);
> }
>
> -static void unregister_region(void *dev)
> +static void unregister_region(void *_cxlr)
> {
> - struct cxl_region *cxlr = to_cxl_region(dev);
> + struct cxl_region *cxlr = _cxlr;
> struct cxl_region_params *p = &cxlr->params;
> + struct device *dev = &cxlr->dev;
> int i;
>
> device_del(dev);
>
> ---
> base-commit: 1c59d383390f970b891b503b7f79b63a02db2ec5
> change-id: 20230829-cxl-clarify-ptrs-f38255b7e52b
>
> Best regards,
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cxl/region: Clarify pointers in unregister_region()
2023-08-29 21:47 [PATCH] cxl/region: Clarify pointers in unregister_region() Ira Weiny
2023-08-30 11:06 ` Jonathan Cameron
@ 2023-08-30 15:48 ` Davidlohr Bueso
2023-08-30 17:36 ` Dave Jiang
2 siblings, 0 replies; 4+ messages in thread
From: Davidlohr Bueso @ 2023-08-30 15:48 UTC (permalink / raw)
To: Ira Weiny
Cc: Jonathan Cameron, Dave Jiang, Alison Schofield, Vishal Verma,
Dan Williams, linux-cxl, linux-kernel, Ben Widawsky
On Tue, 29 Aug 2023, Ira Weiny wrote:
>devm_add_action_or_reset() passes a void * data parameter to the
>callback. In the case of CXL regions, unregister_region() is passed a
>struct cxl_region *.
>
>unregister_region() incorrectly interprets this as a struct device
>pointer. Fortunately the device structure was the first member of
>cxl_region. Therefore the code still works. However, should struct
>cxl_region change the bug could be subtle.
>
>Use the proper types in unregister_region() and extract the device
>pointer correctly.
>
>Fixes: 779dd20cfb56 ("cxl/region: Add region creation support")
>Cc: Ben Widawsky <bwidawsk@kernel.org>
>Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cxl/region: Clarify pointers in unregister_region()
2023-08-29 21:47 [PATCH] cxl/region: Clarify pointers in unregister_region() Ira Weiny
2023-08-30 11:06 ` Jonathan Cameron
2023-08-30 15:48 ` Davidlohr Bueso
@ 2023-08-30 17:36 ` Dave Jiang
2 siblings, 0 replies; 4+ messages in thread
From: Dave Jiang @ 2023-08-30 17:36 UTC (permalink / raw)
To: Ira Weiny, Davidlohr Bueso, Jonathan Cameron, Alison Schofield,
Vishal Verma, Dan Williams
Cc: linux-cxl, linux-kernel, Ben Widawsky
On 8/29/23 14:47, Ira Weiny wrote:
> devm_add_action_or_reset() passes a void * data parameter to the
> callback. In the case of CXL regions, unregister_region() is passed a
> struct cxl_region *.
>
> unregister_region() incorrectly interprets this as a struct device
> pointer. Fortunately the device structure was the first member of
> cxl_region. Therefore the code still works. However, should struct
> cxl_region change the bug could be subtle.
>
> Use the proper types in unregister_region() and extract the device
> pointer correctly.
>
> Fixes: 779dd20cfb56 ("cxl/region: Add region creation support")
> Cc: Ben Widawsky <bwidawsk@kernel.org>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Great catch!
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/cxl/core/region.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e115ba382e04..471d305ef675 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2018,10 +2018,11 @@ static struct cxl_region *to_cxl_region(struct device *dev)
> return container_of(dev, struct cxl_region, dev);
> }
>
> -static void unregister_region(void *dev)
> +static void unregister_region(void *_cxlr)
> {
> - struct cxl_region *cxlr = to_cxl_region(dev);
> + struct cxl_region *cxlr = _cxlr;
> struct cxl_region_params *p = &cxlr->params;
> + struct device *dev = &cxlr->dev;
> int i;
>
> device_del(dev);
>
> ---
> base-commit: 1c59d383390f970b891b503b7f79b63a02db2ec5
> change-id: 20230829-cxl-clarify-ptrs-f38255b7e52b
>
> Best regards,
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-08-30 19:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-29 21:47 [PATCH] cxl/region: Clarify pointers in unregister_region() Ira Weiny
2023-08-30 11:06 ` Jonathan Cameron
2023-08-30 15:48 ` Davidlohr Bueso
2023-08-30 17:36 ` Dave Jiang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).