Re: [PATCH] irq/irq_domain: Quit ignoring error returns from irq_alloc_desc_from().

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


Rob,

What the he*%? ...


On 04/05/2012 08:37 PM, Rob Herring wrote:
On 04/05/2012 06:52 PM, David Daney wrote:
From: David Daney<david.daney@xxxxxxxxxx>

In commit 4bbdd45a (irq_domain/powerpc: eliminate irq_map; use
irq_alloc_desc() instead) code was added that ignores error returns
from irq_alloc_desc_from() by (silently) casting the return value to
unsigned.  The negitive value error return now suddenly looks like a
valid irq number.

Commits cc79ca69 (irq_domain: Move irq_domain code from powerpc to
kernel/irq) and 1bc04f2c (irq_domain: Add support for base irq and
hwirq in legacy mappings) move this code to its current location in

That would be commits:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=4bbdd45afdae208a7c4ade89cf602f89a6397cff
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=cc79ca691c292e9fd44f589c7940b9654e22f2f6
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=1bc04f2cf8c2a1feadbd994f50c40bb145bf2989

irqdomain.c

The result of all of this is a null pointer dereference OOPS if one of
the error cases is hit.

The fix: Don't cast away the negativeness of the return value and then
check for errors.

Signed-off-by: David Daney<david.daney@xxxxxxxxxx>
---
  kernel/irq/irqdomain.c |   11 ++++++-----
  1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index af48e59..9d3e3ae 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -351,6 +351,7 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
  				irq_hw_number_t hwirq)
  {
  	unsigned int virq, hint;
+	int irq;

  	pr_debug("irq: irq_create_mapping(0x%p, 0x%lx)\n", domain, hwirq);

@@ -380,14 +381,14 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
  	hint = hwirq % irq_virq_count;
  	if (hint == 0)
  		hint++;
-	virq = irq_alloc_desc_from(hint, 0);

You are not looking at mainline. hint was removed in later versions, and
the referenced commit ids don't exist.

Please look at Linus' tree before making incorrect statements about whether or not code exists on the 'mainline'

The current kernel.org tree contains the bug and will cause anything using irq_create_mapping() to crash in a semi-random manner.

David Daney


Rob

-	if (!virq)
-		virq = irq_alloc_desc_from(1, 0);
-	if (!virq) {
+	irq = irq_alloc_desc_from(hint, 0);
+	if (irq<= 0)
+		irq = irq_alloc_desc_from(1, 0);
+	if (irq<= 0) {
  		pr_debug("irq: ->  virq allocation failed\n");
  		return 0;
  	}
-
+	virq = irq;
  	if (irq_setup_virq(domain, virq, hwirq)) {
  		if (domain->revmap_type != IRQ_DOMAIN_MAP_LEGACY)
  			irq_free_desc(virq);






[Linux MIPS Home]     [LKML Archive]     [Linux ARM]     [Linux]     [Git]     [Photo]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

Add to Google Powered by Linux