diff mbox

powerpc/pseries: Add null property check to pseries_discover_pic()

Message ID 1461822825-21542-1-git-send-email-sjitindarsingh@gmail.com (mailing list archive)
State Accepted
Headers show

Commit Message

Suraj Jitindar Singh April 28, 2016, 5:53 a.m. UTC
The return value of of_get_property() isn't checked before it is passed
to the strstr() function, if it happens that the return value is null
then this will result in a null pointer being dereferenced.

Add a check to see if the return value of of_get_property() is null and
if it is continue straight on to the next node.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 arch/powerpc/platforms/pseries/setup.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Chris Smart May 2, 2016, 2:30 a.m. UTC | #1
On Thu, Apr 28, 2016 at 03:53:45PM +1000, Suraj Jitindar Singh wrote:
>The return value of of_get_property() isn't checked before it is passed
>to the strstr() function, if it happens that the return value is null
>then this will result in a null pointer being dereferenced.
>
>Add a check to see if the return value of of_get_property() is null and
>if it is continue straight on to the next node.
>

Hi Suraj,

Presumably machines have been shipping with a device tree that contains
"compatible" in the interrupt controller section and so we just happen
to have never hit this. Good thing to check.

Confirming that strstr calls strlen on the first argument, which would
dereference a null pointer.

Reviewed-by: Chris Smart <chris@distroguy.com>

>Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>---
> arch/powerpc/platforms/pseries/setup.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
>index 6e944fc..fa73494 100644
>--- a/arch/powerpc/platforms/pseries/setup.c
>+++ b/arch/powerpc/platforms/pseries/setup.c
>@@ -235,6 +235,8 @@ static void __init pseries_discover_pic(void)
>
> 	for_each_node_by_name(np, "interrupt-controller") {
> 		typep = of_get_property(np, "compatible", NULL);
>+		if (!typep)
>+			continue;
> 		if (strstr(typep, "open-pic")) {
> 			pSeries_mpic_node = of_node_get(np);
> 			ppc_md.init_IRQ       = pseries_mpic_init_IRQ;
>-- 
>2.5.0
>
>_______________________________________________
>Linuxppc-dev mailing list
>Linuxppc-dev@lists.ozlabs.org
>https://lists.ozlabs.org/listinfo/linuxppc-dev
Michael Ellerman May 4, 2016, 10:40 p.m. UTC | #2
On Thu, 2016-28-04 at 05:53:45 UTC, Suraj Jitindar Singh wrote:
> The return value of of_get_property() isn't checked before it is passed
> to the strstr() function, if it happens that the return value is null
> then this will result in a null pointer being dereferenced.
> 
> Add a check to see if the return value of of_get_property() is null and
> if it is continue straight on to the next node.
> 
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> Reviewed-by: Chris Smart <chris@distroguy.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/9ed3f7bcab00c9477e382e6589

cheers
diff mbox

Patch

diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 6e944fc..fa73494 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -235,6 +235,8 @@  static void __init pseries_discover_pic(void)
 
 	for_each_node_by_name(np, "interrupt-controller") {
 		typep = of_get_property(np, "compatible", NULL);
+		if (!typep)
+			continue;
 		if (strstr(typep, "open-pic")) {
 			pSeries_mpic_node = of_node_get(np);
 			ppc_md.init_IRQ       = pseries_mpic_init_IRQ;