Re: [PATCH] PCI: fix using __initdata memory after free in disable_acs_redir parameter

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On 1/15/19 7:32 PM, Logan Gunthorpe wrote:
The disable_acs_redir parameter stores a pointer to the string passed to
pci_setup(). However, the string passed to PCI setup is actually a
temporary copy allocated in static __initdata memory. After init, once
the memory is freed, it is no longer valid to reference this pointer.

This bug was noticed in v5.0-rc1 after a change in commit c5eb1190074c
("PCI / PM: Allow runtime PM without callback functions") caused
pci_disable_acs_redir() to be called during shutdown which manifested
as an unable to handle kernel paging request at:

RIP: 0010:pci_enable_acs+0x3f/0x1e0
Call Trace:
    pci_restore_state.part.44+0x159/0x3c0
    pci_restore_standard_config+0x33/0x40
    pci_pm_runtime_resume+0x2b/0xd0
    ? pci_restore_standard_config+0x40/0x40
    __rpm_callback+0xbc/0x1b0
    rpm_callback+0x1f/0x70
    ? pci_restore_standard_config+0x40/0x40
     rpm_resume+0x4f9/0x710
    ? pci_conf1_read+0xb6/0xf0
    ? pci_conf1_write+0xb2/0xe0
    __pm_runtime_resume+0x47/0x70
    pci_device_shutdown+0x1e/0x60

So this doesn't happen if you revert c5eb1190074c?

I guess this is due dev->state_saved being true set by pci_pm_runtime_suspend() -> pci_save_state() after my patch and now pci_pm_runtime_resume() -> pci_restore_standard_config() -> pci_restore_state() reach the pci_enable_acs(). I think this is possible to trigger also before my patch if device has the runtime PM callback defined?

It was also likely possible to trigger this bug when hotplugging PCI
devices.

To fix this, instead of storing a pointer, we use kstrdup to copy the
disable_acs_redir_param to its own buffer which will never be freed.

I wasn't able to trigger this but I saw
"PCI: Can't parse disable_acs_redir parameter: " followed by a few lines of junk during boot when I defined pci=disable_acs_redir=0000:00:xy.z which disappear after your patch.

Tested-by: Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx>
Reviewed-by: Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx>



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux