LinuxLists.cc - [PATCH] xen/pirq: fix error path cleanup when binding MSIs

2018-02-28 09:20:42

Subject: [PATCH] xen/pirq: fix error path cleanup when binding MSIs

Current cleanup in the error path of xen_bind_pirq_msi_to_irq is
wrong. First of all there's an off-by-one in the cleanup loop, which
can lead to unbinding wrong IRQs.

Secondly IRQs not bound won't be freed, thus leaking IRQ numbers.

Note that there's no need to differentiate between bound and unbound
IRQs when freeing them, __unbind_from_irq will deal with both of them
correctly.

Fixes: 4892c9b4ada9f9 ("xen: add support for MSI message groups")
Reported-by: Hooman Mirhadi <[email protected]>
Signed-off-by: Roger Pau Monné <[email protected]>
---
Cc: Boris Ostrovsky <[email protected]>
Cc: Juergen Gross <[email protected]>
Cc: Amit Shah <[email protected]>
CC: [email protected]
Cc: [email protected]
---
drivers/xen/events/events_base.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index b241bfa529ce..159faf1269fb 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -763,8 +763,8 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc,
mutex_unlock(&irq_mapping_update_lock);
return irq;
error_irq:
- for (; i >= 0; i--)
- __unbind_from_irq(irq + i);
+ while (nvec--)
+ __unbind_from_irq(irq + nvec);
mutex_unlock(&irq_mapping_update_lock);
return ret;
}
--
2.16.1

2018-02-28 09:24:07

by Shah, Amit

[permalink] [raw]

Subject: Re: [PATCH] xen/pirq: fix error path cleanup when binding MSIs

On Mi, 2018-02-28 at 09:19 +0000, Roger Pau Monne wrote:
> Current cleanup in the error path of xen_bind_pirq_msi_to_irq is
> wrong. First of all there's an off-by-one in the cleanup loop, which
> can lead to unbinding wrong IRQs.
>
> Secondly IRQs not bound won't be freed, thus leaking IRQ numbers.
>
> Note that there's no need to differentiate between bound and unbound
> IRQs when freeing them, __unbind_from_irq will deal with both of them
> correctly.
>
> Fixes: 4892c9b4ada9f9 ("xen: add support for MSI message groups")
> Reported-by: Hooman Mirhadi <[email protected]>
> Signed-off-by: Roger Pau Monné <[email protected]>
> ---
> Cc: Boris Ostrovsky <[email protected]>
> Cc: Juergen Gross <[email protected]>
> Cc: Amit Shah <[email protected]>
> CC: [email protected]
> Cc: [email protected]
> ---
> drivers/xen/events/events_base.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/xen/events/events_base.c
> b/drivers/xen/events/events_base.c
> index b241bfa529ce..159faf1269fb 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -763,8 +763,8 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev,
> struct msi_desc *msidesc,
> mutex_unlock(&irq_mapping_update_lock);
> return irq;
> error_irq:
> - for (; i >= 0; i--)
> - __unbind_from_irq(irq + i);
> + while (nvec--)
> + __unbind_from_irq(irq + nvec);
> mutex_unlock(&irq_mapping_update_lock);
> return ret;
> }

Reviewed-by: Amit Shah <[email protected]>

Amit

Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

2018-02-28 14:57:49

by Boris Ostrovsky

[permalink] [raw]

Subject: Re: [PATCH] xen/pirq: fix error path cleanup when binding MSIs

On 02/28/2018 04:22 AM, Shah, Amit wrote:
> On Mi, 2018-02-28 at 09:19 +0000, Roger Pau Monne wrote:
>> Current cleanup in the error path of xen_bind_pirq_msi_to_irq is
>> wrong. First of all there's an off-by-one in the cleanup loop, which
>> can lead to unbinding wrong IRQs.
>>
>> Secondly IRQs not bound won't be freed, thus leaking IRQ numbers.
>>
>> Note that there's no need to differentiate between bound and unbound
>> IRQs when freeing them, __unbind_from_irq will deal with both of them
>> correctly.
>>
>> Fixes: 4892c9b4ada9f9 ("xen: add support for MSI message groups")
>> Reported-by: Hooman Mirhadi <[email protected]>
>> Signed-off-by: Roger Pau Monné <[email protected]>
>> ---
>> Cc: Boris Ostrovsky <[email protected]>
>> Cc: Juergen Gross <[email protected]>
>> Cc: Amit Shah <[email protected]>
>> CC: [email protected]
>> Cc: [email protected]
>> ---
>> drivers/xen/events/events_base.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/xen/events/events_base.c
>> b/drivers/xen/events/events_base.c
>> index b241bfa529ce..159faf1269fb 100644
>> --- a/drivers/xen/events/events_base.c
>> +++ b/drivers/xen/events/events_base.c
>> @@ -763,8 +763,8 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev,
>> struct msi_desc *msidesc,
>> mutex_unlock(&irq_mapping_update_lock);
>> return irq;
>> error_irq:
>> - for (; i >= 0; i--)
>> - __unbind_from_irq(irq + i);
>> + while (nvec--)
>> + __unbind_from_irq(irq + nvec);
>> mutex_unlock(&irq_mapping_update_lock);
>> return ret;
>> }
> Reviewed-by: Amit Shah <[email protected]>

Reviewed-by: Boris Ostrovsky <[email protected]>

2018-02-28 19:23:37

by Jürgen Groß

[permalink] [raw]

Subject: Re: [PATCH] xen/pirq: fix error path cleanup when binding MSIs

On 28/02/18 10:19, Roger Pau Monne wrote:
> Current cleanup in the error path of xen_bind_pirq_msi_to_irq is
> wrong. First of all there's an off-by-one in the cleanup loop, which
> can lead to unbinding wrong IRQs.
>
> Secondly IRQs not bound won't be freed, thus leaking IRQ numbers.
>
> Note that there's no need to differentiate between bound and unbound
> IRQs when freeing them, __unbind_from_irq will deal with both of them
> correctly.
>
> Fixes: 4892c9b4ada9f9 ("xen: add support for MSI message groups")
> Reported-by: Hooman Mirhadi <[email protected]>
> Signed-off-by: Roger Pau Monné <[email protected]>

Committed to xen/tip for-linus-4.16a

Juergen

2018-03-15 17:10:46

by Shah, Amit

[permalink] [raw]