ACL: [SRU][F][PATCH 0/3] Reassign I/O Path of ConnectX-5 Port 1 before Port 2 causes NULL dereference (LP: 1943464)

[Krzysztof Kozlowski]

On 03/11/2021 14:56, frank.heimes at canonical.com wrote:
> BugLink: https://bugs.launchpad.net/bugs/1943464
> 
> SRU Justification:
> 
> [Impact]
> 
> * After reassigning a PCHID of a ConnectX-5 based RoCE Adapter
>   from one physical LPAR to another,
>   running Ubuntu 20.04 with kernel 5.4 (latest),
>   a lifetime issue occurs.
> 
> * Subsequent testing on newer kernels now shows that a
>   NULL pointer dereference in the zPCI code happens (causing a hard crash)
>   that was previously hidden by leaking the struct pci_dev.
>   
> * For a more detailed root cause analysis, see the below original bug description.
> 
> [Fix]
> 
> The following three commits fix this issue in focal:
> 
> * upstream (since v5.12-rc4):
>   0b13525c20febcfecccf6fc1db5969727401317d 0b13525c20fe "s390/pci: fix leak of PCI device structure"
>   backport: https://launchpadlibrarian.net/566161494/0001-s390-pci-fix-leak-of-PCI-device-structure.patch
> 
> * upstream (since v5.14-rc7):
>   2a671f77ee49f3e78997b77fdee139467ff6a598 2a671f77ee49 "s390/pci: fix use after free of zpci_dev"
>   backport: https://launchpadlibrarian.net/566161496/0002-s390-pci-fix-use-after-free-of-zpci_dev.patch
> 
> * upstream (since v5.15-rc5):
>   a46044a92add6a400f4dada7b943b30221f7cc80 a46044a92add "s390/pci: fix zpci_zdev_put() on reserve"
>   backport: https://launchpadlibrarian.net/566161497/0003-s390-pci-fix-zpci_zdev_put-on-reserve.patch
> 
> * Commit 0b13525c20fe fixes a lifetime issue of the struct pci_dev  that was not released on removal,
>   commit 2a671f77ee49 fixes the 'NULL pointer dereference' (causing the hard crash) itself.
>   and commit a46044a92add fixes the handling of multiple events for a single reserve state transition of the device.
>   Without this, the NULL dereference can still be triggered as Reassign I/O Path causes a redudant second removal event.
> 
> * Since none of the three upstream commits does apply cleanly to focal master-next by just cherry-picking them
>   (mainly due to changes in the context), the above backports are needed.
> 
> [Test Case]
> 
> * Two z15 or LinuxONE III LPARs, one with a Connect-X5 based RoCE adapter attached.
> 
> * LPARs need to run Ubuntu 20.04 with kernel 5.4 to hit the lifetime issue
>   (that hides the also potential existing 'NULL pointer dereference') - 
>   with Hirsute and kernel 5.11 the 'NULL pointer dereference' crash occurs.
> 
> * Now change the PCHID (physical channel identifier)
>   to a different one from the 2nd LPAR (at the HMC?).
> 
> * Verify if the reassignment worked properly (by checking the PCHID) and
>   monitor the kernel ring buffer dmesg (diagnostic messages) for 
>   "Krnl PSW" crash (caused by NULL pointer)
>   (for more error details, please see below original bug description).
> 
> * Due hardware availability reasons (the ConnectX-5 cards are only used in special cases),
>   the testing needs to be done by IBM.
> 
> [Regression Potential / What can go wrong]
> 
> * What can go wrong with: 2a671f77ee49 "s390/pci: fix use after free of zpci_dev"
> 
> * The reference count to the struct zpci_dev got increased
>   while it is used by the PCI core.
>   This could cause a leak if not properly released.
> 
> * Hot-plug of there Connect-X5 devices could be broken on s390x entirely,
>   in case the new pointer handing is erroneous.
> 
> * This may even have an impact on "cold plug", too.
> 
> * Fortunately the modifications are quite minimal and thereby traceable,
> 
> * and affect /arch/s390/pci/pci.c and arch/s390/pci/pci_bus.h only,
>   hence are specific to the s390x platform only
>   and there again to "plugging" of zPCI devices.
> 
> * What can go wrong with: 0b13525c20fe "s390/pci: fix leak of PCI device structure"
> 
> * The function zpci_remove_device got expanded with an additional set_error argument,
>   and the internal flow got significantly changed. 
>   In case handled in a wrong way, this may harm the entire remove/release logic.
> 
> * The calls of zpci_remove_device need to be adjusted (as part of the new arg),
>   failures here will most likely be identified at compile time.
> 
> * The initialization of the pci_dev struct got improved,
> 
> * and the flow in __zpci_event_availability carefully changed
>   to reflect the device slot/bus remove characteristics.
>   However, issues here may lead again to general zpci hotplug removal issues.
> 
> * Fortunately all modiications are limited to s390x only (/arch/s390/*
>   and /drivers/pci/hotplug/s390*) obviously affect zpci devices only
>   (and no ccw devices).
> 
> [Other]
> 
> * impish and hirsute already incl. "s390/pci: fix leak of PCI device structure"
>   and "s390/pci: fix use after free of zpci_dev";
>   "s390/pci: fix zpci_zdev_put() on reserve" is tagged for upstream stable v5.14.x / v5.10.x
>   (see https://lore.kernel.org/stable/31dcc776244843aa76deebd49f4ba3fbe4819990.camel@linux.ibm.com/)
>   and since we pick up v5.14.x / v5.10.x for the Ubuntu hirsute and impish kernels,
>   it will arrive there via upstream stable.
> 
> * jammy, will have all commits, since the last is part of v5.15-rc5.
> 

Nice!


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski at canonical.com>


Best regards,
Krzysztof