Re: [PATCH v5 1/2] hisi_acc_vfio_pci: fix NULL dereference in reset_prepare on PF passthrough

[Alex Williamson]

On Wed, 23 Sep 2026 16:25:50 +0800
Longfang Liu <liulongfang@xxxxxxxxxx> wrote:

> When a PF is bound to the driver via driver_override and passed
> through to a VM, its pf_qm stays NULL. The PCI error handler
> reset_prepare() runs during open_device through
> pci_try_reset_function(), before the mig_ops gate, and dereferences
> the NULL pf_qm for the timeout log, crashing the kernel.
> 
> Move the mig_ops check to the entry of reset_prepare() and
> aer_reset_done() so non-migration devices skip the QM_RESETTING
> coordination. Also clear set_reset_flag together with QM_RESETTING
> in aer_reset_done(); the flag was never cleared before, so a later
> timed-out reset could release a foreign lock.
> 
> Fixes: b0eed085903e ("hisi_acc_vfio_pci: Add support for VFIO live migration")
> Fixes: a22099ed7936f ("hisi_acc_vfio_pci: fix VF reset timeout issue")
> Signed-off-by: Longfang Liu <liulongfang@xxxxxxxxxx>
> ---
>  drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> index 86362ec424a5..6a09252258b9 100644
> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> @@ -1157,6 +1157,9 @@ static void hisi_acc_vf_pci_reset_prepare(struct pci_dev *pdev)

Rolling back to one more line of context in this function:

	struct hisi_qm *qm = hisi_acc_vdev->pf_qm;

As per the commit log, we're trying to fix a problem where pf_qm is
NULL and dereferenced, such as in this next line:

>  	struct device *dev = &qm->pdev->dev;
>  	u32 delay = 0;
>  
> +	if (!hisi_acc_vdev->core_device.vdev.mig_ops)
> +		return;
> +
>  	/* All reset requests need to be queued for processing */
>  	while (test_and_set_bit(QM_RESETTING, &qm->misc_ctl)) {
>  		msleep(1);

Further context:

                if (++delay > QM_RESET_WAIT_TIMEOUT) {
                        dev_err(dev, "reset prepare failed\n");
                        return;
                }

So the added test exits before the inline dereference testing
QM_RESETTING, but you're relying on the compiler to move the resolution
of dev into the branch here.  The dereference bug is not fully
resolved, it's only masked by the compiler optimization.

The easy solution is to either move the struct device deref into the
branch where it's needed, or (better) just open code &qm->pdev->dev on
the dev_err() call.  Thanks,

Alex

> @@ -1174,12 +1177,14 @@ static void hisi_acc_vf_pci_aer_reset_done(struct pci_dev *pdev)
>  	struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_drvdata(pdev);
>  	struct hisi_qm *qm = hisi_acc_vdev->pf_qm;
>  
> -	if (hisi_acc_vdev->set_reset_flag)
> -		clear_bit(QM_RESETTING, &qm->misc_ctl);
> -
>  	if (!hisi_acc_vdev->core_device.vdev.mig_ops)
>  		return;
>  
> +	if (hisi_acc_vdev->set_reset_flag) {
> +		clear_bit(QM_RESETTING, &qm->misc_ctl);
> +		hisi_acc_vdev->set_reset_flag = false;
> +	}
> +
>  	mutex_lock(&hisi_acc_vdev->state_mutex);
>  	hisi_acc_vf_reset(hisi_acc_vdev);
>  	mutex_unlock(&hisi_acc_vdev->state_mutex);