Skip to content

Commit

Permalink
of: Fix double free in of_parse_phandle_with_args_map
Browse files Browse the repository at this point in the history 
In of_parse_phandle_with_args_map() the inner loop that
iterates through the map entries calls of_node_put(new)
to free the reference acquired by the previous iteration
of the inner loop. This assumes that the value of "new" is
NULL on the first iteration of the inner loop.

Make sure that this is true in all iterations of the outer
loop by setting "new" to NULL after its value is assigned to "cur".

Extend the unittest to detect the double free and add an additional
test case that actually triggers this path.

Fixes: bd6f2fd ("of: Support parsing phandle argument lists through a nexus node")
Cc: Stephen Boyd <stephen.boyd@linaro.org>
Signed-off-by: "Christian A. Ehrhardt" <lk@c--e.de>
Link: https://lore.kernel.org/r/20231229105411.1603434-1-lk@c--e.de
Signed-off-by: Rob Herring <robh@kernel.org>
  • Loading branch information
Christian A. Ehrhardt authored and Rob Herring committed Jan 9, 2024
1 parent 5e3ef45 commit 4dde835
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 32 deletions.
1 change: 1 addition & 0 deletions drivers/of/base.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1464,6 +1464,7 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
out_args->np = new;
of_node_put(cur);
cur = new;
new = NULL;
}
put:
of_node_put(cur);
Expand Down
10 changes: 9 additions & 1 deletion drivers/of/unittest-data/tests-phandle.dtsi
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@
phandle-map-pass-thru = <0x0 0xf0>;
};

provider5: provider5 {
#phandle-cells = <2>;
phandle-map = <2 7 &provider4 2 3>;
phandle-map-mask = <0xff 0xf>;
phandle-map-pass-thru = <0x0 0xf0>;
};

consumer-a {
phandle-list = <&provider1 1>,
<&provider2 2 0>,
Expand All @@ -66,7 +73,8 @@
<&provider4 4 0x100>,
<&provider4 0 0x61>,
<&provider0>,
<&provider4 19 0x20>;
<&provider4 19 0x20>,
<&provider5 2 7>;
phandle-list-bad-phandle = <12345678 0 0>;
phandle-list-bad-args = <&provider2 1 0>,
<&provider4 0>;
Expand Down
74 changes: 43 additions & 31 deletions drivers/of/unittest.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,9 @@ static void __init of_unittest_parse_phandle_with_args(void)

unittest(passed, "index %i - data error on node %pOF rc=%i\n",
i, args.np, rc);

if (rc == 0)
of_node_put(args.np);
}

/* Check for missing list property */
Expand Down Expand Up @@ -545,8 +548,9 @@ static void __init of_unittest_parse_phandle_with_args(void)

static void __init of_unittest_parse_phandle_with_args_map(void)
{
struct device_node *np, *p0, *p1, *p2, *p3;
struct device_node *np, *p[6] = {};
struct of_phandle_args args;
unsigned int prefs[6];
int i, rc;

np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-b");
Expand All @@ -555,34 +559,24 @@ static void __init of_unittest_parse_phandle_with_args_map(void)
return;
}

p0 = of_find_node_by_path("/testcase-data/phandle-tests/provider0");
if (!p0) {
pr_err("missing testcase data\n");
return;
}

p1 = of_find_node_by_path("/testcase-data/phandle-tests/provider1");
if (!p1) {
pr_err("missing testcase data\n");
return;
}

p2 = of_find_node_by_path("/testcase-data/phandle-tests/provider2");
if (!p2) {
pr_err("missing testcase data\n");
return;
}

p3 = of_find_node_by_path("/testcase-data/phandle-tests/provider3");
if (!p3) {
pr_err("missing testcase data\n");
return;
p[0] = of_find_node_by_path("/testcase-data/phandle-tests/provider0");
p[1] = of_find_node_by_path("/testcase-data/phandle-tests/provider1");
p[2] = of_find_node_by_path("/testcase-data/phandle-tests/provider2");
p[3] = of_find_node_by_path("/testcase-data/phandle-tests/provider3");
p[4] = of_find_node_by_path("/testcase-data/phandle-tests/provider4");
p[5] = of_find_node_by_path("/testcase-data/phandle-tests/provider5");
for (i = 0; i < ARRAY_SIZE(p); ++i) {
if (!p[i]) {
pr_err("missing testcase data\n");
return;
}
prefs[i] = kref_read(&p[i]->kobj.kref);
}

rc = of_count_phandle_with_args(np, "phandle-list", "#phandle-cells");
unittest(rc == 7, "of_count_phandle_with_args() returned %i, expected 7\n", rc);
unittest(rc == 8, "of_count_phandle_with_args() returned %i, expected 7\n", rc);

for (i = 0; i < 8; i++) {
for (i = 0; i < 9; i++) {
bool passed = true;

memset(&args, 0, sizeof(args));
Expand All @@ -593,13 +587,13 @@ static void __init of_unittest_parse_phandle_with_args_map(void)
switch (i) {
case 0:
passed &= !rc;
passed &= (args.np == p1);
passed &= (args.np == p[1]);
passed &= (args.args_count == 1);
passed &= (args.args[0] == 1);
break;
case 1:
passed &= !rc;
passed &= (args.np == p3);
passed &= (args.np == p[3]);
passed &= (args.args_count == 3);
passed &= (args.args[0] == 2);
passed &= (args.args[1] == 5);
Expand All @@ -610,28 +604,36 @@ static void __init of_unittest_parse_phandle_with_args_map(void)
break;
case 3:
passed &= !rc;
passed &= (args.np == p0);
passed &= (args.np == p[0]);
passed &= (args.args_count == 0);
break;
case 4:
passed &= !rc;
passed &= (args.np == p1);
passed &= (args.np == p[1]);
passed &= (args.args_count == 1);
passed &= (args.args[0] == 3);
break;
case 5:
passed &= !rc;
passed &= (args.np == p0);
passed &= (args.np == p[0]);
passed &= (args.args_count == 0);
break;
case 6:
passed &= !rc;
passed &= (args.np == p2);
passed &= (args.np == p[2]);
passed &= (args.args_count == 2);
passed &= (args.args[0] == 15);
passed &= (args.args[1] == 0x20);
break;
case 7:
passed &= !rc;
passed &= (args.np == p[3]);
passed &= (args.args_count == 3);
passed &= (args.args[0] == 2);
passed &= (args.args[1] == 5);
passed &= (args.args[2] == 3);
break;
case 8:
passed &= (rc == -ENOENT);
break;
default:
Expand All @@ -640,6 +642,9 @@ static void __init of_unittest_parse_phandle_with_args_map(void)

unittest(passed, "index %i - data error on node %s rc=%i\n",
i, args.np->full_name, rc);

if (rc == 0)
of_node_put(args.np);
}

/* Check for missing list property */
Expand Down Expand Up @@ -686,6 +691,13 @@ static void __init of_unittest_parse_phandle_with_args_map(void)
"OF: /testcase-data/phandle-tests/consumer-b: #phandle-cells = 2 found 1");

unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);

for (i = 0; i < ARRAY_SIZE(p); ++i) {
unittest(prefs[i] == kref_read(&p[i]->kobj.kref),
"provider%d: expected:%d got:%d\n",
i, prefs[i], kref_read(&p[i]->kobj.kref));
of_node_put(p[i]);
}
}

static void __init of_unittest_property_string(void)
Expand Down

0 comments on commit 4dde835

Please sign in to comment.