Re: [PATCH v2] cxl/acpi: Release device after dev_err()

[Dave Jiang <dave.jiang@intel.com>]

On 7/11/23 09:17, Breno Leitao wrote:
> Kfence detected an user-after-free in the CXL driver. This happens in
> the cxl_decoder_add() fail path. Kfence drops this message:
> 
>    BUG: KFENCE: use-after-free read in resource_string
> 
> This is happening in cxl_parse_cfmws(), where put_device() is called,
> releasing cxld->dev, and then, later, dev_err(cxld->dev) is called
> referencing the released device.
> 
> Just release the device after the message is printed/dev_err().  On top
> of that, cxl_parse_cfmws() must fail (returns rc) in case of
> cxl_decoder_add() or cxl_decoder_autoremove() failing (rc != 0), instead
> of swallowing the error and returning 0.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v1 -> v2
>   * Return the error (rc) instead of swalling it
> 
> ---
>   drivers/cxl/acpi.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 658e6b84a769..efead5cc8a89 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -291,14 +291,16 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
>   	}
>   	rc = cxl_decoder_add(cxld, target_map);
>   err_xormap:
> -	if (rc)
> -		put_device(&cxld->dev);
> -	else
> -		rc = cxl_decoder_autoremove(dev, cxld);
>   	if (rc) {
>   		dev_err(dev, "Failed to add decode range [%#llx - %#llx]\n",
>   			cxld->hpa_range.start, cxld->hpa_range.end);
> -		return 0;
> +		put_device(&cxld->dev);
> +		return rc;
> +	}
> +	rc = cxl_decoder_autoremove(dev, cxld);
> +	if (rc) {
> +		dev_err(dev, "Failed to register autoremove action\n");
> +		return rc;
>   	}
>   	dev_dbg(dev, "add: %s node: %d range [%#llx - %#llx]\n",
>   		dev_name(&cxld->dev),