Skip to content

Commit 4dde835

Browse files
Christian A. Ehrhardtrobherring
Christian A. Ehrhardt
authored and
robherring
committed
of: Fix double free in of_parse_phandle_with_args_map
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>
1 parent 5e3ef45 commit 4dde835

File tree

3 files changed

+53
-32
lines changed

3 files changed

+53
-32
lines changed

‎drivers/of/base.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,6 +1464,7 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
14641464
out_args->np = new;
14651465
of_node_put(cur);
14661466
cur = new;
1467+
new = NULL;
14671468
}
14681469
put:
14691470
of_node_put(cur);

‎drivers/of/unittest-data/tests-phandle.dtsi‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@
4040
phandle-map-pass-thru = <0x0 0xf0>;
4141
};
4242

43+
provider5: provider5 {
44+
#phandle-cells = <2>;
45+
phandle-map = <2 7 &provider4 2 3>;
46+
phandle-map-mask = <0xff 0xf>;
47+
phandle-map-pass-thru = <0x0 0xf0>;
48+
};
49+
4350
consumer-a {
4451
phandle-list = <&provider1 1>,
4552
<&provider2 2 0>,
@@ -66,7 +73,8 @@
6673
<&provider4 4 0x100>,
6774
<&provider4 0 0x61>,
6875
<&provider0>,
69-
<&provider4 19 0x20>;
76+
<&provider4 19 0x20>,
77+
<&provider5 2 7>;
7078
phandle-list-bad-phandle = <12345678 0 0>;
7179
phandle-list-bad-args = <&provider2 1 0>,
7280
<&provider4 0>;

‎drivers/of/unittest.c‎

Lines changed: 43 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,9 @@ static void __init of_unittest_parse_phandle_with_args(void)
456456

457457
unittest(passed, "index %i - data error on node %pOF rc=%i\n",
458458
i, args.np, rc);
459+
460+
if (rc == 0)
461+
of_node_put(args.np);
459462
}
460463

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

546549
static void __init of_unittest_parse_phandle_with_args_map(void)
547550
{
548-
struct device_node *np, *p0, *p1, *p2, *p3;
551+
struct device_node *np, *p[6] = {};
549552
struct of_phandle_args args;
553+
unsigned int prefs[6];
550554
int i, rc;
551555

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

558-
p0 = of_find_node_by_path("/testcase-data/phandle-tests/provider0");
559-
if (!p0) {
560-
pr_err("missing testcase data\n");
561-
return;
562-
}
563-
564-
p1 = of_find_node_by_path("/testcase-data/phandle-tests/provider1");
565-
if (!p1) {
566-
pr_err("missing testcase data\n");
567-
return;
568-
}
569-
570-
p2 = of_find_node_by_path("/testcase-data/phandle-tests/provider2");
571-
if (!p2) {
572-
pr_err("missing testcase data\n");
573-
return;
574-
}
575-
576-
p3 = of_find_node_by_path("/testcase-data/phandle-tests/provider3");
577-
if (!p3) {
578-
pr_err("missing testcase data\n");
579-
return;
562+
p[0] = of_find_node_by_path("/testcase-data/phandle-tests/provider0");
563+
p[1] = of_find_node_by_path("/testcase-data/phandle-tests/provider1");
564+
p[2] = of_find_node_by_path("/testcase-data/phandle-tests/provider2");
565+
p[3] = of_find_node_by_path("/testcase-data/phandle-tests/provider3");
566+
p[4] = of_find_node_by_path("/testcase-data/phandle-tests/provider4");
567+
p[5] = of_find_node_by_path("/testcase-data/phandle-tests/provider5");
568+
for (i = 0; i < ARRAY_SIZE(p); ++i) {
569+
if (!p[i]) {
570+
pr_err("missing testcase data\n");
571+
return;
572+
}
573+
prefs[i] = kref_read(&p[i]->kobj.kref);
580574
}
581575

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

585-
for (i = 0; i < 8; i++) {
579+
for (i = 0; i < 9; i++) {
586580
bool passed = true;
587581

588582
memset(&args, 0, sizeof(args));
@@ -593,13 +587,13 @@ static void __init of_unittest_parse_phandle_with_args_map(void)
593587
switch (i) {
594588
case 0:
595589
passed &= !rc;
596-
passed &= (args.np == p1);
590+
passed &= (args.np == p[1]);
597591
passed &= (args.args_count == 1);
598592
passed &= (args.args[0] == 1);
599593
break;
600594
case 1:
601595
passed &= !rc;
602-
passed &= (args.np == p3);
596+
passed &= (args.np == p[3]);
603597
passed &= (args.args_count == 3);
604598
passed &= (args.args[0] == 2);
605599
passed &= (args.args[1] == 5);
@@ -610,28 +604,36 @@ static void __init of_unittest_parse_phandle_with_args_map(void)
610604
break;
611605
case 3:
612606
passed &= !rc;
613-
passed &= (args.np == p0);
607+
passed &= (args.np == p[0]);
614608
passed &= (args.args_count == 0);
615609
break;
616610
case 4:
617611
passed &= !rc;
618-
passed &= (args.np == p1);
612+
passed &= (args.np == p[1]);
619613
passed &= (args.args_count == 1);
620614
passed &= (args.args[0] == 3);
621615
break;
622616
case 5:
623617
passed &= !rc;
624-
passed &= (args.np == p0);
618+
passed &= (args.np == p[0]);
625619
passed &= (args.args_count == 0);
626620
break;
627621
case 6:
628622
passed &= !rc;
629-
passed &= (args.np == p2);
623+
passed &= (args.np == p[2]);
630624
passed &= (args.args_count == 2);
631625
passed &= (args.args[0] == 15);
632626
passed &= (args.args[1] == 0x20);
633627
break;
634628
case 7:
629+
passed &= !rc;
630+
passed &= (args.np == p[3]);
631+
passed &= (args.args_count == 3);
632+
passed &= (args.args[0] == 2);
633+
passed &= (args.args[1] == 5);
634+
passed &= (args.args[2] == 3);
635+
break;
636+
case 8:
635637
passed &= (rc == -ENOENT);
636638
break;
637639
default:
@@ -640,6 +642,9 @@ static void __init of_unittest_parse_phandle_with_args_map(void)
640642

641643
unittest(passed, "index %i - data error on node %s rc=%i\n",
642644
i, args.np->full_name, rc);
645+
646+
if (rc == 0)
647+
of_node_put(args.np);
643648
}
644649

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

688693
unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
694+
695+
for (i = 0; i < ARRAY_SIZE(p); ++i) {
696+
unittest(prefs[i] == kref_read(&p[i]->kobj.kref),
697+
"provider%d: expected:%d got:%d\n",
698+
i, prefs[i], kref_read(&p[i]->kobj.kref));
699+
of_node_put(p[i]);
700+
}
689701
}
690702

691703
static void __init of_unittest_property_string(void)

0 commit comments

Comments
 (0)