* [PATCH v2] xen/pcifront: check that an AER callback exists before calling it
@ 2026-10-06 2:34 Yehyeong Lee
2026-10-06 15:54 ` Bjorn Helgaas
0 siblings, 1 reply; 2+ messages in thread
From: Yehyeong Lee @ 2026-10-06 2:34 UTC (permalink / raw)
To: jgross, sstabellini, oleksandr_tyshchenko, bhelgaas, jbeulich,
konrad.wilk, xen-devel, linux-pci, linux-kernel
Cc: stable, Yehyeong Lee
pcifront_common_process() handles an AER request from the backend by
dispatching on aer_op.cmd to the bound driver's PCI error handler. It
only checks that err_handler and err_handler->error_detected are present,
then for the mmio_enabled, slot_reset and resume commands it calls the
corresponding callback unconditionally. Those three callbacks are
optional -- the PCI core NULL-checks each of them individually before
use -- and many drivers (for example igb, igc, ice and ixgbevf) install
error_detected without all of them.
aer_op.cmd comes from the shared ring, so a malicious or buggy backend
can send XEN_PCI_OP_aer_mmio (or _slotreset/_resume) for a device whose
driver leaves that callback NULL and make the frontend call through a
NULL pointer, crashing the guest.
Check each callback before calling it. When mmio_enabled or slot_reset is
absent, return PCI_ERS_RESULT_RECOVERED so recovery proceeds, as the PCI
core does for a missing callback; returning PCI_ERS_RESULT_NONE would
instead make xen-pciback tear the guest domain down.
Fixes: 956a9202cd12 ("xen-pcifront: Xen PCI frontend driver.")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
v2: return PCI_ERS_RESULT_RECOVERED for an absent mmio_enabled/slot_reset
instead of PCI_ERS_RESULT_NONE, which xen-pciback treats as fatal and
uses to tear the domain down. (Bjorn acked v1; the ack is dropped here
because v2 changes the value returned for an absent callback.)
v1: https://lore.kernel.org/all/20261005133019.284053-1-yhlee@isslab.korea.ac.kr/
There is an in-flight fix for a refcount leak in this same function
("xen/pcifront: Fix PCI device reference leak in AER handling"); this
change is orthogonal and applies in either order.
drivers/pci/xen-pcifront.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index cffc32d660327..5c6f7796d204a 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -599,11 +599,16 @@ static pci_ers_result_t pcifront_common_process(int cmd,
case XEN_PCI_OP_aer_detected:
return pdrv->err_handler->error_detected(pcidev, state);
case XEN_PCI_OP_aer_mmio:
- return pdrv->err_handler->mmio_enabled(pcidev);
+ if (pdrv->err_handler->mmio_enabled)
+ return pdrv->err_handler->mmio_enabled(pcidev);
+ return PCI_ERS_RESULT_RECOVERED;
case XEN_PCI_OP_aer_slotreset:
- return pdrv->err_handler->slot_reset(pcidev);
+ if (pdrv->err_handler->slot_reset)
+ return pdrv->err_handler->slot_reset(pcidev);
+ return PCI_ERS_RESULT_RECOVERED;
case XEN_PCI_OP_aer_resume:
- pdrv->err_handler->resume(pcidev);
+ if (pdrv->err_handler->resume)
+ pdrv->err_handler->resume(pcidev);
return PCI_ERS_RESULT_NONE;
default:
dev_err(&pdev->xdev->dev,
--
2.43.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v2] xen/pcifront: check that an AER callback exists before calling it
2026-10-06 2:34 [PATCH v2] xen/pcifront: check that an AER callback exists before calling it Yehyeong Lee
@ 2026-10-06 15:54 ` Bjorn Helgaas
0 siblings, 0 replies; 2+ messages in thread
From: Bjorn Helgaas @ 2026-10-06 15:54 UTC (permalink / raw)
To: Yehyeong Lee
Cc: jgross, sstabellini, oleksandr_tyshchenko, bhelgaas, jbeulich,
konrad.wilk, xen-devel, linux-pci, linux-kernel, stable
On Tue, Oct 06, 2026 at 11:34:43AM +0900, Yehyeong Lee wrote:
> pcifront_common_process() handles an AER request from the backend by
> dispatching on aer_op.cmd to the bound driver's PCI error handler. It
> only checks that err_handler and err_handler->error_detected are present,
> then for the mmio_enabled, slot_reset and resume commands it calls the
> corresponding callback unconditionally. Those three callbacks are
> optional -- the PCI core NULL-checks each of them individually before
> use -- and many drivers (for example igb, igc, ice and ixgbevf) install
> error_detected without all of them.
>
> aer_op.cmd comes from the shared ring, so a malicious or buggy backend
> can send XEN_PCI_OP_aer_mmio (or _slotreset/_resume) for a device whose
> driver leaves that callback NULL and make the frontend call through a
> NULL pointer, crashing the guest.
>
> Check each callback before calling it. When mmio_enabled or slot_reset is
> absent, return PCI_ERS_RESULT_RECOVERED so recovery proceeds, as the PCI
> core does for a missing callback; returning PCI_ERS_RESULT_NONE would
> instead make xen-pciback tear the guest domain down.
>
> Fixes: 956a9202cd12 ("xen-pcifront: Xen PCI frontend driver.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> v2: return PCI_ERS_RESULT_RECOVERED for an absent mmio_enabled/slot_reset
> instead of PCI_ERS_RESULT_NONE, which xen-pciback treats as fatal and
> uses to tear the domain down. (Bjorn acked v1; the ack is dropped here
> because v2 changes the value returned for an absent callback.)
> v1: https://lore.kernel.org/all/20261005133019.284053-1-yhlee@isslab.korea.ac.kr/
>
> There is an in-flight fix for a refcount leak in this same function
> ("xen/pcifront: Fix PCI device reference leak in AER handling"); this
> change is orthogonal and applies in either order.
>
> drivers/pci/xen-pcifront.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index cffc32d660327..5c6f7796d204a 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -599,11 +599,16 @@ static pci_ers_result_t pcifront_common_process(int cmd,
> case XEN_PCI_OP_aer_detected:
> return pdrv->err_handler->error_detected(pcidev, state);
> case XEN_PCI_OP_aer_mmio:
> - return pdrv->err_handler->mmio_enabled(pcidev);
> + if (pdrv->err_handler->mmio_enabled)
> + return pdrv->err_handler->mmio_enabled(pcidev);
> + return PCI_ERS_RESULT_RECOVERED;
> case XEN_PCI_OP_aer_slotreset:
> - return pdrv->err_handler->slot_reset(pcidev);
> + if (pdrv->err_handler->slot_reset)
> + return pdrv->err_handler->slot_reset(pcidev);
> + return PCI_ERS_RESULT_RECOVERED;
> case XEN_PCI_OP_aer_resume:
> - pdrv->err_handler->resume(pcidev);
> + if (pdrv->err_handler->resume)
> + pdrv->err_handler->resume(pcidev);
> return PCI_ERS_RESULT_NONE;
> default:
> dev_err(&pdev->xdev->dev,
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-06 15:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-06 2:34 [PATCH v2] xen/pcifront: check that an AER callback exists before calling it Yehyeong Lee
2026-10-06 15:54 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®