All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] irqdomain: Prevent Oops in irq_domain_push_irq()
@ 2017-08-25 12:14 ` Dan Carpenter
  0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2017-08-25 12:14 UTC (permalink / raw)
  To: Marc Zyngier, David Daney; +Cc: Thomas Gleixner, linux-kernel, kernel-janitors

This code generates a Smatch warning:

	kernel/irq/irqdomain.c:1511 irq_domain_push_irq()
	warn: variable dereferenced before check 'root_irq_data' (see line 1508)

irq_get_irq_data() does sometimes return NULL pointers so this seems
like a real bug.  Let's fix this bug by moving the check for NULL
earlier.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Redo changelog.
v3: Redo changelog again.  Make it imperative.

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index b9c688944429..e84b7056bb08 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -1505,10 +1505,10 @@ int irq_domain_push_irq(struct irq_domain *domain, int virq, void *arg)
 	if (WARN_ON(!irq_domain_is_hierarchy(domain)))
 		return -EINVAL;
 
-	if (domain->parent != root_irq_data->domain)
+	if (!root_irq_data)
 		return -EINVAL;
 
-	if (!root_irq_data)
+	if (domain->parent != root_irq_data->domain)
 		return -EINVAL;
 
 	child_irq_data = kzalloc_node(sizeof(*child_irq_data), GFP_KERNEL,

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH v3] irqdomain: Prevent Oops in irq_domain_push_irq()
@ 2017-08-25 12:14 ` Dan Carpenter
  0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2017-08-25 12:14 UTC (permalink / raw)
  To: Marc Zyngier, David Daney; +Cc: Thomas Gleixner, linux-kernel, kernel-janitors

This code generates a Smatch warning:

	kernel/irq/irqdomain.c:1511 irq_domain_push_irq()
	warn: variable dereferenced before check 'root_irq_data' (see line 1508)

irq_get_irq_data() does sometimes return NULL pointers so this seems
like a real bug.  Let's fix this bug by moving the check for NULL
earlier.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Redo changelog.
v3: Redo changelog again.  Make it imperative.

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index b9c688944429..e84b7056bb08 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -1505,10 +1505,10 @@ int irq_domain_push_irq(struct irq_domain *domain, int virq, void *arg)
 	if (WARN_ON(!irq_domain_is_hierarchy(domain)))
 		return -EINVAL;
 
-	if (domain->parent != root_irq_data->domain)
+	if (!root_irq_data)
 		return -EINVAL;
 
-	if (!root_irq_data)
+	if (domain->parent != root_irq_data->domain)
 		return -EINVAL;
 
 	child_irq_data = kzalloc_node(sizeof(*child_irq_data), GFP_KERNEL,

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH v3] irqdomain: Prevent Oops in irq_domain_push_irq()
  2017-08-25 12:14 ` Dan Carpenter
@ 2017-08-25 16:09   ` David Daney
  -1 siblings, 0 replies; 5+ messages in thread
From: David Daney @ 2017-08-25 16:09 UTC (permalink / raw)
  To: Dan Carpenter, Marc Zyngier, David Daney
  Cc: Thomas Gleixner, linux-kernel, kernel-janitors

On 08/25/2017 05:14 AM, Dan Carpenter wrote:
> This code generates a Smatch warning:
> 
> 	kernel/irq/irqdomain.c:1511 irq_domain_push_irq()
> 	warn: variable dereferenced before check 'root_irq_data' (see line 1508)
> 
> irq_get_irq_data() does sometimes return NULL pointers so this seems
> like a real bug.  Let's fix this bug by moving the check for NULL
> earlier.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Thanks for identifying and fixing this.  It looks plausible, so if it 
compiles without error you can add:

Acked-by: David Daney <david.daney@cavium.com>

> ---
> v2: Redo changelog.
> v3: Redo changelog again.  Make it imperative.
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index b9c688944429..e84b7056bb08 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -1505,10 +1505,10 @@ int irq_domain_push_irq(struct irq_domain *domain, int virq, void *arg)
>   	if (WARN_ON(!irq_domain_is_hierarchy(domain)))
>   		return -EINVAL;
>   
> -	if (domain->parent != root_irq_data->domain)
> +	if (!root_irq_data)
>   		return -EINVAL;
>   
> -	if (!root_irq_data)
> +	if (domain->parent != root_irq_data->domain)
>   		return -EINVAL;
>   
>   	child_irq_data = kzalloc_node(sizeof(*child_irq_data), GFP_KERNEL,
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH v3] irqdomain: Prevent Oops in irq_domain_push_irq()
@ 2017-08-25 16:09   ` David Daney
  0 siblings, 0 replies; 5+ messages in thread
From: David Daney @ 2017-08-25 16:09 UTC (permalink / raw)
  To: Dan Carpenter, Marc Zyngier, David Daney
  Cc: Thomas Gleixner, linux-kernel, kernel-janitors

On 08/25/2017 05:14 AM, Dan Carpenter wrote:
> This code generates a Smatch warning:
> 
> 	kernel/irq/irqdomain.c:1511 irq_domain_push_irq()
> 	warn: variable dereferenced before check 'root_irq_data' (see line 1508)
> 
> irq_get_irq_data() does sometimes return NULL pointers so this seems
> like a real bug.  Let's fix this bug by moving the check for NULL
> earlier.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Thanks for identifying and fixing this.  It looks plausible, so if it 
compiles without error you can add:

Acked-by: David Daney <david.daney@cavium.com>

> ---
> v2: Redo changelog.
> v3: Redo changelog again.  Make it imperative.
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index b9c688944429..e84b7056bb08 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -1505,10 +1505,10 @@ int irq_domain_push_irq(struct irq_domain *domain, int virq, void *arg)
>   	if (WARN_ON(!irq_domain_is_hierarchy(domain)))
>   		return -EINVAL;
>   
> -	if (domain->parent != root_irq_data->domain)
> +	if (!root_irq_data)
>   		return -EINVAL;
>   
> -	if (!root_irq_data)
> +	if (domain->parent != root_irq_data->domain)
>   		return -EINVAL;
>   
>   	child_irq_data = kzalloc_node(sizeof(*child_irq_data), GFP_KERNEL,
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread
* [tip:irq/core] irqdomain: Prevent potential NULL pointer dereference in irq_domain_push_irq()
  2017-08-25 12:14 ` Dan Carpenter
  (?)
  (?)
@ 2017-08-25 20:43 ` tip-bot for Dan Carpenter
  -1 siblings, 0 replies; 5+ messages in thread
From: tip-bot for Dan Carpenter @ 2017-08-25 20:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, dan.carpenter, linux-kernel, marc.zyngier, david.daney, hpa, tglx

Commit-ID:  20c4d49c0f304f3f945bbd560b26afa98f75a0c4
Gitweb:     http://git.kernel.org/tip/20c4d49c0f304f3f945bbd560b26afa98f75a0c4
Author:     Dan Carpenter <dan.carpenter@oracle.com>
AuthorDate: Fri, 25 Aug 2017 15:14:09 +0300
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 25 Aug 2017 22:40:26 +0200

irqdomain: Prevent potential NULL pointer dereference in irq_domain_push_irq()

This code generates a Smatch warning:

  kernel/irq/irqdomain.c:1511 irq_domain_push_irq()
  warn: variable dereferenced before check 'root_irq_data' (see line 1508)

irq_get_irq_data() can return a NULL pointer, but the code dereferences
the returned pointer before checking it.

Move the NULL pointer check before the dereference.

[ tglx: Rewrote changelog to be precise and conforming to the instructions
  	in submitting-patches and added a Fixes tag. Sigh! ]

Fixes: 495c38d3001f ("irqdomain: Add irq_domain_{push,pop}_irq() functions")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: David Daney <david.daney@cavium.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: kernel-janitors@vger.kernel.org
Link: http://lkml.kernel.org/r/20170825121409.6rfv4vt6ztz2oqkt@mwanda

---
 kernel/irq/irqdomain.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 1ff9912..d623517 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -1504,10 +1504,10 @@ int irq_domain_push_irq(struct irq_domain *domain, int virq, void *arg)
 	if (WARN_ON(!irq_domain_is_hierarchy(domain)))
 		return -EINVAL;
 
-	if (domain->parent != root_irq_data->domain)
+	if (!root_irq_data)
 		return -EINVAL;
 
-	if (!root_irq_data)
+	if (domain->parent != root_irq_data->domain)
 		return -EINVAL;
 
 	child_irq_data = kzalloc_node(sizeof(*child_irq_data), GFP_KERNEL,

^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-08-25 20:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-25 12:14 [PATCH v3] irqdomain: Prevent Oops in irq_domain_push_irq() Dan Carpenter
2017-08-25 12:14 ` Dan Carpenter
2017-08-25 16:09 ` David Daney
2017-08-25 16:09   ` David Daney
2017-08-25 20:43 ` [tip:irq/core] irqdomain: Prevent potential NULL pointer dereference " tip-bot for Dan Carpenter

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.