Two patches fixing one seen problem and another potential one. Both
have been introduced in the 6.7 kernel.
Juergen Gross (2):
xen/evtchn: avoid WARN() when unbinding an event channel
xen/events: increment refcnt only if event channel is refcounted
drivers/xen/events/events_base.c | 22 +++++++++++++---------
drivers/xen/evtchn.c | 6 ++++++
2 files changed, 19 insertions(+), 9 deletions(-)
--
2.35.3
When unbinding a user event channel, the related handler might be
called a last time in case the kernel was built with
CONFIG_DEBUG_SHIRQ. This might cause a WARN() in the handler.
Avoid that by adding an "unbinding" flag to struct user_event which
will short circuit the handler.
Fixes: 9e90e58c11b7 ("xen: evtchn: Allow shared registration of IRQ handers")
Reported-by: Demi Marie Obenour <[email protected]>
Tested-by: Demi Marie Obenour <[email protected]>
Signed-off-by: Juergen Gross <[email protected]>
---
drivers/xen/evtchn.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
index 59717628ca42..f6a2216c2c87 100644
--- a/drivers/xen/evtchn.c
+++ b/drivers/xen/evtchn.c
@@ -85,6 +85,7 @@ struct user_evtchn {
struct per_user_data *user;
evtchn_port_t port;
bool enabled;
+ bool unbinding;
};
static void evtchn_free_ring(evtchn_port_t *ring)
@@ -164,6 +165,10 @@ static irqreturn_t evtchn_interrupt(int irq, void *data)
struct per_user_data *u = evtchn->user;
unsigned int prod, cons;
+ /* Handler might be called when tearing down the IRQ. */
+ if (evtchn->unbinding)
+ return IRQ_HANDLED;
+
WARN(!evtchn->enabled,
"Interrupt for port %u, but apparently not enabled; per-user %p\n",
evtchn->port, u);
@@ -421,6 +426,7 @@ static void evtchn_unbind_from_user(struct per_user_data *u,
BUG_ON(irq < 0);
+ evtchn->unbinding = true;
unbind_from_irqhandler(irq, evtchn);
del_evtchn(u, evtchn);
--
2.35.3
In bind_evtchn_to_irq_chip() don't increment the refcnt of the event
channel blindly. In case the event channel is NOT refcounted, issue a
warning instead.
Add an additional safety net by doing the refcnt increment only if the
caller has specified IRQF_SHARED in the irqflags parameter.
Fixes: 9e90e58c11b7 ("xen: evtchn: Allow shared registration of IRQ handers")
Signed-off-by: Juergen Gross <[email protected]>
---
drivers/xen/events/events_base.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 2faa4bf78c7a..81effbd53dc5 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1190,7 +1190,7 @@ int xen_pirq_from_irq(unsigned irq)
EXPORT_SYMBOL_GPL(xen_pirq_from_irq);
static int bind_evtchn_to_irq_chip(evtchn_port_t evtchn, struct irq_chip *chip,
- struct xenbus_device *dev)
+ struct xenbus_device *dev, bool shared)
{
int ret = -ENOMEM;
struct irq_info *info;
@@ -1224,7 +1224,8 @@ static int bind_evtchn_to_irq_chip(evtchn_port_t evtchn, struct irq_chip *chip,
*/
bind_evtchn_to_cpu(info, 0, false);
} else if (!WARN_ON(info->type != IRQT_EVTCHN)) {
- info->refcnt++;
+ if (shared && !WARN_ON(info->refcnt < 0))
+ info->refcnt++;
}
ret = info->irq;
@@ -1237,13 +1238,13 @@ static int bind_evtchn_to_irq_chip(evtchn_port_t evtchn, struct irq_chip *chip,
int bind_evtchn_to_irq(evtchn_port_t evtchn)
{
- return bind_evtchn_to_irq_chip(evtchn, &xen_dynamic_chip, NULL);
+ return bind_evtchn_to_irq_chip(evtchn, &xen_dynamic_chip, NULL, false);
}
EXPORT_SYMBOL_GPL(bind_evtchn_to_irq);
int bind_evtchn_to_irq_lateeoi(evtchn_port_t evtchn)
{
- return bind_evtchn_to_irq_chip(evtchn, &xen_lateeoi_chip, NULL);
+ return bind_evtchn_to_irq_chip(evtchn, &xen_lateeoi_chip, NULL, false);
}
EXPORT_SYMBOL_GPL(bind_evtchn_to_irq_lateeoi);
@@ -1295,7 +1296,8 @@ static int bind_ipi_to_irq(unsigned int ipi, unsigned int cpu)
static int bind_interdomain_evtchn_to_irq_chip(struct xenbus_device *dev,
evtchn_port_t remote_port,
- struct irq_chip *chip)
+ struct irq_chip *chip,
+ bool shared)
{
struct evtchn_bind_interdomain bind_interdomain;
int err;
@@ -1307,14 +1309,14 @@ static int bind_interdomain_evtchn_to_irq_chip(struct xenbus_device *dev,
&bind_interdomain);
return err ? : bind_evtchn_to_irq_chip(bind_interdomain.local_port,
- chip, dev);
+ chip, dev, shared);
}
int bind_interdomain_evtchn_to_irq_lateeoi(struct xenbus_device *dev,
evtchn_port_t remote_port)
{
return bind_interdomain_evtchn_to_irq_chip(dev, remote_port,
- &xen_lateeoi_chip);
+ &xen_lateeoi_chip, false);
}
EXPORT_SYMBOL_GPL(bind_interdomain_evtchn_to_irq_lateeoi);
@@ -1430,7 +1432,8 @@ static int bind_evtchn_to_irqhandler_chip(evtchn_port_t evtchn,
{
int irq, retval;
- irq = bind_evtchn_to_irq_chip(evtchn, chip, NULL);
+ irq = bind_evtchn_to_irq_chip(evtchn, chip, NULL,
+ irqflags & IRQF_SHARED);
if (irq < 0)
return irq;
retval = request_irq(irq, handler, irqflags, devname, dev_id);
@@ -1471,7 +1474,8 @@ static int bind_interdomain_evtchn_to_irqhandler_chip(
{
int irq, retval;
- irq = bind_interdomain_evtchn_to_irq_chip(dev, remote_port, chip);
+ irq = bind_interdomain_evtchn_to_irq_chip(dev, remote_port, chip,
+ irqflags & IRQF_SHARED);
if (irq < 0)
return irq;
--
2.35.3
On 13.03.24 09:14, Juergen Gross wrote:
Hello Juergen
> In bind_evtchn_to_irq_chip() don't increment the refcnt of the event
> channel blindly. In case the event channel is NOT refcounted, issue a
> warning instead.
>
> Add an additional safety net by doing the refcnt increment only if the
> caller has specified IRQF_SHARED in the irqflags parameter.
>
> Fixes: 9e90e58c11b7 ("xen: evtchn: Allow shared registration of IRQ handers")
> Signed-off-by: Juergen Gross <[email protected]>
Reviewed-by: Oleksandr Tyshchenko <[email protected]>
> ---
> drivers/xen/events/events_base.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> index 2faa4bf78c7a..81effbd53dc5 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -1190,7 +1190,7 @@ int xen_pirq_from_irq(unsigned irq)
> EXPORT_SYMBOL_GPL(xen_pirq_from_irq);
>
> static int bind_evtchn_to_irq_chip(evtchn_port_t evtchn, struct irq_chip *chip,
> - struct xenbus_device *dev)
> + struct xenbus_device *dev, bool shared)
> {
> int ret = -ENOMEM;
> struct irq_info *info;
> @@ -1224,7 +1224,8 @@ static int bind_evtchn_to_irq_chip(evtchn_port_t evtchn, struct irq_chip *chip,
> */
> bind_evtchn_to_cpu(info, 0, false);
> } else if (!WARN_ON(info->type != IRQT_EVTCHN)) {
> - info->refcnt++;
> + if (shared && !WARN_ON(info->refcnt < 0))
> + info->refcnt++;
> }
>
> ret = info->irq;
> @@ -1237,13 +1238,13 @@ static int bind_evtchn_to_irq_chip(evtchn_port_t evtchn, struct irq_chip *chip,
>
> int bind_evtchn_to_irq(evtchn_port_t evtchn)
> {
> - return bind_evtchn_to_irq_chip(evtchn, &xen_dynamic_chip, NULL);
> + return bind_evtchn_to_irq_chip(evtchn, &xen_dynamic_chip, NULL, false);
> }
> EXPORT_SYMBOL_GPL(bind_evtchn_to_irq);
>
> int bind_evtchn_to_irq_lateeoi(evtchn_port_t evtchn)
> {
> - return bind_evtchn_to_irq_chip(evtchn, &xen_lateeoi_chip, NULL);
> + return bind_evtchn_to_irq_chip(evtchn, &xen_lateeoi_chip, NULL, false);
> }
> EXPORT_SYMBOL_GPL(bind_evtchn_to_irq_lateeoi);
>
> @@ -1295,7 +1296,8 @@ static int bind_ipi_to_irq(unsigned int ipi, unsigned int cpu)
>
> static int bind_interdomain_evtchn_to_irq_chip(struct xenbus_device *dev,
> evtchn_port_t remote_port,
> - struct irq_chip *chip)
> + struct irq_chip *chip,
> + bool shared)
> {
> struct evtchn_bind_interdomain bind_interdomain;
> int err;
> @@ -1307,14 +1309,14 @@ static int bind_interdomain_evtchn_to_irq_chip(struct xenbus_device *dev,
> &bind_interdomain);
>
> return err ? : bind_evtchn_to_irq_chip(bind_interdomain.local_port,
> - chip, dev);
> + chip, dev, shared);
> }
>
> int bind_interdomain_evtchn_to_irq_lateeoi(struct xenbus_device *dev,
> evtchn_port_t remote_port)
> {
> return bind_interdomain_evtchn_to_irq_chip(dev, remote_port,
> - &xen_lateeoi_chip);
> + &xen_lateeoi_chip, false);
> }
> EXPORT_SYMBOL_GPL(bind_interdomain_evtchn_to_irq_lateeoi);
>
> @@ -1430,7 +1432,8 @@ static int bind_evtchn_to_irqhandler_chip(evtchn_port_t evtchn,
> {
> int irq, retval;
>
> - irq = bind_evtchn_to_irq_chip(evtchn, chip, NULL);
> + irq = bind_evtchn_to_irq_chip(evtchn, chip, NULL,
> + irqflags & IRQF_SHARED);
> if (irq < 0)
> return irq;
> retval = request_irq(irq, handler, irqflags, devname, dev_id);
> @@ -1471,7 +1474,8 @@ static int bind_interdomain_evtchn_to_irqhandler_chip(
> {
> int irq, retval;
>
> - irq = bind_interdomain_evtchn_to_irq_chip(dev, remote_port, chip);
> + irq = bind_interdomain_evtchn_to_irq_chip(dev, remote_port, chip,
> + irqflags & IRQF_SHARED);
> if (irq < 0)
> return irq;
>
On 13.03.24 09:14, Juergen Gross wrote:
Hello Juergen
> When unbinding a user event channel, the related handler might be
> called a last time in case the kernel was built with
> CONFIG_DEBUG_SHIRQ. This might cause a WARN() in the handler.
>
> Avoid that by adding an "unbinding" flag to struct user_event which
> will short circuit the handler.
>
> Fixes: 9e90e58c11b7 ("xen: evtchn: Allow shared registration of IRQ handers")
> Reported-by: Demi Marie Obenour <[email protected]>
> Tested-by: Demi Marie Obenour <[email protected]>
> Signed-off-by: Juergen Gross <[email protected]>
Reviewed-by: Oleksandr Tyshchenko <[email protected]>
> ---
> drivers/xen/evtchn.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
> index 59717628ca42..f6a2216c2c87 100644
> --- a/drivers/xen/evtchn.c
> +++ b/drivers/xen/evtchn.c
> @@ -85,6 +85,7 @@ struct user_evtchn {
> struct per_user_data *user;
> evtchn_port_t port;
> bool enabled;
> + bool unbinding;
> };
>
> static void evtchn_free_ring(evtchn_port_t *ring)
> @@ -164,6 +165,10 @@ static irqreturn_t evtchn_interrupt(int irq, void *data)
> struct per_user_data *u = evtchn->user;
> unsigned int prod, cons;
>
> + /* Handler might be called when tearing down the IRQ. */
> + if (evtchn->unbinding)
> + return IRQ_HANDLED;
> +
> WARN(!evtchn->enabled,
> "Interrupt for port %u, but apparently not enabled; per-user %p\n",
> evtchn->port, u);
> @@ -421,6 +426,7 @@ static void evtchn_unbind_from_user(struct per_user_data *u,
>
> BUG_ON(irq < 0);
>
> + evtchn->unbinding = true;
> unbind_from_irqhandler(irq, evtchn);
>
> del_evtchn(u, evtchn);