e1000: Fix invalid memory reference

PATCH In review Kenji Kaneshige 3 participants 5 messages Dec 13, 2005 – Dec 19, 2005

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Kenji Kaneshige

author

2005-12-13 10:28 UTC #0 lore raw

PATCH 1/1 e1000: Fix invalid memory reference

Hi,

I encountered a kernel panic which was caused by the invalid memory access by e1000 driver. The following patch fixes this issue.

Thanks,
Kenji Kaneshige

This patch fixes invalid memory reference in the e1000 driver which would cause kernel panic.

Signed-off-by Kenji Kaneshige <[email protected]>

 drivers/net/e1000/e1000_param.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

drivers/net/e1000/e1000_param.c +7 -3

Index: linux-2.6.15-rc5/drivers/net/e1000/e1000_param.c
===================================================================
@@ -545,7 +545,7 @@ e1000_check_fiber_options(struct e1000_a
 static void __devinit
 e1000_check_copper_options(struct e1000_adapter *adapter)
 {
-	int speed, dplx;
+	int speed, dplx, an;
 	int bd = adapter->bd_number;
 
 	{ /* Speed */

@@ -641,8 +641,12 @@ e1000_check_copper_options(struct e1000_
 					 .p = an_list }}
 		};
 
-		int an = AutoNeg[bd];
-		e1000_validate_option(&an, &opt, adapter);
+		if (num_AutoNeg > bd) {
+			an = AutoNeg[bd];
+			e1000_validate_option(&an, &opt, adapter);
+		} else {
+			an = opt.def;
+		}
 		adapter->hw.autoneg_advertised = an;
 	}

Kenji Kaneshige

author

2005-12-19 02:35 UTC #2 lore raw

Hi Jeff,

Quoted 2 lines

Could you provide the test case you used to get the kernel panic? and system related information.

I encountered the kernel panic when I repeated pci hotplug with e1000 card on ia64 box. But please noted that the current e1000 driver always refers invalid memory regardless of pci hotplug.

In my understanding, e1000 driver set the adapter->bd_number incrementally when a new e1000 card is initialized. So first card has 0, second card has 1,..., as bd_number. On the other hand, num_AutoNeg is 0 on my environment because I don't put any module options for e1000 driver. Here, in the following code path you mentioned, "int an = AutoNeg[bd];" cause invalid memory reference.

Quoted 17 lines

   if ((num_AutoNeg > bd) && (speed != 0 || dplx != 0)) {
        DPRINTK(PROBE, INFO,
               "AutoNeg specified along with Speed or Duplex, "
               "parameter ignored\n");
        adapter->hw.autoneg_advertised = AUTONEG_ADV_DEFAULT;
    } else { /* Autoneg */
                          .
                          .
                          .

        int an = AutoNeg[bd];
        e1000_validate_option(&an, &opt, adapter);
        adapter->hw.autoneg_advertised = an;
    }

Quoted 3 lines

num_Autoneg > bd will never be true at this point in the code because we do the following test before we execute this branch.

Why????????
Do you mean (speed != 0 || dplx != 0) will always be true when num_Autoneg > bd is true? If yes, why do you need the following if statement? Do you mean the current e1000 driver has another bug?

Quoted 1 line

   if ((num_AutoNeg > bd) && (speed != 0 || dplx != 0)) {

Thanks,
Kenji Kaneshige

Quoted 81 lines · Jeff Kirsher

Quoted 46 lines · Kenji Kaneshige
Hi,
I encountered a kernel panic which was caused by the invalid memory access by e1000 driver. The following patch fixes this issue.
Thanks,
Kenji Kaneshige
This patch fixes invalid memory reference in the e1000 driver which would cause kernel panic.
Signed-off-by Kenji Kaneshige <[email protected]>
drivers/net/e1000/e1000_param.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
Index: linux-2.6.15-rc5/drivers/net/e1000/e1000_param.c
===================================================================
drivers/net/e1000/e1000_param.c +0 -0
@@ -545,7 +545,7 @@ e1000_check_fiber_options(struct e1000_a
static void __devinit e1000_check_copper_options(struct e1000_adapter *adapter) {
drivers/net/e1000/e1000_param.c +7 -4
-       int speed, dplx;
+       int speed, dplx, an;
       int bd = adapter->bd_number;
 
       { /* Speed */
@@ -641,8 +641,12 @@ e1000_check_copper_options(struct e1000_
                                        .p = an_list }}
               };
 
-               int an = AutoNeg[bd];
-               e1000_validate_option(&an, &opt, adapter);
+               if (num_AutoNeg > bd) {
+                       an = AutoNeg[bd];
+                       e1000_validate_option(&an, &opt, adapter);
+               } else {
+                       an = opt.def;
+               }
               adapter->hw.autoneg_advertised = an;
       }
 
-
Quoted 5 lines
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Could you provide the test case you used to get the kernel panic? and system related information.
num_Autoneg > bd will never be true at this point in the code because we do the following test before we execute this branch.
   if ((num_AutoNeg > bd) && (speed != 0 || dplx != 0)) {
        DPRINTK(PROBE, INFO,
               "AutoNeg specified along with Speed or Duplex, "
               "parameter ignored\n");
        adapter->hw.autoneg_advertised = AUTONEG_ADV_DEFAULT;
    } else { /* Autoneg */
                          .
                          .
                          .
        int an = AutoNeg[bd];
        e1000_validate_option(&an, &opt, adapter);
        adapter->hw.autoneg_advertised = an;
    }
--
Cheers,
Jeff

Jeff Kirsher

2005-12-19 21:26 UTC #3 lore raw

In reply to Kenji Kaneshige

Quoted 46 lines · Kenji Kaneshige

Hi,

I encountered a kernel panic which was caused by the invalid memory access by e1000 driver. The following patch fixes this issue.

Thanks,
Kenji Kaneshige

This patch fixes invalid memory reference in the e1000 driver which would cause kernel panic.

Signed-off-by Kenji Kaneshige <[email protected]>

 drivers/net/e1000/e1000_param.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

Index: linux-2.6.15-rc5/drivers/net/e1000/e1000_param.c
===================================================================

drivers/net/e1000/e1000_param.c +7 -4

@@ -545,7 +545,7 @@ e1000_check_fiber_options(struct e1000_a
 static void __devinit
 e1000_check_copper_options(struct e1000_adapter *adapter)
 {
-       int speed, dplx;
+       int speed, dplx, an;
        int bd = adapter->bd_number;
 
        { /* Speed */

@@ -641,8 +641,12 @@ e1000_check_copper_options(struct e1000_
                                         .p = an_list }}
                };
 
-               int an = AutoNeg[bd];
-               e1000_validate_option(&an, &opt, adapter);
+               if (num_AutoNeg > bd) {
+                       an = AutoNeg[bd];
+                       e1000_validate_option(&an, &opt, adapter);
+               } else {
+                       an = opt.def;
+               }
                adapter->hw.autoneg_advertised = an;
        }
 
-

Quoted 5 lines

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Sorry, you are right. Looks fine.

ACK.

--
Cheers,
Jeff

Jesse Brandeburg

2005-12-19 22:36 UTC #4 lore raw

PATCH 1/1 e1000: Fix invalid memory reference

In reply to Kenji Kaneshige

Thanks for the patch!
Jeff, if you would be so kind as to apply this, thanks...

This patch fixes invalid memory reference in the e1000 driver which would cause kernel panic.

Signed-off-by Kenji Kaneshige <[email protected]>

Acked-by Jesse Brandeburg <[email protected]>

---------- Forwarded message ----------
From: Kenji Kaneshige <[email protected]>
Date: Dec 13, 2005 2:27 AM
Subject: [BUG][PATCH] e1000: Fix invalid memory reference
To: Linux Kernel Mailing List <[email protected]>
Cc: Andrew Morton <[email protected]>

Hi,

I encountered a kernel panic which was caused by the invalid memory access by e1000 driver. The following patch fixes this issue.

Thanks,
Kenji Kaneshige

This patch fixes invalid memory reference in the e1000 driver which would cause kernel panic.

Signed-off-by Kenji Kaneshige <[email protected]>

 drivers/net/e1000/e1000_param.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

drivers/net/e1000/e1000_param.c +7 -3

Index: linux-2.6.15-rc5/drivers/net/e1000/e1000_param.c
===================================================================
@@ -545,7 +545,7 @@ e1000_check_fiber_options(struct e1000_a
 static void __devinit
 e1000_check_copper_options(struct e1000_adapter *adapter)
 {
-       int speed, dplx;
+       int speed, dplx, an;
        int bd = adapter->bd_number;
 
        { /* Speed */

@@ -641,8 +641,12 @@ e1000_check_copper_options(struct e1000_
                                         .p = an_list }}
                };
 
-               int an = AutoNeg[bd];
-               e1000_validate_option(&an, &opt, adapter);
+               if (num_AutoNeg > bd) {
+                       an = AutoNeg[bd];
+                       e1000_validate_option(&an, &opt, adapter);
+               } else {
+                       an = opt.def;
+               }
                adapter->hw.autoneg_advertised = an;
        }