e1000_request_msix() requests the RX, TX, and other-cause interrupt
handlers sequentially. If a later request_irq() fails, the function
returns without releasing handlers that were registered earlier. The
caller then disables MSI-X and falls back to MSI or legacy interrupts,
leaving those handlers registered against disabled MSI-X vectors.

Free all handlers registered before the failing request in reverse order
before returning the error. The failed vector is not freed, and the
existing fallback path can then disable MSI-X without retaining stale
handlers.

This can be reproduced with a QEMU e1000e device and a kernel built with
CONFIG_E1000E=y, CONFIG_FAULT_INJECTION=y, CONFIG_FAILSLAB=y,
CONFIG_FAULT_INJECTION_DEBUG_FS=y,
CONFIG_FAULT_INJECTION_STACKTRACE_FILTER=y, and CONFIG_KALLSYMS_ALL=y.
Start QEMU with an e1000e device, for example using the
-device e1000e option, then run the following commands as root inside
the guest. Replace <e1000e-iface> with the actual interface name, such
as enp0s3:

    mount -t debugfs none /sys/kernel/debug
    ip link set <e1000e-iface> down
    failslab=/sys/kernel/debug/failslab
    req=$(awk '$3 == "request_threaded_irq" { print $1; exit }' \
        /proc/kallsyms)
    req_end=$(printf '%x' $((0x$req + 0x2000)))
    echo N > "$failslab/ignore-gfp-wait"
    echo 0 > "$failslab/probability"
    echo 0 > "$failslab/interval"
    echo 0 > "$failslab/times"
    echo 32 > "$failslab/stacktrace-depth"
    echo "0x$req" > "$failslab/require-start"
    echo "0x$req_end" > "$failslab/require-end"
    echo 3 > "$failslab/interval"
    echo 1 > "$failslab/times"
    echo 100 > "$failslab/probability"
    echo 2 > "$failslab/verbose"
    ip link set <e1000e-iface> up
    dmesg | grep -E 'remove_proc_entry|WARNING: fs/proc/generic.c'

The third qualifying allocation in request_threaded_irq() then fails,
after the RX and TX handlers have been installed. On an unfixed kernel,
the last command reports a warning such as:

    remove_proc_entry: removing non-empty directory 'irq/<n>'
    WARNING: fs/proc/generic.c:<line> at remove_proc_entry

The fixed kernel reaches the MSI or legacy interrupt fallback without
this warning. The failure is deliberately injected to exercise the
error path; it does not claim that a third request_irq() failure occurs
spontaneously during normal operation.

Fixes: 4662e82b2cb4 ("e1000e: add support for new 82574L part")
Cc: [email protected]
Assisted-by: LLM Codex
Reviewed-by: Aleksandr Loktionov <[email protected]>
Signed-off-by: Runyu Xiao <[email protected]>

---
v2:
- Add exact QEMU/failslab reproduction steps and the expected warning.
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
b/drivers/net/ethernet/intel/e1000e/netdev.c
index 844f31ab37ad4..f55aec340342b 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -2139,7 +2139,7 @@ static int e1000_request_msix(struct e1000_adapter 
*adapter)
                          e1000_intr_msix_tx, 0, adapter->tx_ring->name,
                          netdev);
        if (err)
-               return err;
+               goto err_irq;
        adapter->tx_ring->itr_register = adapter->hw.hw_addr +
            E1000_EITR_82574(vector);
        adapter->tx_ring->itr_val = adapter->itr;
@@ -2148,11 +2148,16 @@ static int e1000_request_msix(struct e1000_adapter 
*adapter)
        err = request_irq(adapter->msix_entries[vector].vector,
                          e1000_msix_other, 0, netdev->name, netdev);
        if (err)
-               return err;
+               goto err_irq;
 
        e1000_configure_msix(adapter);
 
        return 0;
+
+err_irq:
+       while (vector)
+               free_irq(adapter->msix_entries[--vector].vector, netdev);
+       return err;
 }
 
 /**
-- 
2.34.1

Reply via email to