[PATCH v1 iwl-net] igc: fix null pointer dereference in igc_eeprom_test() on NVM-less devices

[PATCH v1 iwl-net] igc: fix null pointer dereference in igc_eeprom_test() on NVM-less devices

[Kohei Enju]

On devices without NVM, hw->nvm.ops.validate is set to NULL, therefore
functions that perform EEPROM-related operations such as
igc_ethtool_set_eeprom() and igc_probe() check for NVM presence in
advance. However igc_eeprom_test() unconditionally calls
hw->nvm.ops.validate(), potentially causing a null pointer dereference.

NVM-less devices may not be common but possible, so add NULL check
before calling hw->nvm.ops.validate().

Fixes: f026d8ca2904 ("igc: add support to eeprom, registers and link self-tests")
Signed-off-by: Kohei Enju <kohei@enjuk.jp>
---
 drivers/net/ethernet/intel/igc/igc_diag.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_diag.c b/drivers/net/ethernet/intel/igc/igc_diag.c
index a43d7244ee70..973d26a5a6c9 100644
--- a/drivers/net/ethernet/intel/igc/igc_diag.c
+++ b/drivers/net/ethernet/intel/igc/igc_diag.c
@@ -158,7 +158,7 @@ bool igc_eeprom_test(struct igc_adapter *adapter, u64 *data)
 
 	*data = 0;
 
-	if (hw->nvm.ops.validate(hw) != IGC_SUCCESS) {
+	if (hw->nvm.ops.validate && hw->nvm.ops.validate(hw) != IGC_SUCCESS) {
 		*data = 1;
 		return false;
 	}
-- 
2.51.0

Re: [PATCH v1 iwl-net] igc: fix null pointer dereference in igc_eeprom_test() on NVM-less devices

[Lifshits, Vitaly]

On 2/5/2026 10:50 AM, Kohei Enju wrote:
> On devices without NVM, hw->nvm.ops.validate is set to NULL, therefore
> functions that perform EEPROM-related operations such as
> igc_ethtool_set_eeprom() and igc_probe() check for NVM presence in
> advance. However igc_eeprom_test() unconditionally calls
> hw->nvm.ops.validate(), potentially causing a null pointer dereference.
> 
> NVM-less devices may not be common but possible, so add NULL check
> before calling hw->nvm.ops.validate().
> 
> Fixes: f026d8ca2904 ("igc: add support to eeprom, registers and link self-tests")
> Signed-off-by: Kohei Enju <kohei@enjuk.jp>
> ---
>   drivers/net/ethernet/intel/igc/igc_diag.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_diag.c b/drivers/net/ethernet/intel/igc/igc_diag.c
> index a43d7244ee70..973d26a5a6c9 100644
> --- a/drivers/net/ethernet/intel/igc/igc_diag.c
> +++ b/drivers/net/ethernet/intel/igc/igc_diag.c
> @@ -158,7 +158,7 @@ bool igc_eeprom_test(struct igc_adapter *adapter, u64 *data)
>   
>   	*data = 0;
>   
> -	if (hw->nvm.ops.validate(hw) != IGC_SUCCESS) {
> +	if (hw->nvm.ops.validate && hw->nvm.ops.validate(hw) != IGC_SUCCESS) {
>   		*data = 1;
>   		return false;
>   	}


Hi Kohei,

Thank you for your patch.

Since there are no NVM-less devices I suggest removing the flash-less 
code entirely from the init flow.

Re: [PATCH v1 iwl-net] igc: fix null pointer dereference in

[Kohei Enju]

On Thu, 5 Feb 2026 11:16:50 +0200, "Lifshits, Vitaly" wrote:

> On 2/5/2026 10:50 AM, Kohei Enju wrote:
> > On devices without NVM, hw->nvm.ops.validate is set to NULL, therefore
> > functions that perform EEPROM-related operations such as
> > igc_ethtool_set_eeprom() and igc_probe() check for NVM presence in
> > advance. However igc_eeprom_test() unconditionally calls
> > hw->nvm.ops.validate(), potentially causing a null pointer dereference.
> > 
> > NVM-less devices may not be common but possible, so add NULL check
> > before calling hw->nvm.ops.validate().
> > 
> > Fixes: f026d8ca2904 ("igc: add support to eeprom, registers and link self-tests")
> > Signed-off-by: Kohei Enju <kohei@enjuk.jp>
> > ---
> >   drivers/net/ethernet/intel/igc/igc_diag.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/igc/igc_diag.c b/drivers/net/ethernet/intel/igc/igc_diag.c
> > index a43d7244ee70..973d26a5a6c9 100644
> > --- a/drivers/net/ethernet/intel/igc/igc_diag.c
> > +++ b/drivers/net/ethernet/intel/igc/igc_diag.c
> > @@ -158,7 +158,7 @@ bool igc_eeprom_test(struct igc_adapter *adapter, u64 *data)
> >   
> >   	*data = 0;
> >   
> > -	if (hw->nvm.ops.validate(hw) != IGC_SUCCESS) {
> > +	if (hw->nvm.ops.validate && hw->nvm.ops.validate(hw) != IGC_SUCCESS) {
> >   		*data = 1;
> >   		return false;
> >   	}
> 
> 
> Hi Kohei,
> 
> Thank you for your patch.
> 
> Since there are no NVM-less devices I suggest removing the flash-less 
> code entirely from the init flow.

Oh, I see there're no NVM-less devices. Then removing sounds good to me.

Could you clarify what you mean by "init flow"? Do you mean removing
only the flash-less branch in igc_init_nvm_params_i225(), or removing
all flash-less related code including igc_get_flash_presence_i225() and
its callers?

After clarification, I'd love to work on it. Thank you for taking a
look!

Re: [PATCH v1 iwl-net] igc: fix null pointer dereference in

[Lifshits, Vitaly]

On 2/5/2026 11:26 AM, Kohei Enju wrote:
> On Thu, 5 Feb 2026 11:16:50 +0200, "Lifshits, Vitaly" wrote:
> 
>> On 2/5/2026 10:50 AM, Kohei Enju wrote:
>>> On devices without NVM, hw->nvm.ops.validate is set to NULL, therefore
>>> functions that perform EEPROM-related operations such as
>>> igc_ethtool_set_eeprom() and igc_probe() check for NVM presence in
>>> advance. However igc_eeprom_test() unconditionally calls
>>> hw->nvm.ops.validate(), potentially causing a null pointer dereference.
>>>
>>> NVM-less devices may not be common but possible, so add NULL check
>>> before calling hw->nvm.ops.validate().
>>>
>>> Fixes: f026d8ca2904 ("igc: add support to eeprom, registers and link self-tests")
>>> Signed-off-by: Kohei Enju <kohei@enjuk.jp>
>>> ---
>>>    drivers/net/ethernet/intel/igc/igc_diag.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/igc/igc_diag.c b/drivers/net/ethernet/intel/igc/igc_diag.c
>>> index a43d7244ee70..973d26a5a6c9 100644
>>> --- a/drivers/net/ethernet/intel/igc/igc_diag.c
>>> +++ b/drivers/net/ethernet/intel/igc/igc_diag.c
>>> @@ -158,7 +158,7 @@ bool igc_eeprom_test(struct igc_adapter *adapter, u64 *data)
>>>    
>>>    	*data = 0;
>>>    
>>> -	if (hw->nvm.ops.validate(hw) != IGC_SUCCESS) {
>>> +	if (hw->nvm.ops.validate && hw->nvm.ops.validate(hw) != IGC_SUCCESS) {
>>>    		*data = 1;
>>>    		return false;
>>>    	}
>>
>>
>> Hi Kohei,
>>
>> Thank you for your patch.
>>
>> Since there are no NVM-less devices I suggest removing the flash-less
>> code entirely from the init flow.
> 
> Oh, I see there're no NVM-less devices. Then removing sounds good to me.
> 
> Could you clarify what you mean by "init flow"? Do you mean removing
> only the flash-less branch in igc_init_nvm_params_i225(), or removing
> all flash-less related code including igc_get_flash_presence_i225() and
> its callers?
> 
> After clarification, I'd love to work on it. Thank you for taking a
> look!

No, you shouldn’t remove this function.

However, if for any reason the flash is not present, the driver should 
fail initialization.

There are two related places that need to be updated to enforce this:

igc_probe() in igc_main.c
igc_init_nvm_params_i225() in igc_i225.c

This way we avoid supporting a configuration that doesn’t exist, and we 
prevent the driver from partially initializing in an invalid state.

Re: [PATCH v1 iwl-net] igc: fix null pointer dereference in

[Kohei Enju]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3583 bytes --]

On Thu, 5 Feb 2026 12:16:06 +0200, "Lifshits, Vitaly" wrote:

> >> Hi Kohei,
> >>
> >> Thank you for your patch.
> >>
> >> Since there are no NVM-less devices I suggest removing the flash-less
> >> code entirely from the init flow.
> > 
> > Oh, I see there're no NVM-less devices. Then removing sounds good to me.
> > 
> > Could you clarify what you mean by "init flow"? Do you mean removing
> > only the flash-less branch in igc_init_nvm_params_i225(), or removing
> > all flash-less related code including igc_get_flash_presence_i225() and
> > its callers?
> > 
> > After clarification, I'd love to work on it. Thank you for taking a
> > look!
> 
> No, you shouldn’t remove this function.
> 
> However, if for any reason the flash is not present, the driver should 
> fail initialization.

I see. I understand we should fail igc_probe() for NVM-less devices.

> 
> There are two related places that need to be updated to enforce this:
> 
> igc_probe() in igc_main.c
> igc_init_nvm_params_i225() in igc_i225.c
> 
> This way we avoid supporting a configuration that doesn’t exist, and we 
> prevent the driver from partially initializing in an invalid state.

As far as I've skimmed the code, the only call trace is:

igc_probe()
  ei->get_invariants() (always igc_get_invariants_base())
    igc_init_nvm_params_i225()

so modifying igc_init_nvm_params_i225() is sufficient and IIUC we don't
have to modify igc_probe().

igc_init_nvm_params_i225() returns -EIO when there is no NVM, and its
caller igc_get_invariants_base() propagates the error back to
igc_probe().
Note that igc_get_invariants_base() currently ignores the return value
of igc_init_nvm_params_i225(), so I added that check as well.

diff --git a/drivers/net/ethernet/intel/igc/igc_base.c b/drivers/net/ethernet/intel/igc/igc_base.c
index 1613b562d17c..e4200279e15f 100644
--- a/drivers/net/ethernet/intel/igc/igc_base.c
+++ b/drivers/net/ethernet/intel/igc/igc_base.c
@@ -235,6 +235,9 @@ static s32 igc_get_invariants_base(struct igc_hw *hw)
                break;
        }

+       if (ret_val)
+               goto out;
+
        /* setup PHY parameters */
        ret_val = igc_init_phy_params_base(hw);
        if (ret_val)
diff --git a/drivers/net/ethernet/intel/igc/igc_i225.c b/drivers/net/ethernet/intel/igc/igc_i225.c
index 5226d10cc95b..ee1a8eeed9d5 100644
--- a/drivers/net/ethernet/intel/igc/igc_i225.c
+++ b/drivers/net/ethernet/intel/igc/igc_i225.c
@@ -476,21 +476,17 @@ s32 igc_init_nvm_params_i225(struct igc_hw *hw)
 {
        struct igc_nvm_info *nvm = &hw->nvm;

+       /* fail initialization for NVM-less devices */
+       if (!igc_get_flash_presence_i225(hw))
+               return -EIO;
+
        nvm->ops.acquire = igc_acquire_nvm_i225;
        nvm->ops.release = igc_release_nvm_i225;
+       nvm->ops.read = igc_read_nvm_srrd_i225;
+       nvm->ops.write = igc_write_nvm_srwr_i225;
+       nvm->ops.validate = igc_validate_nvm_checksum_i225;
+       nvm->ops.update = igc_update_nvm_checksum_i225;

-       /* NVM Function Pointers */
-       if (igc_get_flash_presence_i225(hw)) {
-               nvm->ops.read = igc_read_nvm_srrd_i225;
-               nvm->ops.write = igc_write_nvm_srwr_i225;
-               nvm->ops.validate = igc_validate_nvm_checksum_i225;
-               nvm->ops.update = igc_update_nvm_checksum_i225;
-       } else {
-               nvm->ops.read = igc_read_nvm_eerd;
-               nvm->ops.write = NULL;
-               nvm->ops.validate = NULL;
-               nvm->ops.update = NULL;
-       }
        return 0;
 }

Does this diff make sense to you?