Skip to content

Commit 8441c7d

Browse files
Robert Richterdavejiang
authored andcommitted
cxl: Check for invalid addresses returned from translation functions on errors
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. Propagating an invalid address from a failed translation may cause userspace to think it has received a valid SPA, when in fact it is wrong. The CXL userspace API, using trace events, expects ULLONG_MAX to indicate a translation failure. If ULLONG_MAX is not returned immediately, subsequent calculations can transform that bad address into a different value (!ULLONG_MAX), and an invalid SPA may be returned to userspace. This can lead to incorrect diagnostics and erroneous corrective actions. [ dj: Added user impact statement from Alison. ] [ dj: Fixed checkpatch tab alignment issue. ] Reviewed-by: Dave Jiang <dave.jiang@intel.com> Signed-off-by: Robert Richter <rrichter@amd.com> Fixes: c3dd676 ("cxl/region: Add inject and clear poison by region offset") Fixes: b78b9e7 ("cxl/region: Refactor address translation funcs for testing") Reviewed-by: Alison Schofield <alison.schofield@intel.com> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com> Link: https://patch.msgid.link/20260107120544.410993-1-rrichter@amd.com Signed-off-by: Dave Jiang <dave.jiang@intel.com>
1 parent d4026a4 commit 8441c7d

3 files changed

Lines changed: 45 additions & 21 deletions

File tree

drivers/cxl/core/hdm.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ resource_size_t cxl_dpa_size(struct cxl_endpoint_decoder *cxled)
530530

531531
resource_size_t cxl_dpa_resource_start(struct cxl_endpoint_decoder *cxled)
532532
{
533-
resource_size_t base = -1;
533+
resource_size_t base = RESOURCE_SIZE_MAX;
534534

535535
lockdep_assert_held(&cxl_rwsem.dpa);
536536
if (cxled->dpa_res)

drivers/cxl/core/region.c

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3118,7 +3118,7 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
31183118
struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
31193119
struct cxl_region_params *p = &cxlr->params;
31203120
struct cxl_endpoint_decoder *cxled = NULL;
3121-
u64 dpa_offset, hpa_offset, hpa;
3121+
u64 base, dpa_offset, hpa_offset, hpa;
31223122
u16 eig = 0;
31233123
u8 eiw = 0;
31243124
int pos;
@@ -3136,8 +3136,14 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
31363136
ways_to_eiw(p->interleave_ways, &eiw);
31373137
granularity_to_eig(p->interleave_granularity, &eig);
31383138

3139-
dpa_offset = dpa - cxl_dpa_resource_start(cxled);
3139+
base = cxl_dpa_resource_start(cxled);
3140+
if (base == RESOURCE_SIZE_MAX)
3141+
return ULLONG_MAX;
3142+
3143+
dpa_offset = dpa - base;
31403144
hpa_offset = cxl_calculate_hpa_offset(dpa_offset, pos, eiw, eig);
3145+
if (hpa_offset == ULLONG_MAX)
3146+
return ULLONG_MAX;
31413147

31423148
/* Apply the hpa_offset to the region base address */
31433149
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,
31463152
if (cxlrd->ops.hpa_to_spa)
31473153
hpa = cxlrd->ops.hpa_to_spa(cxlrd, hpa);
31483154

3155+
if (hpa == ULLONG_MAX)
3156+
return ULLONG_MAX;
3157+
31493158
if (!cxl_resource_contains_addr(p->res, hpa)) {
31503159
dev_dbg(&cxlr->dev,
31513160
"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,
31703179
struct cxl_region_params *p = &cxlr->params;
31713180
struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
31723181
struct cxl_endpoint_decoder *cxled;
3173-
u64 hpa, hpa_offset, dpa_offset;
3182+
u64 hpa_offset = offset;
3183+
u64 dpa, dpa_offset;
31743184
u16 eig = 0;
31753185
u8 eiw = 0;
31763186
int pos;
@@ -3187,10 +3197,13 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
31873197
* CXL HPA is assumed to equal SPA.
31883198
*/
31893199
if (cxlrd->ops.spa_to_hpa) {
3190-
hpa = cxlrd->ops.spa_to_hpa(cxlrd, p->res->start + offset);
3191-
hpa_offset = hpa - p->res->start;
3192-
} else {
3193-
hpa_offset = offset;
3200+
hpa_offset = cxlrd->ops.spa_to_hpa(cxlrd, p->res->start + offset);
3201+
if (hpa_offset == ULLONG_MAX) {
3202+
dev_dbg(&cxlr->dev, "HPA not found for %pr offset %#llx\n",
3203+
p->res, offset);
3204+
return -ENXIO;
3205+
}
3206+
hpa_offset -= p->res->start;
31943207
}
31953208

31963209
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,
32073220
cxled = p->targets[i];
32083221
if (cxled->pos != pos)
32093222
continue;
3223+
3224+
dpa = cxl_dpa_resource_start(cxled);
3225+
if (dpa != RESOURCE_SIZE_MAX)
3226+
dpa += dpa_offset;
3227+
32103228
result->cxlmd = cxled_to_memdev(cxled);
3211-
result->dpa = cxl_dpa_resource_start(cxled) + dpa_offset;
3229+
result->dpa = dpa;
32123230

32133231
return 0;
32143232
}

tools/testing/cxl/test/cxl_translate.c

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ static u64 to_hpa(u64 dpa_offset, int pos, u8 r_eiw, u16 r_eig, u8 hb_ways,
6868

6969
/* Calculate base HPA offset from DPA and position */
7070
hpa_offset = cxl_calculate_hpa_offset(dpa_offset, pos, r_eiw, r_eig);
71+
if (hpa_offset == ULLONG_MAX)
72+
return ULLONG_MAX;
7173

7274
if (math == XOR_MATH) {
7375
cximsd->nr_maps = hbiw_to_nr_maps[hb_ways];
@@ -258,19 +260,23 @@ static int test_random_params(void)
258260
pos = get_random_u32() % ways;
259261
dpa = get_random_u64() >> 12;
260262

263+
reverse_dpa = ULLONG_MAX;
264+
reverse_pos = -1;
265+
261266
hpa = cxl_calculate_hpa_offset(dpa, pos, eiw, eig);
262-
reverse_dpa = cxl_calculate_dpa_offset(hpa, eiw, eig);
263-
reverse_pos = cxl_calculate_position(hpa, eiw, eig);
264-
265-
if (reverse_dpa != dpa || reverse_pos != pos) {
266-
pr_err("test random iter %d FAIL hpa=%llu, dpa=%llu reverse_dpa=%llu, pos=%d reverse_pos=%d eiw=%u eig=%u\n",
267-
i, hpa, dpa, reverse_dpa, pos, reverse_pos, eiw,
268-
eig);
269-
270-
if (failures++ > 10) {
271-
pr_err("test random too many failures, stop\n");
272-
break;
273-
}
267+
if (hpa != ULLONG_MAX) {
268+
reverse_dpa = cxl_calculate_dpa_offset(hpa, eiw, eig);
269+
reverse_pos = cxl_calculate_position(hpa, eiw, eig);
270+
if (reverse_dpa == dpa && reverse_pos == pos)
271+
continue;
272+
}
273+
274+
pr_err("test random iter %d FAIL hpa=%llu, dpa=%llu reverse_dpa=%llu, pos=%d reverse_pos=%d eiw=%u eig=%u\n",
275+
i, hpa, dpa, reverse_dpa, pos, reverse_pos, eiw, eig);
276+
277+
if (failures++ > 10) {
278+
pr_err("test random too many failures, stop\n");
279+
break;
274280
}
275281
}
276282
pr_info("..... test random: PASS %d FAIL %d\n", i - failures, failures);

0 commit comments

Comments
 (0)