[Bug 300062] linuxkpi: pci_release_region() never removes pdev->mmio entries ("driver did not clean up; leaking mmio resources" on detach)

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=300062

            Bug ID: 300062
           Summary: linuxkpi: pci_release_region() never removes
                    pdev->mmio entries ("driver did not clean up; leaking
                    mmio resources" on detach)
           Product: Base System
           Version: CURRENT
          Hardware: amd64
                OS: Any
            Status: New
          Severity: Affects Some People
          Priority: ---
         Component: kern
          Assignee: bugs@FreeBSD.org
          Reporter: michal+freebsd@kilijanek.pl
                CC: bz@FreeBSD.org

------------------------------------------------------------------------
Description
------------------------------------------------------------------------

Every detach of iwlwifi (AX210, 8086:2725) prints:

  iwlwifi0: lkpi_pci_dev_release_common: driver did not clean up; leaking mmio
resources
  iwlwifi0: detached

The message appeared 4 times out of 4 when running `devctl detach iwlwifi0`
(re-attach works). iwlwifi uses the managed API
(sys/contrib/dev/iwlwifi/pcie/drv.c):

  pcim_enable_device(pdev);
  pcim_request_all_regions(pdev, DRV_NAME);
  hw_base = pcim_iomap(pdev, 0, 0);

so it is not expected to release the regions itself.

------------------------------------------------------------------------
Suspected cause (code reading, main 175595ba6)
------------------------------------------------------------------------

sys/compat/linuxkpi/common/src/linux_pci.c:

lkpi_pci_request_region() (line 1168) records each requested BAR in
pdev->mmio (TAILQ_INSERT_TAIL at line 1216) with mmio->res = the
struct resource *.  On release, lkpi_pci_devres_release() calls
pci_release_region() for each BAR, but linuxkpi_pci_release_region()
(line 1262) matches the list entry like this (line 1285):

        TAILQ_FOREACH_SAFE(mmio, &pdev->mmio, next, p) {
                if (rle->res != (void *)rman_get_bushandle(mmio->res))
                        continue;
                TAILQ_REMOVE(&pdev->mmio, mmio, next);
                free(mmio, M_DEVBUF);
        }

rle->res is a struct resource * while rman_get_bushandle() returns the
bus-space handle (the mapping address), so the condition is (practically)
always true and the entry is never removed.  The resource itself is then
released with bus_release_resource(), leaving a stale pci_mmio_region
that still points at it, and lkpi_pci_dev_release_common() (line 391)
warns about the non-empty list on detach.

Besides the memory leak and the warning, a stale entry referencing a
released resource could be dereferenced by a later
linuxkpi_pci_iounmap() walk on the same pdev (it calls
rman_get_bushandle(mmio->res) on every entry).  I have not observed a
crash from that.

------------------------------------------------------------------------
Proposed fix (untested)
------------------------------------------------------------------------

--- a/sys/compat/linuxkpi/common/src/linux_pci.c
+++ b/sys/compat/linuxkpi/common/src/linux_pci.c
@@ linuxkpi_pci_release_region(struct pci_dev *pdev, int bar)
        TAILQ_FOREACH_SAFE(mmio, &pdev->mmio, next, p) {
-               if (rle->res != (void *)rman_get_bushandle(mmio->res))
+               if (rle->res != mmio->res)
                        continue;
                TAILQ_REMOVE(&pdev->mmio, mmio, next);
                free(mmio, M_DEVBUF);
        }

To verify: devctl detach/attach of an iwlwifi (or any pcim_request_*
user) should no longer print "leaking mmio resources".

------------------------------------------------------------------------
Environment
------------------------------------------------------------------------

Machine:  Lenovo ThinkPad T14 Gen 2a (20XLS34M00)
Wi-Fi:    Intel AX210, pci 8086:2725 sub 8086:0024, iwlwifi, fw 89
Kernel:   16.0-CURRENT built from main (GENERIC + platform devices, plus
          an unrelated LinuxKPI NAPI change, freebsd-src PR #2497); detach
          was not exercised on the packaged GENERIC kernel
Repro:    service netif stop wlan0; devctl detach iwlwifi0

-- 
You are receiving this mail because:
You are the assignee for the bug.