Using device_find_child() to lookup the proper device to destroy
causes an unbalance in device refcount, since device_find_child()
calls an implicit get_device() to increment the device's reference
count before returning the pointer. cxl_port_find_switch_decoder()
directly converts this pointer to a cxl_decoder and returns it without
releasing the reference properly. We should call put_device() to
decrement reference count.

As comment of device_find_child() says, 'NOTE: you will need to drop
the reference with put_device() after use'.

Found by code review.

Cc: stable@vger.kernel.org
Fixes: d6879d8cfb81 ("cxl/region: Add function to find a port's switch decoder by range")
Signed-off-by: Ma Ke <make24@iscas.ac.cn>
---
 drivers/cxl/core/region.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 6e5e1460068d..cae16d761261 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3234,6 +3234,9 @@ cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa)
 	struct device *cxld_dev = device_find_child(&port->dev, hpa,
 						    match_decoder_by_range);
 
+	/* Drop the refcnt bumped implicitly by device_find_child */
+	put_device(cxld_dev);
+
 	return cxld_dev ? to_cxl_decoder(cxld_dev) : NULL;
 }
 
-- 
2.25.1

On 7/14/2025 5:16 PM, Ma Ke wrote:
> Using device_find_child() to lookup the proper device to destroy
> causes an unbalance in device refcount, since device_find_child()
> calls an implicit get_device() to increment the device's reference
> count before returning the pointer. cxl_port_find_switch_decoder()
> directly converts this pointer to a cxl_decoder and returns it without
> releasing the reference properly. We should call put_device() to
> decrement reference count.
>
> As comment of device_find_child() says, 'NOTE: you will need to drop
> the reference with put_device() after use'.
>
> Found by code review.
>
> Cc: stable@vger.kernel.org
> Fixes: d6879d8cfb81 ("cxl/region: Add function to find a port's switch decoder by range")
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
> ---
>  drivers/cxl/core/region.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 6e5e1460068d..cae16d761261 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3234,6 +3234,9 @@ cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa)
>  	struct device *cxld_dev = device_find_child(&port->dev, hpa,
>  						    match_decoder_by_range);
>  
> +	/* Drop the refcnt bumped implicitly by device_find_child */
> +	put_device(cxld_dev);
> +
>  	return cxld_dev ? to_cxl_decoder(cxld_dev) : NULL;
>  }
>  

It is not needed, because cxl_port_find_switch_decoder() is only invoked in cxl_find_root_decoder(), and cxl_find_root_decoder() is only called in cxl_add_to_region(), there is a __free(put_cxl_root_decoder) in cxl_add_to_region() for the device dereference.


Ming