A comma-separated target list can be miscounted, causing cxl create-region
to reject a valid multi-target request when --ways is omitted. The
generated filter can also acquire a trailing comma.
This happens because the separator is re-evaluated after tokenization has
already modified the input.
Choose the separator before tokenizing and use the target count when
emitting the flattened list.
Fixes: 3d6cd829ec08 ("cxl/region: Use cxl_filter_walk() to gather create-region
targets")
Assisted-by: LLM
Signed-off-by: Alison Schofield <[email protected]>
---
cxl/region.c | 21 +++++++-----
test/cxl-create-region.sh | 72 +++++++++++++++++++++++++++++++++++++++
2 files changed, 84 insertions(+), 9 deletions(-)
diff --git a/cxl/region.c b/cxl/region.c
index 85d4d9bb54f2..43c228f2f999 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -124,24 +124,25 @@ static const struct option destroy_options[] = {
static const char *to_csv(int *count, const char **strings)
{
ssize_t len = 0, cursor = 0;
+ int i, new_count = 0, emitted = 0;
char *csv, *list, *save;
- int i, new_count = 0;
- const char *arg;
+ const char *arg, *sep;
if (!*count)
return NULL;
for (i = 0; i < *count; i++) {
/*
- * An entry in strings may itself by a space or comma
- * separated list, so decompose that for the final csv
+ * An entry may itself be space- or comma-separated. strtok_r()
+ * modifies @list, so choose the separator before tokenizing.
*/
list = strdup(strings[i]);
if (!list)
return NULL;
- for (arg = strtok_r(list, which_sep(list), &save); arg;
- arg = strtok_r(NULL, which_sep(list), &save)) {
+ sep = which_sep(list);
+ for (arg = strtok_r(list, sep, &save); arg;
+ arg = strtok_r(NULL, sep, &save)) {
len += strlen(arg);
new_count++;
}
@@ -161,10 +162,12 @@ static const char *to_csv(int *count, const char
**strings)
return NULL;
}
- for (arg = strtok_r(list, which_sep(list), &save); arg;
- arg = strtok_r(NULL, which_sep(list), &save)) {
+ sep = which_sep(list);
+ for (arg = strtok_r(list, sep, &save); arg;
+ arg = strtok_r(NULL, sep, &save)) {
cursor += snprintf(csv + cursor, len - cursor, "%s%s",
- arg, i + 1 < new_count ? "," : "");
+ arg,
+ ++emitted < new_count ? "," : "");
if (cursor >= len) {
csv[len - 1] = 0;
break;
diff --git a/test/cxl-create-region.sh b/test/cxl-create-region.sh
index 927edf4c17d4..6fc845c7d382 100644
--- a/test/cxl-create-region.sh
+++ b/test/cxl-create-region.sh
@@ -226,6 +226,76 @@ setup_x3()
memdevs="$mem0 $mem1 $mem2"
}
+# Build a 4-way target list over an x2 root in interleave position order.
+setup_interleave_shape()
+{
+ x2_decoder=$($CXL list -b cxl_test -D -d root |
+ jq -r ".[] |
+ select(.pmem_capable == true) |
+ select(.nr_targets == 2) |
+ .decoder")
+
+ if [[ ! $x2_decoder ]]; then
+ echo "no pmem capable x2 root decoder found"
+ return 1
+ fi
+
+ pos0=$(find_memdev "$x2_decoder" 0 0)
+ pos1=$(find_memdev "$x2_decoder" 1 0)
+ pos2=$(find_memdev "$x2_decoder" 0 1)
+ pos3=$(find_memdev "$x2_decoder" 1 1)
+
+ if [[ ! $pos0 || ! $pos1 || ! $pos2 || ! $pos3 ]]; then
+ echo "$x2_decoder: no memdev pair for each interleave position"
+ return 1
+ fi
+}
+
+# A comma-separated target list must preserve its count and order.
+create_csv_targets()
+{
+ if ! setup_interleave_shape; then
+ echo "skipping comma separated target list"
+ return
+ fi
+
+ if ! json=$($CXL create-region -b cxl_test -d "$x2_decoder" \
+ -m "$pos0,$pos1,$pos2,$pos3"); then
+ echo "$x2_decoder: refused $pos0,$pos1,$pos2,$pos3"
+ err "$LINENO"
+ fi
+
+ region=$(jq -r ".region" <<< "$json")
+ ways=$(jq -r ".interleave_ways" <<< "$json")
+ if [[ $ways -ne 4 ]]; then
+ echo "$region: interleaves $ways ways, expected 4"
+ err "$LINENO"
+ fi
+
+ targets=$(jq -r "[.mappings | sort_by(.position) | .[].memdev] |
+ join(\" \")" <<< "$json")
+ if [[ "$targets" != "$pos0 $pos1 $pos2 $pos3" ]]; then
+ echo "$region: targets '$targets', expected '$pos0 $pos1 $pos2
$pos3'"
+ err "$LINENO"
+ fi
+
+ destroy_regions "$region"
+
+ # Also verify a rejected filter is echoed without a trailing comma.
+ if out=$($CXL create-region -b cxl_test -d "$x2_decoder" \
+ -m nomemdev0,nomemdev1,nomemdev2 2>&1); then
+ echo "create-region accepted names that are not memdevs"
+ err "$LINENO"
+ fi
+
+ if ! grep -Fq -- "filter: nomemdev0,nomemdev1,nomemdev2" <<< "$out" ||
+ grep -Fq -- "nomemdev2," <<< "$out"; then
+ echo "target list echoed back as:"
+ echo "$out"
+ err "$LINENO"
+ fi
+}
+
# test region creation on devices behind a single-port host-bridge
create_single
@@ -239,6 +309,8 @@ for mem in ${mems[@]}; do
create_subregions "$mem"
done
+create_csv_targets
+
# Repeat x1/x2/x3/x4 region creation with XOR interleave arithmetic.
load_cxl_test interleave_arithmetic=1
--
2.37.3
