Re: [PATCH] pci: dwc: pcie_designware: Fix a sleep-in-atomic-context bug in dw_pcie_prog_outbound_atu

[Jia-Ju Bai]

On 2018/9/11 16:40, Gustavo Pimentel wrote:
> Hi Jia,
>
> On 02/09/2018 04:38, Jia-Ju Bai wrote:
> > The driver may sleep with holding a spinlock and in an interupt handler.
> >
> > The function call paths (from bottom to top) in Linux-4.16 are:
> >
> > [FUNC] usleep_range
> > drivers/pci/dwc/pcie-designware.c, 181:
> > 	usleep_range in dw_pcie_prog_outbound_atu
> > drivers/pci/dwc/pcie-designware-host.c, 479:
> > 	dw_pcie_prog_outbound_atu in dw_pcie_rd_other_conf
> > drivers/pci/dwc/pcie-designware-host.c, 561:
> > 	dw_pcie_rd_other_conf in dw_pcie_rd_conf
> > drivers/pci/access.c, 66:
> > 	[FUNC_PTR]dw_pcie_rd_conf in pci_bus_read_config_word
> > drivers/pci/access.c, 918:
> > 	pci_bus_read_config_word in pci_read_config_word
> > drivers/block/umem.c, 630:
> > 	pci_read_config_word in mm_interrupt (interrupt handler)
> >
> > [FUNC] usleep_range
> > drivers/pci/dwc/pcie-designware.c, 181:
> > 	usleep_range in dw_pcie_prog_outbound_atu
> > drivers/pci/dwc/pcie-designware-host.c, 479:
> > 	dw_pcie_prog_outbound_atu in dw_pcie_rd_other_conf
> > drivers/pci/dwc/pcie-designware-host.c, 561:
> > 	dw_pcie_rd_other_conf in dw_pcie_rd_conf
> > drivers/pci/access.c, 66:
> > 	[FUNC_PTR]dw_pcie_rd_conf in pci_bus_read_config_word
> > drivers/pci/access.c, 918:
> > 	pci_bus_read_config_word in pci_read_config_word
> > drivers/ata/pata_efar.c, 115:
> > 	pci_read_config_word in efar_set_piomode
> > drivers/ata/pata_efar.c, 113:
> > 	_raw_spin_lock_irqsave in efar_set_piomod
> >
> > [FUNC] usleep_range
> > drivers/pci/dwc/pcie-designware.c, 181:
> > 	usleep_range in dw_pcie_prog_outbound_atu
> > drivers/pci/dwc/pcie-designware-host.c, 479:
> > 	dw_pcie_prog_outbound_atu in dw_pcie_rd_other_conf
> > drivers/pci/dwc/pcie-designware-host.c, 561:
> > 	dw_pcie_rd_other_conf in dw_pcie_rd_conf
> > drivers/pci/access.c, 66:
> > 	[FUNC_PTR]dw_pcie_rd_conf in pci_bus_read_config_word
> > drivers/pci/access.c, 918:
> > 	pci_bus_read_config_word in pci_read_config_word
> > drivers/block/mtip32xx/mtip32xx.c, 158:
> > 	pci_read_config_word in mtip_check_surprise_removal
> > drivers/block/mtip32xx/mtip32xx.c, 843:
> > 	mtip_check_surprise_removal in mtip_handle_irq
> > drivers/block/mtip32xx/mtip32xx.c, 879:
> > 	mtip_handle_irq in mtip_irq_handler (interrupt handler)
> >
> > [FUNC] usleep_range
> > drivers/pci/dwc/pcie-designware.c, 181:
> > 	usleep_range in dw_pcie_prog_outbound_atu
> > drivers/pci/dwc/pcie-designware-host.c, 479:
> > 	dw_pcie_prog_outbound_atu in dw_pcie_rd_other_conf
> > drivers/pci/dwc/pcie-designware-host.c, 561:
> > 	dw_pcie_rd_other_conf in dw_pcie_rd_conf
> > drivers/pci/access.c, 66:
> > 	[FUNC_PTR]dw_pcie_rd_conf in pci_bus_read_config_word
> > drivers/pci/access.c, 918:
> > 	pci_bus_read_config_word in pci_read_config_word
> > drivers/gpu/vga/vgaarb.c, 645:
> > 	pci_read_config_word in vga_arbiter_add_pci_device
> > drivers/gpu/vga/vgaarb.c, 629:
> > 	_raw_spin_lock_irqsave in vga_arbiter_add_pci_device
> >
> > [FUNC] usleep_range
> > drivers/pci/dwc/pcie-designware.c, 181:
> > 	usleep_range in dw_pcie_prog_outbound_atu
> > drivers/pci/dwc/pcie-designware-host.c, 479:
> > 	dw_pcie_prog_outbound_atu in dw_pcie_rd_other_conf
> > drivers/pci/dwc/pcie-designware-host.c, 561:
> > 	dw_pcie_rd_other_conf in dw_pcie_rd_conf
> > drivers/pci/access.c, 66:
> > 	[FUNC_PTR]dw_pcie_rd_conf in pci_bus_read_config_word
> > drivers/pci/access.c, 918:
> > 	pci_bus_read_config_word in pci_read_config_word
> > drivers/pci/ats.c, 139:
> > 	pci_read_config_word in pci_ats_queue_depth
> > drivers/iommu/intel-iommu.c, 1519:
> > 	pci_ats_queue_depth in iommu_enable_dev_iotlb
> > drivers/iommu/intel-iommu.c, 5295:
> > 	iommu_enable_dev_iotlb in intel_iommu_enable_pasid
> > drivers/iommu/intel-iommu.c, 5241:
> > 	_raw_spin_lock_irqsave in intel_iommu_enable_pasid
> >
> > To fix this bug, usleep_range() is replaced with udelay().
> >
> > This bug is found by my static analysis tool DSAC.
> >
> > Signed-off-by: Jia-Ju Bai <baijiaju1990@xxxxxxxxx>
> > ---
> >   drivers/pci/controller/dwc/pcie-designware.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 778c4f76a884..7f50f7e51543 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -135,7 +135,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
> >   		if (val & PCIE_ATU_ENABLE)
> >   			return;
> >   
> > -		usleep_range(LINK_WAIT_IATU_MIN, LINK_WAIT_IATU_MAX);
> > +		udelay(LINK_WAIT_IATU_MAX);
> You might use the mdelay instead of udelay, however this fix and others were
> already made on patch on [1]
>
> [1] https://lkml.org/lkml/2018/9/10/284

Okay, that is good.


Best wishes,
Jia-Ju Bai