Re: [PATCH] xen/pcifront: check that an AER callback exists before calling it

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: returning PCI_ERS_RESULT_NONE for missing AER callbacks kills VM
--

commit fe0aa6b512c4c3fcc950e0b0317c794634341eab
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

xen/pcifront: check that an AER callback exists before calling it

This commit adds NULL checks before invoking AER callbacks like mmio_enabled,
slot_reset, and resume in pcifront_common_process(). If a callback is
missing, the code now breaks out of the switch and defaults to returning
PCI_ERS_RESULT_NONE.

> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index cffc32d660327..490913676f244 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -599,12 +599,17 @@ 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);
> +			break;
>  		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);
> +			break;

[Severity: High]
Does breaking out of the switch here when a callback is missing cause the
guest VM to be forcefully terminated by the host backend?

If the code breaks out of the switch, pcifront_common_process() falls
through and returns PCI_ERS_RESULT_NONE.

In the host backend, functions like xen_pcibk_mmio_enabled() and
xen_pcibk_slot_reset() strictly treat PCI_ERS_RESULT_NONE as a fatal
condition:

drivers/xen/xen-pciback/pci_stub.c:xen_pcibk_mmio_enabled() {
    ...
    if (result == PCI_ERS_RESULT_NONE ||
        result == PCI_ERS_RESULT_DISCONNECT) {
        dev_dbg(&dev->dev,
            "No AER mmio_enabled service or disconnected!\n");
        kill_domain_by_device(psdev);
    }
    ...
}

The standard PCI core defaults to PCI_ERS_RESULT_RECOVERED when these
callbacks are missing. Could xen-pcifront return a success status to allow
recovery to proceed, rather than returning PCI_ERS_RESULT_NONE which leads
to killing the domain?

While this patch replaces a guest NULL pointer dereference with a
host-initiated VM kill, changing the mechanism, it appears to fail to fix
the underlying guest death.

>  		case XEN_PCI_OP_aer_resume:
> -			pdrv->err_handler->resume(pcidev);
> -			return PCI_ERS_RESULT_NONE;
> +			if (pdrv->err_handler->resume)
> +				pdrv->err_handler->resume(pcidev);
> +			break;
>  		default:
>  			dev_err(&pdev->xdev->dev,
>  				"bad request in aer recovery operation!\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005133019.284053-1-yhlee@isslab.korea.ac.kr?part=1