[patch] xen-event: validate irq before get evtchn by irq

[patch] xen-event: validate irq before get evtchn by irq

[Joe Jin]

Hi,

When get event channel by irq by irq, may irq is not a valid one under
some condition -- so far we found irq is -1 during suspend/resume, so
worth to validate it rather than return incorrect data.

Please review and comment.

Signed-off-by: Joe Jin <joe.jin@oracle.com>
Tested-by: Gurudas Pai <gurudas.pai@oracle.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>

---
 events.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index ac7b42f..006dd7b 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -175,6 +175,9 @@ static struct irq_info *info_for_irq(unsigned irq)
 
 static unsigned int evtchn_from_irq(unsigned irq)
 {
+	if (WARN(irq < 0 || irq >= nr_irqs, "Invalid irq %d!\n", irq))
+		return 0;
+
 	return info_for_irq(irq)->evtchn;
 }
 

[patch] xen-event: validate irq before get evtchn by irq

[Joe Jin]

Hi,

When get event channel by irq by irq, may irq is not a valid one under
some condition -- so far we found irq is -1 during suspend/resume, so
worth to validate it rather than return incorrect data.

Please review and comment.

Signed-off-by: Joe Jin <joe.jin@oracle.com>
Tested-by: Gurudas Pai <gurudas.pai@oracle.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>

---
 events.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index ac7b42f..006dd7b 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -175,6 +175,9 @@ static struct irq_info *info_for_irq(unsigned irq)
 
 static unsigned int evtchn_from_irq(unsigned irq)
 {
+	if (WARN(irq < 0 || irq >= nr_irqs, "Invalid irq %d!\n", irq))
+		return 0;
+
 	return info_for_irq(irq)->evtchn;
 }
 

Re: [patch] xen-event: validate irq before get evtchn by irq

[Ian Campbell]

On Fri, 2011-01-07 at 06:50 +0000, Joe Jin wrote:
> Hi,
> 
> When get event channel by irq by irq, may irq is not a valid one under
> some condition -- so far we found irq is -1 during suspend/resume, so
> worth to validate it rather than return incorrect data.

You've accidentally got "by irq" twice there and "may irq is not a valid
one" doesn't read quite right.

Perhaps something like:

"When getting an event channel from an IRQ if the IRQ is not valid then
warn and return an error in the form of the invalid event channel.

This catches errors such as trying to notify an IRQ before connecting to
the backend (e.g. during suspend/resume)."

> Please review and comment.
> 
> Signed-off-by: Joe Jin <joe.jin@oracle.com>
> Tested-by: Gurudas Pai <gurudas.pai@oracle.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Thanks,
Ian.

> Cc: Jeremy Fitzhardinge <jeremy@goop.org>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> 
> ---
>  events.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index ac7b42f..006dd7b 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -175,6 +175,9 @@ static struct irq_info *info_for_irq(unsigned irq)
>  
>  static unsigned int evtchn_from_irq(unsigned irq)
>  {
> +	if (WARN(irq < 0 || irq >= nr_irqs, "Invalid irq %d!\n", irq))
> +		return 0;
> +
>  	return info_for_irq(irq)->evtchn;
>  }
>  
> 

Re: [patch] xen-event: validate irq before get evtchn by irq

[Ian Campbell]

On Fri, 2011-01-07 at 06:50 +0000, Joe Jin wrote:
> Hi,
> 
> When get event channel by irq by irq, may irq is not a valid one under
> some condition -- so far we found irq is -1 during suspend/resume, so
> worth to validate it rather than return incorrect data.

You've accidentally got "by irq" twice there and "may irq is not a valid
one" doesn't read quite right.

Perhaps something like:

"When getting an event channel from an IRQ if the IRQ is not valid then
warn and return an error in the form of the invalid event channel.

This catches errors such as trying to notify an IRQ before connecting to
the backend (e.g. during suspend/resume)."

> Please review and comment.
> 
> Signed-off-by: Joe Jin <joe.jin@oracle.com>
> Tested-by: Gurudas Pai <gurudas.pai@oracle.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Thanks,
Ian.

> Cc: Jeremy Fitzhardinge <jeremy@goop.org>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> 
> ---
>  events.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index ac7b42f..006dd7b 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -175,6 +175,9 @@ static struct irq_info *info_for_irq(unsigned irq)
>  
>  static unsigned int evtchn_from_irq(unsigned irq)
>  {
> +	if (WARN(irq < 0 || irq >= nr_irqs, "Invalid irq %d!\n", irq))
> +		return 0;
> +
>  	return info_for_irq(irq)->evtchn;
>  }
>  
>