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
===================================================================
--- linux-2.6.15-rc5.orig/drivers/net/e1000/e1000_param.c
+++ 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;
}
On 12/13/05, Kenji Kaneshige <[email protected]> wrote:
> 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
> ===================================================================
> --- linux-2.6.15-rc5.orig/drivers/net/e1000/e1000_param.c
> +++ 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;
> }
>
> -
> 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
Hi Jeff,
> 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.
> 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;
> }
> 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?
> if ((num_AutoNeg > bd) && (speed != 0 || dplx != 0)) {
Thanks,
Kenji Kaneshige
Jeff Kirsher wrote:
> On 12/13/05, Kenji Kaneshige <[email protected]> wrote:
>
>>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
>>===================================================================
>>--- linux-2.6.15-rc5.orig/drivers/net/e1000/e1000_param.c
>>+++ 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;
>> }
>>
>>-
>>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
>
On 12/13/05, Kenji Kaneshige <[email protected]> wrote:
> 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
> ===================================================================
> --- linux-2.6.15-rc5.orig/drivers/net/e1000/e1000_param.c
> +++ 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;
> }
>
> -
> 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
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(-)
Index: linux-2.6.15-rc5/drivers/net/e1000/e1000_param.c
===================================================================
--- linux-2.6.15-rc5.orig/drivers/net/e1000/e1000_param.c
+++ 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;
}