[Xen-devel] arm: Minor bug report & Fix in gic_route_irq_to_guest

[Sengul Thomas <thomas.sengul@xxxxxxxxx>]

Hello,

I found that when calling gic_route_irq_to_guest in construct_dom0 function,
it uses local variable "name" for passing devname argument.
And, gic_route_irq_to_guest just copies the pointer of this devname
and afterward,
reading this devname gives data abort.

Here goes a simple fix: just copying the data, not the pointer

ps. I'm writing this patch on top of the following source
    repo: git://xenbits.xen.org/people/julieng/xen-unstable.git
    branch: arndale
and, I'm curious is it ok?


Signed-off-by: Thomas Sengul <thomas.sengul@xxxxxxxxx>
---
 xen/arch/arm/gic.c |   13 ++++++++++++-
 xen/arch/arm/irq.c |   14 +++++++++++++-
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 63caeb8..012aae9 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -468,7 +468,10 @@ void __init release_irq(unsigned int irq)
     do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );

     if (action && action->free_on_release)
+    {
+        xfree((void *)action->name);
         xfree(action);
+    }
 }

 static int __setup_irq(struct irq_desc *desc, unsigned int irq,
@@ -617,13 +620,20 @@ int gic_route_irq_to_guest(struct domain *d,
unsigned int irq,
     struct irq_desc *desc = irq_to_desc(irq);
     unsigned long flags;
     int retval;
+    char *name;

     action = xmalloc(struct irqaction);
     if (!action)
         return -ENOMEM;

     action->dev_id = d;
-    action->name = devname;
+
+#define MIN_ACTION_NAME_LEN 16
+    name = xmalloc_array(char, MIN_ACTION_NAME_LEN);
+    if (!name)
+        return -ENOMEM;
+    strlcpy(name, devname, strnlen(devname, MIN_ACTION_NAME_LEN));
+    action->name = name;

     spin_lock_irqsave(&desc->lock, flags);
     spin_lock(&gic.lock);
@@ -635,6 +645,7 @@ int gic_route_irq_to_guest(struct domain *d,
unsigned int irq,

     retval = __setup_irq(desc, irq, action);
     if (retval) {
+        xfree((void *)action->name);
         xfree(action);
         goto out;
     }
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 8c96a0a..e6c24f9 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -99,6 +99,7 @@ int __init request_irq(unsigned int irq,
 {
     struct irqaction *action;
     int retval;
+    char *name;

     /*
      * Sanity-check: shared interrupts must pass in a real dev-ID,
@@ -116,13 +117,24 @@ int __init request_irq(unsigned int irq,
         return -ENOMEM;

     action->handler = handler;
-    action->name = devname;
+
+#define MIN_ACTION_NAME_LEN 16
+    name = xmalloc_array(char, MIN_ACTION_NAME_LEN);
+    if (!name)
+        return -ENOMEM;
+    strlcpy(name, devname, strnlen(devname, MIN_ACTION_NAME_LEN));
+    action->name = name;
+
     action->dev_id = dev_id;
     action->free_on_release = 1;

     retval = setup_irq(irq, action);
     if (retval)
+    {
+        xfree((void *)action->name);
         xfree(action);
+    }
+

     return retval;
 }


Sincerely,
Thomas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel