| Message ID | 20241218024415.2494267-2-joe@pf.is.s.u-tokyo.ac.jp |
|---|---|
| State | New |
| Headers | show |
| Series | memory: tegra20-emc: fix an OF node reference bug in tegra_emc_find_node_by_ram_code() | expand |
On 18/12/2024 03:44, Joe Hattori wrote: > As of_find_node_by_name() release the reference of the argument OF node, > tegra_emc_find_node_by_ram_code() releases some OF nodes while still in > use, resulting in possible UAFs. Avoid calling of_node_put() at the end Why avoiding? Avoiding is optional, aren't you fixing some bug? > of the loop, and call of_node_get() before some calls of > of_find_node_by_name(). > > This bug was found by an experimental verification tool that I am > developing. > > Fixes: 96e5da7c8424 ("memory: tegra: Introduce Tegra20 EMC driver") > Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp> > --- > drivers/memory/tegra/tegra20-emc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c > index 7193f848d17e..fef54179ddee 100644 > --- a/drivers/memory/tegra/tegra20-emc.c > +++ b/drivers/memory/tegra/tegra20-emc.c > @@ -474,6 +474,7 @@ tegra_emc_find_node_by_ram_code(struct tegra_emc *emc) > > ram_code = tegra_read_ram_code(); > > + of_node_get(dev->of_node); > for (np = of_find_node_by_name(dev->of_node, "emc-tables"); np; > np = of_find_node_by_name(np, "emc-tables")) { > err = of_property_read_u32(np, "nvidia,ram-code", &value); > @@ -481,6 +482,7 @@ tegra_emc_find_node_by_ram_code(struct tegra_emc *emc) > struct device_node *lpddr2_np; > bool cfg_mismatches = false; > > + of_node_get(np); > lpddr2_np = of_find_node_by_name(np, "lpddr2"); > if (lpddr2_np) { > const struct lpddr2_info *info; > @@ -518,7 +520,6 @@ tegra_emc_find_node_by_ram_code(struct tegra_emc *emc) > } > > if (cfg_mismatches) { > - of_node_put(np); Anyway these two patches show that my previous advice was wrong. You add code which you immediately remove. I took v1 with tweaked commit msg. > continue; > } > } Best regards, Krzysztof
diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c index 7193f848d17e..fef54179ddee 100644 --- a/drivers/memory/tegra/tegra20-emc.c +++ b/drivers/memory/tegra/tegra20-emc.c @@ -474,6 +474,7 @@ tegra_emc_find_node_by_ram_code(struct tegra_emc *emc) ram_code = tegra_read_ram_code(); + of_node_get(dev->of_node); for (np = of_find_node_by_name(dev->of_node, "emc-tables"); np; np = of_find_node_by_name(np, "emc-tables")) { err = of_property_read_u32(np, "nvidia,ram-code", &value); @@ -481,6 +482,7 @@ tegra_emc_find_node_by_ram_code(struct tegra_emc *emc) struct device_node *lpddr2_np; bool cfg_mismatches = false; + of_node_get(np); lpddr2_np = of_find_node_by_name(np, "lpddr2"); if (lpddr2_np) { const struct lpddr2_info *info; @@ -518,7 +520,6 @@ tegra_emc_find_node_by_ram_code(struct tegra_emc *emc) } if (cfg_mismatches) { - of_node_put(np); continue; } }
As of_find_node_by_name() release the reference of the argument OF node, tegra_emc_find_node_by_ram_code() releases some OF nodes while still in use, resulting in possible UAFs. Avoid calling of_node_put() at the end of the loop, and call of_node_get() before some calls of of_find_node_by_name(). This bug was found by an experimental verification tool that I am developing. Fixes: 96e5da7c8424 ("memory: tegra: Introduce Tegra20 EMC driver") Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp> --- drivers/memory/tegra/tegra20-emc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)