pmdomain: core: Fix error checking in genpd_dev_pm_attach_by_id()
Subsystems (2)
PATCH 1/1 pmdomain: core: Fix error checking in genpd_dev_pm_attach_by_id()
The error checking for of_count_phandle_with_args() does not handle negative error codes correctly. The problem is that "index" is a u32 so in the condition "if (index >= num_domains)" negative error codes stored in "num_domains" are type promoted to very high positive values and "index" is always going to be valid.
Test for negative error codes first and then test if "index" is valid.
--- drivers/pmdomain/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pmdomain/core.c +1 -1
@@ -3176,7 +3176,7 @@ struct device *genpd_dev_pm_attach_by_id(struct device *dev,/* Verify that the index is within a valid range. */ num_domains =of_count_phandle_with_args (dev->of_node,"power-domains" ,"#power-domain-cells" ); -if (index >= num_domains) +if (num_domains <0 || index >= num_domains)return NULL ;/* Allocate and register device on the genpd bus. */
Signature · 1 line
2.47.2
Quoted 11 lines · Dan Carpenter
The error checking for
of_count_phandle_with_args()does not handle negative error codes correctly. The problem is that "index" is a u32 so in the condition "if (index >=num_domains)" negative error codes stored in "num_domains" are type promoted to very high positive values and "index" is always going to be valid.Test for negative error codes first and then test if "index" is valid.
Fixes 3ccf3f0cd197 ("PM / Domains: Enable genpd_dev_pm_attach_by_id|name() for single PM domain")Signed-off-by Dan Carpenter <[email protected]>
Thanks for the fix! It looks correct to me!
What puzzles me though, if this is a real problem I am sure we would have been receiving bug reports, don't you think?
Kind regards
Uffe
Quoted 20 lines
--- drivers/pmdomain/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/pmdomain/core.c +1
-2@@ -3176,7 +3176,7 @@ struct device *genpd_dev_pm_attach_by_id(struct device *dev,/* Verify that the index is within a valid range. */ num_domains =of_count_phandle_with_args (dev->of_node,"power-domains" ,"#power-domain-cells" ); -if (index >= num_domains) +if (num_domains <0 || index >= num_domains)return NULL ;/* Allocate and register device on the genpd bus. */ --2.47.2
Quoted 10 lines · Ulf Hansson
Quoted 9 lines · Dan Carpenter
The error checking for
of_count_phandle_with_args()does not handle negative error codes correctly. The problem is that "index" is a u32 so in the condition "if (index >=num_domains)" negative error codes stored in "num_domains" are type promoted to very high positive values and "index" is always going to be valid.Test for negative error codes first and then test if "index" is valid.
Quoted 8 lines
Quoted 2 lines
Fixes 3ccf3f0cd197 ("PM / Domains: Enable genpd_dev_pm_attach_by_id|name() for single PM domain")Signed-off-by Dan Carpenter <[email protected]>Thanks for the fix! It looks correct to me!
What puzzles me though, if this is a real problem I am sure we would have been receiving bug reports, don't you think?
I think it would only cause an issue for invalid devicetrees? So it's probably not an issue people often see in real life.
regards,
dan carpenter
Quoted 15 lines · Dan Carpenter
Quoted 13 lines · Ulf Hansson
Quoted 11 lines · Dan Carpenter
The error checking for
of_count_phandle_with_args()does not handle negative error codes correctly. The problem is that "index" is a u32 so in the condition "if (index >=num_domains)" negative error codes stored in "num_domains" are type promoted to very high positive values and "index" is always going to be valid.Test for negative error codes first and then test if "index" is valid.
Fixes 3ccf3f0cd197 ("PM / Domains: Enable genpd_dev_pm_attach_by_id|name() for single PM domain")Signed-off-by Dan Carpenter <[email protected]>
Quoted 8 lines
Quoted 5 lines
Thanks for the fix! It looks correct to me!
What puzzles me though, if this is a real problem I am sure we would have been receiving bug reports, don't you think?
I think it would only cause an issue for invalid devicetrees? So it's probably not an issue people often see in real life.
Yes, you are probably right.
Anyway, I have applied this for fixes and added a stable tag.
Thanks!
Uffe