Re: [PATCH 2/2] misc: pci_endpoint_test: Improve logic to check status for doorbell test case

On Thu, Jul 30, 2026 at 12:33:57PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> 
> New issues:
> - [Medium] PCIe surprise removals masked by incorrectly evaluating an all-1s read as success.
> 
> Pre-existing issues:
> - [High] Unvalidated endpoint input leads to NULL pointer dereference and out-of-bounds MMIO write.
> - [High] Race condition between module teardown and active ioctls leading to double-free or UAF of IRQ vectors.
> - [High] Unbounded, uninterruptible waits on endpoint interrupts create unkillable tasks and a global ioctl deadlock.
> --
> 
> --- Patch [2]: [PATCH 2/2] misc: pci_endpoint_test: Improve logic to check status for doorbell test case ---
> commit e34bb71884c618c46be8a57cc0405805a9d2c585
> Author: Niklas Cassel <[email protected]>
> 
> misc: pci_endpoint_test: Improve logic to check status for doorbell test case
> 
> This patch modifies the doorbell test case to verify if the SUCCESS bit was
> set rather than checking if the FAIL bit was set. This handles cases where
> an IRQ timeout occurs or buggy EPC drivers send multiple IRQs.
> 
> > diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> > --- a/drivers/misc/pci_endpoint_test.c
> > +++ b/drivers/misc/pci_endpoint_test.c
> > @@ -1094,7 +1094,7 @@ static int pci_endpoint_test_doorbell(struct pci_endpoint_test *test)
> >  	left = wait_for_completion_timeout(&test->irq_raised, msecs_to_jiffies(1000));
> >  
> >  	status = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS);
> > -	if (!left || (status & STATUS_DOORBELL_ENABLE_FAIL)) {
> > +	if (!left || !(status & STATUS_DOORBELL_ENABLE_SUCCESS)) {
> 
> [Severity: Medium]
> If the PCIe endpoint surprise-disconnects or the link goes down, MMIO reads
> like pci_endpoint_test_readl() will return all-1s (0xFFFFFFFF). By checking
> !(status & STATUS_DOORBELL_ENABLE_SUCCESS), an all-1s read evaluates as true
> for the bitwise AND, making the logical NOT false.
> 
> Does this inadvertently bypass the error handler during a hardware failure?

With this logic, every readl() in drivers/misc/pci_endpoint_test.c should check
if the return value is 0xFFFFFFFF.


Which they do not:

reg = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS);          
        if (!(reg & STATUS_READ_SUCCESS))                                       
                ret = -EIO;                                   


This brings pci_endpoint_test_doorbell() in line with pci_endpoint_test_read().

I still think this patch makes sense.


Kind regards,
Niklas