* [PATCH] Fix get_node_by_path string equality check
@ 2017-04-18 0:08 Tim Montague
[not found] ` <f5424c7ce8334730a64873b9912132d3-xbOadlNgr96L7B8feK0ILA@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Tim Montague @ 2017-04-18 0:08 UTC (permalink / raw)
To: Rob Herring; +Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA
When determining if to recurse into a node, get_node_by_path does not
check if the length of each node name is equal. If searching for
/foo/baz, this can result in recursing into /food because
strneq("foo", "food", 3) is true.
Signed-off-by: Tim Montague <tmontague@ghs.com>
---
livetree.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/livetree.c b/livetree.c
index 3673de0..aecd278 100644
--- a/livetree.c
+++ b/livetree.c
@@ -478,7 +478,8 @@ struct node *get_node_by_path(struct node *tree, const char *path)
p = strchr(path, '/');
for_each_child(tree, child) {
- if (p && strneq(path, child->name, p-path))
+ if (p && (strlen(child->name) == p-path) &&
+ strneq(path, child->name, p-path))
return get_node_by_path(child, p+1);
else if (!p && streq(path, child->name))
return child;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix get_node_by_path string equality check
[not found] ` <f5424c7ce8334730a64873b9912132d3-xbOadlNgr96L7B8feK0ILA@public.gmane.org>
@ 2017-04-18 2:05 ` David Gibson
[not found] ` <20170418020540.GA12235-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: David Gibson @ 2017-04-18 2:05 UTC (permalink / raw)
To: Tim Montague; +Cc: Rob Herring, devicetree-compiler-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 1339 bytes --]
On Tue, Apr 18, 2017 at 12:08:18AM +0000, Tim Montague wrote:
> When determining if to recurse into a node, get_node_by_path does not
> check if the length of each node name is equal. If searching for
> /foo/baz, this can result in recursing into /food because
> strneq("foo", "food", 3) is true.
>
> Signed-off-by: Tim Montague <tmontague-5j0BnJlmnLM@public.gmane.org>
Looks like I made the classic strncmp() blunder, how embarrassing.
Fix looks good, but I could trouble you to add a testcase exercising
this?
> ---
> livetree.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/livetree.c b/livetree.c
> index 3673de0..aecd278 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -478,7 +478,8 @@ struct node *get_node_by_path(struct node *tree, const char *path)
> p = strchr(path, '/');
>
> for_each_child(tree, child) {
> - if (p && strneq(path, child->name, p-path))
> + if (p && (strlen(child->name) == p-path) &&
> + strneq(path, child->name, p-path))
> return get_node_by_path(child, p+1);
> else if (!p && streq(path, child->name))
> return child;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] Fix get_node_by_path string equality check
[not found] ` <20170418020540.GA12235-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
@ 2017-04-18 3:18 ` Tim Montague
[not found] ` <77edbc7797e144cd9a635e5b1d0b2dfe-xbOadlNgr96L7B8feK0ILA@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Tim Montague @ 2017-04-18 3:18 UTC (permalink / raw)
To: David Gibson; +Cc: Rob Herring, devicetree-compiler-u79uwXL29TY76Z2rM5mHXA
When determining if to recurse into a node, get_node_by_path does not
check if the length of each node name is equal. If searching for
/foo/baz, this can result in recursing into /foobar because
strneq("foo", "foobar", 3) is true.
This can result in a reference to /foo/baz to be incorrectly set to
/foobar/baz. A test for this was added.
Signed-off-by: Tim Montague <tmontague@ghs.com>
---
livetree.c | 3 ++-
tests/path-references.c | 12 +++++++++++-
tests/path-references.dts | 13 +++++++++++++
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/livetree.c b/livetree.c
index 3673de0..aecd278 100644
--- a/livetree.c
+++ b/livetree.c
@@ -478,7 +478,8 @@ struct node *get_node_by_path(struct node *tree, const char *path)
p = strchr(path, '/');
for_each_child(tree, child) {
- if (p && strneq(path, child->name, p-path))
+ if (p && (strlen(child->name) == p-path) &&
+ strneq(path, child->name, p-path))
return get_node_by_path(child, p+1);
else if (!p && streq(path, child->name))
return child;
diff --git a/tests/path-references.c b/tests/path-references.c
index c8d25fb..5e332e8 100644
--- a/tests/path-references.c
+++ b/tests/path-references.c
@@ -66,7 +66,7 @@ int main(int argc, char *argv[])
void *fdt;
const char *p;
int len, multilen;
- int n1, n2;
+ int n1, n2, n3, n4;
test_init(argc, argv);
fdt = load_blob_arg(argc, argv);
@@ -92,6 +92,16 @@ int main(int argc, char *argv[])
if ((!streq(p, "/node1") || !streq(p + strlen("/node1") + 1, "/node2")))
FAIL("multiref has wrong value");
+ /* Check reference to nested nodes with common prefix */
+ n3 = fdt_path_offset(fdt, "/foo/baz");
+ if (n3 < 0)
+ FAIL("fdt_path_offset(/foo/baz): %s", fdt_strerror(n3));
+ n4 = fdt_path_offset(fdt, "/foobar/baz");
+ if (n4 < 0)
+ FAIL("fdt_path_offset(/foobar/baz): %s", fdt_strerror(n4));
+ check_ref(fdt, n3, "/foobar/baz");
+ check_ref(fdt, n4, "/foo/baz");
+
check_rref(fdt);
PASS();
diff --git a/tests/path-references.dts b/tests/path-references.dts
index b00fd79..3808aa6 100644
--- a/tests/path-references.dts
+++ b/tests/path-references.dts
@@ -12,4 +12,17 @@
ref = &{/node1}; /* reference after target */
lref = &n1;
};
+ /* Check references to nested nodes with common prefix */
+ foobar {
+ n3: baz {
+ ref = &{/foo/baz};
+ lref = &n4;
+ };
+ };
+ foo {
+ n4: baz {
+ ref = &{/foobar/baz};
+ lref = &n3;
+ };
+ };
};
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix get_node_by_path string equality check
[not found] ` <77edbc7797e144cd9a635e5b1d0b2dfe-xbOadlNgr96L7B8feK0ILA@public.gmane.org>
@ 2017-04-18 3:43 ` David Gibson
[not found] ` <20170418034332.GH12235-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: David Gibson @ 2017-04-18 3:43 UTC (permalink / raw)
To: Tim Montague; +Cc: Rob Herring, devicetree-compiler-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 4340 bytes --]
On Tue, Apr 18, 2017 at 03:18:55AM +0000, Tim Montague wrote:
> When determining if to recurse into a node, get_node_by_path does not
> check if the length of each node name is equal. If searching for
> /foo/baz, this can result in recursing into /foobar because
> strneq("foo", "foobar", 3) is true.
>
> This can result in a reference to /foo/baz to be incorrectly set to
> /foobar/baz. A test for this was added.
>
> Signed-off-by: Tim Montague <tmontague-5j0BnJlmnLM@public.gmane.org>
Ah.. it looks like your mailer has mangled the patch, I'm afraid:
$ git am -s ~/foobox/
Applying: Fix get_node_by_path string equality check
.git/rebase-apply/patch:16: trailing whitespace.
if (p && (strlen(child->name) == p-path) &&
.git/rebase-apply/patch:17: trailing whitespace.
strneq(path, child->name, p-path))
.git/rebase-apply/patch:30: trailing whitespace.
int n1, n2, n3, n4;
.git/rebase-apply/patch:38: trailing whitespace.
/* Check reference to nested nodes with common prefix */
.git/rebase-apply/patch:39: trailing whitespace.
n3 = fdt_path_offset(fdt, "/foo/baz");
error: patch failed: livetree.c:478
error: livetree.c: patch does not apply
error: patch failed: tests/path-references.c:66
error: tests/path-references.c: patch does not apply
error: patch failed: tests/path-references.dts:12
error: tests/path-references.dts: patch does not apply
Patch failed at 0001 Fix get_node_by_path string equality check
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Can you check your mailer settings. If it's not clear how to stop it
doing that, it's probably safe to attach the patch instead.
> ---
> livetree.c | 3 ++-
> tests/path-references.c | 12 +++++++++++-
> tests/path-references.dts | 13 +++++++++++++
> 3 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/livetree.c b/livetree.c
> index 3673de0..aecd278 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -478,7 +478,8 @@ struct node *get_node_by_path(struct node *tree, const char *path)
> p = strchr(path, '/');
>
> for_each_child(tree, child) {
> - if (p && strneq(path, child->name, p-path))
> + if (p && (strlen(child->name) == p-path) &&
> + strneq(path, child->name, p-path))
> return get_node_by_path(child, p+1);
> else if (!p && streq(path, child->name))
> return child;
> diff --git a/tests/path-references.c b/tests/path-references.c
> index c8d25fb..5e332e8 100644
> --- a/tests/path-references.c
> +++ b/tests/path-references.c
> @@ -66,7 +66,7 @@ int main(int argc, char *argv[])
> void *fdt;
> const char *p;
> int len, multilen;
> - int n1, n2;
> + int n1, n2, n3, n4;
>
> test_init(argc, argv);
> fdt = load_blob_arg(argc, argv);
> @@ -92,6 +92,16 @@ int main(int argc, char *argv[])
> if ((!streq(p, "/node1") || !streq(p + strlen("/node1") + 1, "/node2")))
> FAIL("multiref has wrong value");
>
> + /* Check reference to nested nodes with common prefix */
> + n3 = fdt_path_offset(fdt, "/foo/baz");
> + if (n3 < 0)
> + FAIL("fdt_path_offset(/foo/baz): %s", fdt_strerror(n3));
> + n4 = fdt_path_offset(fdt, "/foobar/baz");
> + if (n4 < 0)
> + FAIL("fdt_path_offset(/foobar/baz): %s", fdt_strerror(n4));
> + check_ref(fdt, n3, "/foobar/baz");
> + check_ref(fdt, n4, "/foo/baz");
> +
> check_rref(fdt);
>
> PASS();
> diff --git a/tests/path-references.dts b/tests/path-references.dts
> index b00fd79..3808aa6 100644
> --- a/tests/path-references.dts
> +++ b/tests/path-references.dts
> @@ -12,4 +12,17 @@
> ref = &{/node1}; /* reference after target */
> lref = &n1;
> };
> + /* Check references to nested nodes with common prefix */
> + foobar {
> + n3: baz {
> + ref = &{/foo/baz};
> + lref = &n4;
> + };
> + };
> + foo {
> + n4: baz {
> + ref = &{/foobar/baz};
> + lref = &n3;
> + };
> + };
> };
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] Fix get_node_by_path string equality check
[not found] ` <20170418034332.GH12235-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
@ 2017-04-19 1:15 ` Tim Montague
[not found] ` <7417379a9afc4272b5b62b8ff4e50719-xbOadlNgr96L7B8feK0ILA@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Tim Montague @ 2017-04-19 1:15 UTC (permalink / raw)
To: David Gibson; +Cc: Rob Herring, devicetree-compiler-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]
> Ah.. it looks like your mailer has mangled the patch, I'm afraid:
>
> $ git am -s ~/foobox/
> Applying: Fix get_node_by_path string equality check
> .git/rebase-apply/patch:16: trailing whitespace.
> if (p && (strlen(child->name) == p-path) &&
> .git/rebase-apply/patch:17: trailing whitespace.
> strneq(path, child->name, p-path))
> .git/rebase-apply/patch:30: trailing whitespace.
> int n1, n2, n3, n4;
> .git/rebase-apply/patch:38: trailing whitespace.
> /* Check reference to nested nodes with common prefix */
> .git/rebase-apply/patch:39: trailing whitespace.
> n3 = fdt_path_offset(fdt, "/foo/baz");
> error: patch failed: livetree.c:478
> error: livetree.c: patch does not apply
> error: patch failed: tests/path-references.c:66
> error: tests/path-references.c: patch does not apply
> error: patch failed: tests/path-references.dts:12
> error: tests/path-references.dts: patch does not apply
> Patch failed at 0001 Fix get_node_by_path string equality check
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
> Can you check your mailer settings. If it's not clear how to stop it
> doing that, it's probably safe to attach the patch instead.
Sorry about that, Outlook doesn't always behave. The patch is attached.
-Tim
[-- Attachment #2: 0001-Fix-get_node_by_path-string-equality-check.patch --]
[-- Type: application/octet-stream, Size: 2657 bytes --]
From 2bdd399513498a65913996638fdfd59c07704db8 Mon Sep 17 00:00:00 2001
From: Tim Montague <tmontague@ghs.com>
Date: Mon, 17 Apr 2017 16:51:05 -0700
Subject: [PATCH] Fix get_node_by_path string equality check
When determining if to recurse into a node, get_node_by_path does not
check if the length of each node name is equal. If searching for
/foo/baz, this can result in recursing into /foobar because
strneq("foo", "foobar", 3) is true.
This can result in a reference to /foo/baz to be incorrectly set to
/foobar/baz. A test for this was added.
Signed-off-by: Tim Montague <tmontague@ghs.com>
---
livetree.c | 3 ++-
tests/path-references.c | 12 +++++++++++-
tests/path-references.dts | 13 +++++++++++++
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/livetree.c b/livetree.c
index 3673de0..aecd278 100644
--- a/livetree.c
+++ b/livetree.c
@@ -478,7 +478,8 @@ struct node *get_node_by_path(struct node *tree, const char *path)
p = strchr(path, '/');
for_each_child(tree, child) {
- if (p && strneq(path, child->name, p-path))
+ if (p && (strlen(child->name) == p-path) &&
+ strneq(path, child->name, p-path))
return get_node_by_path(child, p+1);
else if (!p && streq(path, child->name))
return child;
diff --git a/tests/path-references.c b/tests/path-references.c
index c8d25fb..5e332e8 100644
--- a/tests/path-references.c
+++ b/tests/path-references.c
@@ -66,7 +66,7 @@ int main(int argc, char *argv[])
void *fdt;
const char *p;
int len, multilen;
- int n1, n2;
+ int n1, n2, n3, n4;
test_init(argc, argv);
fdt = load_blob_arg(argc, argv);
@@ -92,6 +92,16 @@ int main(int argc, char *argv[])
if ((!streq(p, "/node1") || !streq(p + strlen("/node1") + 1, "/node2")))
FAIL("multiref has wrong value");
+ /* Check reference to nested nodes with common prefix */
+ n3 = fdt_path_offset(fdt, "/foo/baz");
+ if (n3 < 0)
+ FAIL("fdt_path_offset(/foo/baz): %s", fdt_strerror(n3));
+ n4 = fdt_path_offset(fdt, "/foobar/baz");
+ if (n4 < 0)
+ FAIL("fdt_path_offset(/foobar/baz): %s", fdt_strerror(n4));
+ check_ref(fdt, n3, "/foobar/baz");
+ check_ref(fdt, n4, "/foo/baz");
+
check_rref(fdt);
PASS();
diff --git a/tests/path-references.dts b/tests/path-references.dts
index b00fd79..8c66d80 100644
--- a/tests/path-references.dts
+++ b/tests/path-references.dts
@@ -12,4 +12,17 @@
ref = &{/node1}; /* reference after target */
lref = &n1;
};
+ /* Check references to nested nodes with common prefix */
+ foobar {
+ n3: baz {
+ ref = &{/foo/baz};
+ lref = &n4;
+ };
+ };
+ foo {
+ n4: baz {
+ ref = &{/foobar/baz};
+ lref = &n3;
+ };
+ };
};
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix get_node_by_path string equality check
[not found] ` <7417379a9afc4272b5b62b8ff4e50719-xbOadlNgr96L7B8feK0ILA@public.gmane.org>
@ 2017-04-19 1:30 ` David Gibson
0 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2017-04-19 1:30 UTC (permalink / raw)
To: Tim Montague; +Cc: Rob Herring, devicetree-compiler-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 1845 bytes --]
On Wed, Apr 19, 2017 at 01:15:08AM +0000, Tim Montague wrote:
> > Ah.. it looks like your mailer has mangled the patch, I'm afraid:
> >
> > $ git am -s ~/foobox/
> > Applying: Fix get_node_by_path string equality check
> > .git/rebase-apply/patch:16: trailing whitespace.
> > if (p && (strlen(child->name) == p-path) &&
> > .git/rebase-apply/patch:17: trailing whitespace.
> > strneq(path, child->name, p-path))
> > .git/rebase-apply/patch:30: trailing whitespace.
> > int n1, n2, n3, n4;
> > .git/rebase-apply/patch:38: trailing whitespace.
> > /* Check reference to nested nodes with common prefix */
> > .git/rebase-apply/patch:39: trailing whitespace.
> > n3 = fdt_path_offset(fdt, "/foo/baz");
> > error: patch failed: livetree.c:478
> > error: livetree.c: patch does not apply
> > error: patch failed: tests/path-references.c:66
> > error: tests/path-references.c: patch does not apply
> > error: patch failed: tests/path-references.dts:12
> > error: tests/path-references.dts: patch does not apply
> > Patch failed at 0001 Fix get_node_by_path string equality check
> > The copy of the patch that failed is found in: .git/rebase-apply/patch
> > When you have resolved this problem, run "git am --continue".
> > If you prefer to skip this patch, run "git am --skip" instead.
> > To restore the original branch and stop patching, run "git am --abort".
> >
> > Can you check your mailer settings. If it's not clear how to stop it
> > doing that, it's probably safe to attach the patch instead.
>
> Sorry about that, Outlook doesn't always behave. The patch is attached.
Thanks, applied to master.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-04-19 1:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-18 0:08 [PATCH] Fix get_node_by_path string equality check Tim Montague
[not found] ` <f5424c7ce8334730a64873b9912132d3-xbOadlNgr96L7B8feK0ILA@public.gmane.org>
2017-04-18 2:05 ` David Gibson
[not found] ` <20170418020540.GA12235-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2017-04-18 3:18 ` Tim Montague
[not found] ` <77edbc7797e144cd9a635e5b1d0b2dfe-xbOadlNgr96L7B8feK0ILA@public.gmane.org>
2017-04-18 3:43 ` David Gibson
[not found] ` <20170418034332.GH12235-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2017-04-19 1:15 ` Tim Montague
[not found] ` <7417379a9afc4272b5b62b8ff4e50719-xbOadlNgr96L7B8feK0ILA@public.gmane.org>
2017-04-19 1:30 ` David Gibson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.