Re: [1/2] powerpc/83xx: fix use-after-free in mpc831x_usb_cfg()

Re: [1/2] powerpc/83xx: fix use-after-free in mpc831x_usb_cfg()

[Markus Elfring]

> we developed a coccinelle script to detect such problems.

How do you think about to give any attribution to this development software
in your commit descriptions?


> After a period of testing, we will send it to the LMKL mailing list later.

I am also curious then on how this area will evolve further.

Regards,
Markus

Re: Coccinelle: Checking of_node_put() calls with SmPL

[Markus Elfring]

> we developed a coccinelle script to detect such problems.

Would you find the implementation of the function “dt_init_idle_driver”
suspicious according to discussed source code search patterns?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cpuidle/dt_idle_states.c?id=e9a83bd2322035ed9d7dcf35753d3f984d76c6a5#n208
https://elixir.bootlin.com/linux/v5.2/source/drivers/cpuidle/dt_idle_states.c#L208


> This script is still being improved.

Will corresponding software development challenges become more interesting?

Regards,
Markus

[PATCH 1/2] powerpc/83xx: fix use-after-free in mpc831x_usb_cfg()

[Wen Yang]

The immr_node variable is still being used after the of_node_put() call,
which may result in use-after-free.
Fix this issue by calling of_node_put() after the last usage.

Fixes: fd066e850351 ("powerpc/mpc8308: fix USB DR controller initialization")
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Cc: Scott Wood <oss@buserror.net>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Markus Elfring <Markus.Elfring@web.de>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
---
 arch/powerpc/platforms/83xx/usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/usb.c b/arch/powerpc/platforms/83xx/usb.c
index 3d247d7..19dcef5 100644
--- a/arch/powerpc/platforms/83xx/usb.c
+++ b/arch/powerpc/platforms/83xx/usb.c
@@ -158,11 +158,10 @@ int mpc831x_usb_cfg(void)
 
 	iounmap(immap);
 
-	of_node_put(immr_node);
-
 	/* Map USB SOC space */
 	ret = of_address_to_resource(np, 0, &res);
 	if (ret) {
+		of_node_put(immr_node);
 		of_node_put(np);
 		return ret;
 	}
@@ -203,6 +202,7 @@ int mpc831x_usb_cfg(void)
 
 out:
 	iounmap(usb_regs);
+	of_node_put(immr_node);
 	of_node_put(np);
 	return ret;
 }
-- 
2.9.5

Re: [1/2] powerpc/83xx: fix use-after-free in mpc831x_usb_cfg()

[Markus Elfring]

> The immr_node variable is still being used after the of_node_put() call,
> which may result in use-after-free.

Was any known source code analysis tool involved to point such
a questionable implementation detail out for further software
development considerations?

Regards,
Markus