| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aV1XnLrAiONbeA_X@aschofie-mobl2.lan>
Date: Tue, 6 Jan 2026 10:42:36 -0800
From: Alison Schofield <alison.schofield@...el.com>
To: Robert Richter <rrichter@....com>
CC: Davidlohr Bueso <dave@...olabs.net>, Jonathan Cameron
<jonathan.cameron@...wei.com>, Dave Jiang <dave.jiang@...el.com>, "Vishal
Verma" <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>, "Dan
Williams" <dan.j.williams@...el.com>, <linux-cxl@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] cxl: Check for invalid addresses returned from
translation functions on errors
On Tue, Jan 06, 2026 at 06:23:58PM +0100, Robert Richter wrote:
> Translation functions may return an invalid address in case of errors.
> If the address is not checked the further use of the invalid value
> will cause an address corruption.
>
> Consistently check for a valid address returned by translation
> functions. Use RESOURCE_SIZE_MAX to indicate an invalid address for
> type resource_size_t. Depending on the type either RESOURCE_SIZE_MAX
> or ULLONG_MAX is used to indicate an address error.
Thanks for separating this out. A couple of comments below:
>
> Signed-off-by: Robert Richter <rrichter@....com>
> ---
> v2:
> * separated from this patch series (Alison):
> [PATCH v8 00/13] cxl: ACPI PRM Address Translation Support and AMD Zen5 enablement
> * improved error handling logic and early return on error in
> region_offset_to_dpa_result() (Dave),
> * use RESOURCE_SIZE_MAX to indicate an invalid address for
> resource_size_t types (Alison, kernel test robot),
> * improved patch description (Alison),
> * added line wrap for code >80 chars.
> ---
> Signed-off-by: Robert Richter <rrichter@....com>
> ---
> drivers/cxl/core/hdm.c | 2 +-
> drivers/cxl/core/region.c | 34 ++++++++++++++++++++------
> tools/testing/cxl/test/cxl_translate.c | 5 ++--
> 3 files changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 1c5d2022c87a..031672e92b0b 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -530,7 +530,7 @@ resource_size_t cxl_dpa_size(struct cxl_endpoint_decoder *cxled)
>
> resource_size_t cxl_dpa_resource_start(struct cxl_endpoint_decoder *cxled)
> {
> - resource_size_t base = -1;
> + resource_size_t base = RESOURCE_SIZE_MAX;
>
> lockdep_assert_held(&cxl_rwsem.dpa);
> if (cxled->dpa_res)
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index fc36a5413d3f..5bd1213737fa 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3118,7 +3118,7 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
> struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> struct cxl_region_params *p = &cxlr->params;
> struct cxl_endpoint_decoder *cxled = NULL;
> - u64 dpa_offset, hpa_offset, hpa;
> + u64 base, dpa_offset, hpa_offset, hpa;
> u16 eig = 0;
> u8 eiw = 0;
> int pos;
> @@ -3136,8 +3136,14 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
> ways_to_eiw(p->interleave_ways, &eiw);
> granularity_to_eig(p->interleave_granularity, &eig);
>
> - dpa_offset = dpa - cxl_dpa_resource_start(cxled);
> + base = cxl_dpa_resource_start(cxled);
> + if (base == RESOURCE_SIZE_MAX)
> + return ULLONG_MAX;
> +
> + dpa_offset = dpa - base;
> hpa_offset = cxl_calculate_hpa_offset(dpa_offset, pos, eiw, eig);
> + if (hpa_offset == ULLONG_MAX)
> + return ULLONG_MAX;
>
> /* Apply the hpa_offset to the region base address */
> hpa = hpa_offset + p->res->start + p->cache_size;
> @@ -3146,6 +3152,9 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
> if (cxlrd->ops.hpa_to_spa)
> hpa = cxlrd->ops.hpa_to_spa(cxlrd, hpa);
>
> + if (hpa == ULLONG_MAX)
> + return ULLONG_MAX;
> +
> if (!cxl_resource_contains_addr(p->res, hpa)) {
> dev_dbg(&cxlr->dev,
> "Addr trans fail: hpa 0x%llx not in region\n", hpa);
> @@ -3170,7 +3179,8 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
> struct cxl_region_params *p = &cxlr->params;
> struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> struct cxl_endpoint_decoder *cxled;
> - u64 hpa, hpa_offset, dpa_offset;
> + u64 hpa_offset = offset;
> + u64 dpa, dpa_offset;
> u16 eig = 0;
> u8 eiw = 0;
> int pos;
> @@ -3187,10 +3197,13 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
> * CXL HPA is assumed to equal SPA.
> */
> if (cxlrd->ops.spa_to_hpa) {
> - hpa = cxlrd->ops.spa_to_hpa(cxlrd, p->res->start + offset);
> - hpa_offset = hpa - p->res->start;
> - } else {
> - hpa_offset = offset;
> + hpa_offset = cxlrd->ops.spa_to_hpa(cxlrd, p->res->start + offset);
> + if (hpa_offset == ULLONG_MAX) {
> + dev_dbg(&cxlr->dev, "HPA not found for %pr offset %#llx\n",
> + p->res, offset);
> + return -ENXIO;
> + }
> + hpa_offset -= p->res->start;
> }
>
> pos = cxl_calculate_position(hpa_offset, eiw, eig);
> @@ -3207,8 +3220,13 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
> cxled = p->targets[i];
> if (cxled->pos != pos)
> continue;
> +
> + dpa = cxl_dpa_resource_start(cxled);
We want to return -ENXIO, not 0 in this case.
So jump out here immediately - right?
if (dpa == REsOURCE_SIZE_MAX)
return -ENXIO;
> + if (dpa != RESOURCE_SIZE_MAX)
> + dpa += dpa_offset;
> +
> result->cxlmd = cxled_to_memdev(cxled);
> - result->dpa = cxl_dpa_resource_start(cxled) + dpa_offset;
> + result->dpa = dpa;
>
> return 0;
> }
> diff --git a/tools/testing/cxl/test/cxl_translate.c b/tools/testing/cxl/test/cxl_translate.c
> index 2200ae21795c..c2af918b853e 100644
> --- a/tools/testing/cxl/test/cxl_translate.c
> +++ b/tools/testing/cxl/test/cxl_translate.c
> @@ -69,7 +69,7 @@ static u64 to_hpa(u64 dpa_offset, int pos, u8 r_eiw, u16 r_eig, u8 hb_ways,
> /* Calculate base HPA offset from DPA and position */
> hpa_offset = cxl_calculate_hpa_offset(dpa_offset, pos, r_eiw, r_eig);
>
> - if (math == XOR_MATH) {
> + if (hpa_offset != ULLONG_MAX && math == XOR_MATH) {
Prefer no compound statement. ie.
if (hpa_offset == ULLONG_MAX)
return ULLONG_MAX;
then continue as is.
> cximsd->nr_maps = hbiw_to_nr_maps[hb_ways];
> if (cximsd->nr_maps)
> return cxl_do_xormap_calc(cximsd, hpa_offset, hb_ways);
> @@ -262,7 +262,8 @@ static int test_random_params(void)
Here we continue to do calcs after we may already know that hpa is
ULLONG_MAX. Need to skip calculating the reverse_dpa,pos and go right
to adding this as a failure.
> reverse_dpa = cxl_calculate_dpa_offset(hpa, eiw, eig);
> reverse_pos = cxl_calculate_position(hpa, eiw, eig);
>
> - if (reverse_dpa != dpa || reverse_pos != pos) {
> + if (hpa == ULLONG_MAX || reverse_dpa != dpa ||
> + reverse_pos != pos) {
> pr_err("test random iter %d FAIL hpa=%llu, dpa=%llu reverse_dpa=%llu, pos=%d reverse_pos=%d eiw=%u eig=%u\n",
> i, hpa, dpa, reverse_dpa, pos, reverse_pos, eiw,
> eig);
>
> base-commit: 88c72bab77aaf389beccf762e112828253ca0564
> --
> 2.47.3
>
>
Powered by blists - more mailing lists