Re: [PATCH v9 5/5] misc: pci_endpoint_test: Add AER error handlers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] AER error handlers do not abort pending tests, causing ioctl threads to hang permanently in D state.

Pre-existing issues:
- [High] A use-after-free vulnerability exists in the ioctl handler if the device is removed while a file descriptor remains open.
--

--- Patch [5]: [PATCH v9 5/5] misc: pci_endpoint_test: Add AER error handlers ---
commit 1d34d50e68e455ba411f0fd8383353d65b5616cf
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

misc: pci_endpoint_test: Add AER error handlers

This commit introduces AER error handlers to the PCI endpoint test driver.
It adds functions to restore the configuration space and report the correct
recovery state when an error is detected.

> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index 3635741c3e7a7..b7a1497157008 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -1325,6 +1325,8 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
>  	misc_device->parent = &pdev->dev;
>  	misc_device->fops = &pci_endpoint_test_fops;
>  
> +	pci_save_state(pdev);
> +
>  	ret = misc_register(misc_device);

[Severity: High]
This is a pre-existing issue, but does this code have a use-after-free
vulnerability if the device is removed while a file descriptor remains open?

The test structure is allocated earlier in pci_endpoint_test_probe() using
devm_kzalloc(). If a userspace process has the misc device open and the
PCI device is unbound, pci_endpoint_test_remove() calls misc_deregister()
which doesn't block waiting for open file descriptors to close.

When the device unbinds, devres will free the test structure. If the
userspace process then calls ioctl(), pci_endpoint_test_ioctl() will retrieve
the freed structure from file->private_data and dereference it when attempting
to acquire the mutex:

struct pci_endpoint_test *test = to_endpoint_test(file->private_data);
struct pci_dev *pdev = test->pdev;

mutex_lock(&test->mutex);

Could this be fixed by changing the lifetime management of the test structure
or ensuring the teardown synchronization waits for open file descriptors?

[ ... ]

> @@ -1452,12 +1454,33 @@ static const struct pci_device_id pci_endpoint_test_tbl[] = {
>  };
>  MODULE_DEVICE_TABLE(pci, pci_endpoint_test_tbl);
>  
> +static pci_ers_result_t pci_endpoint_test_error_detected(struct pci_dev *pdev,
> +					      pci_channel_state_t state)
> +{
> +	if (state == pci_channel_io_perm_failure)
> +		return PCI_ERS_RESULT_DISCONNECT;
> +
> +	return PCI_ERS_RESULT_NEED_RESET;
> +}

[Severity: Medium]
Will this error handler leave tasks permanently hanging in D state if an AER
error occurs during a test?

Test functions like pci_endpoint_test_write(), pci_endpoint_test_read(), or
pci_endpoint_test_copy() initiate DMA and sleep on
wait_for_completion(&test->irq_raised) without a timeout while holding
test->mutex:

pci_endpoint_test_write() {
    ...
    wait_for_completion(&test->irq_raised);
    ...
}

If an AER error occurs during this wait, the newly introduced error handlers
execute to recover the link but fail to complete the wait queue. Since the
device state is reset, the expected interrupt never arrives, leaving the ioctl
thread stuck forever.

Should the error handler complete test->irq_raised to wake up pending ioctl
threads and abort the tests?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-pci-port-reset-v9-0-53570b92064d@oss.qualcomm.com?part=5