Message ID | 20230714093146.2253438-1-leitao@debian.org |
---|---|
State | Accepted |
Commit | 3840e10e8dcafc89a2d96ab7dbb640434c8da8cf |
Headers | show |
Series | [v3,1/2] cxl/acpi: Fix UAF in the error path | expand |
On Fri, Jul 14, 2023 at 02:31:45AM -0700, Breno Leitao wrote: > KASAN and KFENCE detected an user-after-free in the CXL driver. This > happens in the cxl_decoder_add() fail path. KASAN prints the following > error: > > BUG: KASAN: slab-use-after-free in cxl_parse_cfmws (drivers/cxl/acpi.c:299) > > This is happening in cxl_parse_cfmws(), where put_device() is called, > releasing cxld, which is accessed later. > > Just use the local variables in the dev_err() instead of pointing to the > released memory. > > Fixes: e50fe01e1f2a ("cxl/core: Drop ->platform_res attribute for root decoders") > Signed-off-by: Breno Leitao <leitao@debian.org> Reviewed-by: Alison Schofield <alison.schofield@intel.com> > --- > v1 -> v2 > * Return the error (rc) instead of swalling it > v2 -> v3 > * Split the change in two patches > * Fix the reference instead of the order > > drivers/cxl/acpi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > index 658e6b84a769..642983da01cb 100644 > --- a/drivers/cxl/acpi.c > +++ b/drivers/cxl/acpi.c > @@ -297,7 +297,7 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg, > 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); > + res->start, res->end); > return 0; > } > dev_dbg(dev, "add: %s node: %d range [%#llx - %#llx]\n", > -- > 2.34.1 >
Breno Leitao wrote: > KASAN and KFENCE detected an user-after-free in the CXL driver. This > happens in the cxl_decoder_add() fail path. KASAN prints the following > error: > > BUG: KASAN: slab-use-after-free in cxl_parse_cfmws (drivers/cxl/acpi.c:299) > > This is happening in cxl_parse_cfmws(), where put_device() is called, > releasing cxld, which is accessed later. > > Just use the local variables in the dev_err() instead of pointing to the > released memory. > > Fixes: e50fe01e1f2a ("cxl/core: Drop ->platform_res attribute for root decoders") > Signed-off-by: Breno Leitao <leitao@debian.org> > --- > v1 -> v2 > * Return the error (rc) instead of swalling it > v2 -> v3 > * Split the change in two patches > * Fix the reference instead of the order > > drivers/cxl/acpi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > index 658e6b84a769..642983da01cb 100644 > --- a/drivers/cxl/acpi.c > +++ b/drivers/cxl/acpi.c > @@ -297,7 +297,7 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg, > 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); > + res->start, res->end); Came here after the 0day report... Since this is switching the reuse @res, it can also switch to using %pr to print the resource.
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c index 658e6b84a769..642983da01cb 100644 --- a/drivers/cxl/acpi.c +++ b/drivers/cxl/acpi.c @@ -297,7 +297,7 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg, 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); + res->start, res->end); return 0; } dev_dbg(dev, "add: %s node: %d range [%#llx - %#llx]\n",
KASAN and KFENCE detected an user-after-free in the CXL driver. This happens in the cxl_decoder_add() fail path. KASAN prints the following error: BUG: KASAN: slab-use-after-free in cxl_parse_cfmws (drivers/cxl/acpi.c:299) This is happening in cxl_parse_cfmws(), where put_device() is called, releasing cxld, which is accessed later. Just use the local variables in the dev_err() instead of pointing to the released memory. Fixes: e50fe01e1f2a ("cxl/core: Drop ->platform_res attribute for root decoders") Signed-off-by: Breno Leitao <leitao@debian.org> --- v1 -> v2 * Return the error (rc) instead of swalling it v2 -> v3 * Split the change in two patches * Fix the reference instead of the order drivers/cxl/acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)