* [PATCH 1/2] x86: drop "index" parameter from get_free_pirq()
@ 2012-09-05 12:24 Jan Beulich
2012-09-05 12:36 ` Andrew Cooper
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2012-09-05 12:24 UTC (permalink / raw)
To: xen-devel
[-- Attachment #1: Type: text/plain, Size: 1947 bytes --]
It's unused.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1816,7 +1816,7 @@ static inline bool_t is_free_pirq(const
pirq->arch.hvm.emuirq == IRQ_UNBOUND));
}
-int get_free_pirq(struct domain *d, int type, int index)
+int get_free_pirq(struct domain *d, int type)
{
int i;
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -71,7 +71,7 @@ static int physdev_hvm_map_pirq(
else
{
if ( *pirq < 0 )
- *pirq = get_free_pirq(d, type, *index);
+ *pirq = get_free_pirq(d, type);
ret = map_domain_emuirq_pirq(d, *pirq, *index);
}
break;
@@ -187,7 +187,7 @@ int physdev_map_pirq(domid_t domid, int
}
else
{
- pirq = get_free_pirq(d, type, *index);
+ pirq = get_free_pirq(d, type);
if ( pirq < 0 )
{
dprintk(XENLOG_G_ERR, "dom%d: no free pirq\n", d->domain_id);
@@ -705,7 +705,7 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
break;
spin_lock(&d->event_lock);
- ret = get_free_pirq(d, out.type, 0);
+ ret = get_free_pirq(d, out.type);
if ( ret >= 0 )
{
struct pirq *info = pirq_get_info(d, ret);
--- a/xen/include/asm-x86/irq.h
+++ b/xen/include/asm-x86/irq.h
@@ -136,7 +136,7 @@ int pirq_shared(struct domain *d , int i
int map_domain_pirq(struct domain *d, int pirq, int irq, int type,
void *data);
int unmap_domain_pirq(struct domain *d, int pirq);
-int get_free_pirq(struct domain *d, int type, int index);
+int get_free_pirq(struct domain *d, int type);
void free_domain_pirqs(struct domain *d);
int map_domain_emuirq_pirq(struct domain *d, int pirq, int irq);
int unmap_domain_pirq_emuirq(struct domain *d, int pirq);
[-- Attachment #2: x86-get_free_pirq-drop-index.patch --]
[-- Type: text/plain, Size: 1993 bytes --]
x86: drop "index" parameter from get_free_pirq()
It's unused.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1816,7 +1816,7 @@ static inline bool_t is_free_pirq(const
pirq->arch.hvm.emuirq == IRQ_UNBOUND));
}
-int get_free_pirq(struct domain *d, int type, int index)
+int get_free_pirq(struct domain *d, int type)
{
int i;
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -71,7 +71,7 @@ static int physdev_hvm_map_pirq(
else
{
if ( *pirq < 0 )
- *pirq = get_free_pirq(d, type, *index);
+ *pirq = get_free_pirq(d, type);
ret = map_domain_emuirq_pirq(d, *pirq, *index);
}
break;
@@ -187,7 +187,7 @@ int physdev_map_pirq(domid_t domid, int
}
else
{
- pirq = get_free_pirq(d, type, *index);
+ pirq = get_free_pirq(d, type);
if ( pirq < 0 )
{
dprintk(XENLOG_G_ERR, "dom%d: no free pirq\n", d->domain_id);
@@ -705,7 +705,7 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
break;
spin_lock(&d->event_lock);
- ret = get_free_pirq(d, out.type, 0);
+ ret = get_free_pirq(d, out.type);
if ( ret >= 0 )
{
struct pirq *info = pirq_get_info(d, ret);
--- a/xen/include/asm-x86/irq.h
+++ b/xen/include/asm-x86/irq.h
@@ -136,7 +136,7 @@ int pirq_shared(struct domain *d , int i
int map_domain_pirq(struct domain *d, int pirq, int irq, int type,
void *data);
int unmap_domain_pirq(struct domain *d, int pirq);
-int get_free_pirq(struct domain *d, int type, int index);
+int get_free_pirq(struct domain *d, int type);
void free_domain_pirqs(struct domain *d);
int map_domain_emuirq_pirq(struct domain *d, int pirq, int irq);
int unmap_domain_pirq_emuirq(struct domain *d, int pirq);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 1/2] x86: drop "index" parameter from get_free_pirq()
2012-09-05 12:24 [PATCH 1/2] x86: drop "index" parameter from get_free_pirq() Jan Beulich
@ 2012-09-05 12:36 ` Andrew Cooper
2012-09-05 12:44 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cooper @ 2012-09-05 12:36 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
On 05/09/12 13:24, Jan Beulich wrote:
> It's unused.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -1816,7 +1816,7 @@ static inline bool_t is_free_pirq(const
> pirq->arch.hvm.emuirq == IRQ_UNBOUND));
> }
>
> -int get_free_pirq(struct domain *d, int type, int index)
> +int get_free_pirq(struct domain *d, int type)
> {
> int i;
>
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -71,7 +71,7 @@ static int physdev_hvm_map_pirq(
> else
> {
> if ( *pirq < 0 )
> - *pirq = get_free_pirq(d, type, *index);
> + *pirq = get_free_pirq(d, type);
> ret = map_domain_emuirq_pirq(d, *pirq, *index);
Relatedly (and I had already noticed this but not got round to making a
patch because of other more urgent bugs)
You still have a chance here of passing an error into
map_domain_emuirq_pirq, in the pirq value. This is not a security issue
as map_domain_emuirq_pirq does range check pirq, but may turn into a
problem if the implementation of map_domain_emuirq_pirq changes. I
would say that for correctness sake, physdev_hvm_map_pirq() should range
check get_free_pirq(), even if this will lead to a double range check of
the value.
~Andrew
> }
> break;
> @@ -187,7 +187,7 @@ int physdev_map_pirq(domid_t domid, int
> }
> else
> {
> - pirq = get_free_pirq(d, type, *index);
> + pirq = get_free_pirq(d, type);
> if ( pirq < 0 )
> {
> dprintk(XENLOG_G_ERR, "dom%d: no free pirq\n", d->domain_id);
> @@ -705,7 +705,7 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
> break;
>
> spin_lock(&d->event_lock);
> - ret = get_free_pirq(d, out.type, 0);
> + ret = get_free_pirq(d, out.type);
> if ( ret >= 0 )
> {
> struct pirq *info = pirq_get_info(d, ret);
> --- a/xen/include/asm-x86/irq.h
> +++ b/xen/include/asm-x86/irq.h
> @@ -136,7 +136,7 @@ int pirq_shared(struct domain *d , int i
> int map_domain_pirq(struct domain *d, int pirq, int irq, int type,
> void *data);
> int unmap_domain_pirq(struct domain *d, int pirq);
> -int get_free_pirq(struct domain *d, int type, int index);
> +int get_free_pirq(struct domain *d, int type);
> void free_domain_pirqs(struct domain *d);
> int map_domain_emuirq_pirq(struct domain *d, int pirq, int irq);
> int unmap_domain_pirq_emuirq(struct domain *d, int pirq);
>
>
>
--
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 1/2] x86: drop "index" parameter from get_free_pirq()
2012-09-05 12:36 ` Andrew Cooper
@ 2012-09-05 12:44 ` Jan Beulich
0 siblings, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2012-09-05 12:44 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
>>> On 05.09.12 at 14:36, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 05/09/12 13:24, Jan Beulich wrote:
>> @@ -71,7 +71,7 @@ static int physdev_hvm_map_pirq(
>> else
>> {
>> if ( *pirq < 0 )
>> - *pirq = get_free_pirq(d, type, *index);
>> + *pirq = get_free_pirq(d, type);
>> ret = map_domain_emuirq_pirq(d, *pirq, *index);
>
>
> Relatedly (and I had already noticed this but not got round to making a
> patch because of other more urgent bugs)
>
> You still have a chance here of passing an error into
> map_domain_emuirq_pirq, in the pirq value. This is not a security issue
> as map_domain_emuirq_pirq does range check pirq, but may turn into a
> problem if the implementation of map_domain_emuirq_pirq changes. I
> would say that for correctness sake, physdev_hvm_map_pirq() should range
> check get_free_pirq(), even if this will lead to a double range check of
> the value.
Yes, that would be more clean.
Jan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-09-05 12:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-05 12:24 [PATCH 1/2] x86: drop "index" parameter from get_free_pirq() Jan Beulich
2012-09-05 12:36 ` Andrew Cooper
2012-09-05 12:44 ` Jan Beulich
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.