netdev - Re: [Intel-wired-lan] [PATCH v2 iwl-net] igc: don't fail igc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5d0627e4-4289-486b-80bb-a6cdc085e149@molgen.mpg.de>
Date: Wed, 10 Sep 2025 15:50:32 +0200
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Kohei Enju <enjuk@...zon.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
 Tony Nguyen <anthony.l.nguyen@...el.com>,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Kurt Kanzenbach <kurt@...utronix.de>,
 Aleksandr Loktionov <aleksandr.loktionov@...el.com>,
 Vitaly Lifshits <vitaly.lifshits@...el.com>, kohei.enju@...il.com
Subject: Re: [Intel-wired-lan] [PATCH v2 iwl-net] igc: don't fail igc_probe()
 on LED setup error

Dear Kohei,


Thank you for your patch.

Am 10.09.25 um 15:47 schrieb Kohei Enju:
> When igc_led_setup() fails, igc_probe() fails and triggers kernel panic
> in free_netdev() since unregister_netdev() is not called. [1]
> This behavior can be tested using fault-injection framework, especially
> the failslab feature. [2]
> 
> Since LED support is not mandatory, treat LED setup failures as
> non-fatal and continue probe with a warning message, consequently
> avoiding the kernel panic.
> 
> [1]
>   kernel BUG at net/core/dev.c:12047!
>   Oops: invalid opcode: 0000 [#1] SMP NOPTI
>   CPU: 0 UID: 0 PID: 937 Comm: repro-igc-led-e Not tainted 6.17.0-rc4-enjuk-tnguy-00865-gc4940196ab02 #64 PREEMPT(voluntary)
>   Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
>   RIP: 0010:free_netdev+0x278/0x2b0
>   [...]
>   Call Trace:
>    <TASK>
>    igc_probe+0x370/0x910
>    local_pci_probe+0x3a/0x80
>    pci_device_probe+0xd1/0x200
>   [...]
> 
> [2]
>   #!/bin/bash -ex
> 
>   FAILSLAB_PATH=/sys/kernel/debug/failslab/
>   DEVICE=0000:00:05.0
>   START_ADDR=$(grep " igc_led_setup" /proc/kallsyms \
>           | awk '{printf("0x%s", $1)}')
>   END_ADDR=$(printf "0x%x" $((START_ADDR + 0x100)))
> 
>   echo $START_ADDR > $FAILSLAB_PATH/require-start
>   echo $END_ADDR > $FAILSLAB_PATH/require-end
>   echo 1 > $FAILSLAB_PATH/times
>   echo 100 > $FAILSLAB_PATH/probability
>   echo N > $FAILSLAB_PATH/ignore-gfp-wait
> 
>   echo $DEVICE > /sys/bus/pci/drivers/igc/bind

Sweet!

> Fixes: ea578703b03d ("igc: Add support for LEDs on i225/i226")
> Signed-off-by: Kohei Enju <enjuk@...zon.com>
> ---
> Changes:
>    v1->v2:
>      - don't fail probe when led setup fails
>      - rephrase subject and commit message
>    v1: https://lore.kernel.org/intel-wired-lan/20250906055239.29396-1-enjuk@amazon.com/
> ---
>   drivers/net/ethernet/intel/igc/igc.h      |  1 +
>   drivers/net/ethernet/intel/igc/igc_main.c | 12 +++++++++---
>   2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
> index 266bfcf2a28f..a427f05814c1 100644
> --- a/drivers/net/ethernet/intel/igc/igc.h
> +++ b/drivers/net/ethernet/intel/igc/igc.h
> @@ -345,6 +345,7 @@ struct igc_adapter {
>   	/* LEDs */
>   	struct mutex led_mutex;
>   	struct igc_led_classdev *leds;
> +	bool leds_available;
>   };
>   
>   void igc_up(struct igc_adapter *adapter);
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index e79b14d50b24..728d7ca5338b 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -7335,8 +7335,14 @@ static int igc_probe(struct pci_dev *pdev,
>   
>   	if (IS_ENABLED(CONFIG_IGC_LEDS)) {
>   		err = igc_led_setup(adapter);
> -		if (err)
> -			goto err_register;
> +		if (err) {
> +			netdev_warn_once(netdev,
> +					 "LED init failed (%d); continuing without LED support\n",
> +					 err);
> +			adapter->leds_available = false;
> +		} else {
> +			adapter->leds_available = true;
> +		}
>   	}
>   
>   	return 0;
> @@ -7392,7 +7398,7 @@ static void igc_remove(struct pci_dev *pdev)
>   	cancel_work_sync(&adapter->watchdog_task);
>   	hrtimer_cancel(&adapter->hrtimer);
>   
> -	if (IS_ENABLED(CONFIG_IGC_LEDS))
> +	if (IS_ENABLED(CONFIG_IGC_LEDS) && adapter->leds_available)
>   		igc_led_free(adapter);
>   
>   	/* Release control of h/w to f/w.  If f/w is AMT enabled, this

Reviewed-by: Paul Menzel <pmenzel@...gen.mpg.de>


Kind regards,

Paul