Re: [BUG][PATCH] e1000: Fix invalid memory reference



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 <kaneshige.kenji@xxxxxxxxxxxxxx> 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 <kaneshige.kenji@xxxxxxxxxxxxxx>

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 majordomo@xxxxxxxxxxxxxxx
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


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



Relevant Pages

  • Re: Vortex full-duplex doesnt work?
    ... | ether=0,0,0x201,0,eth0 should set my 3c900 card to full duplex AUI. ... driver calls all of the right hooks for this to work. ... send the line "unsubscribe linux-kernel" in ...
    (Linux-Kernel)
  • Re: Linux in a binary world... a doomsday scenario
    ... standards to measure it by, but I get 2000fps or thereabouts out of ... Most people don't seem to have performance problems, with their radeons, so perhaps my card is a bit different and don't fit the driver. ... To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ ...
    (Linux-Kernel)
  • Re: kernel 2.6: cant get 3c575/PCMCIA working - other PCMCIA card work
    ... The card is a 3CCFE575BT-D. ... the driver was called 3c575_cs. ... Under 2.6 with the kernel drivers, ... send the line "unsubscribe linux-kernel" in ...
    (Linux-Kernel)
  • Re: Exposing ROMs though sysfs
    ... When the driver for one of these card loads it could ... >> copy of the ROM into RAM and cache it in a PCI structure. ... send the line "unsubscribe linux-kernel" in ...
    (Linux-Kernel)
  • RE: [UPDATED PATCH] EFI support for ia32 kernels
    ... >> reuse a single driver image for multiple architectures assuming there ... As one of the people responsible for the EFI Specification and our ... Perhaps the UNDI network card interface that Intel developed ... BIOS can't shadow that much ROM code. ...
    (Linux-Kernel)