• Jacob Keller's avatar
    e100: fix buffer overrun in e100_get_regs · 51032e6f
    Jacob Keller authored
    The e100_get_regs function is used to implement a simple register dump
    for the e100 device. The data is broken into a couple of MAC control
    registers, and then a series of PHY registers, followed by a memory dump
    buffer.
    
    The total length of the register dump is defined as (1 + E100_PHY_REGS)
    * sizeof(u32) + sizeof(nic->mem->dump_buf).
    
    The logic for filling in the PHY registers uses a convoluted inverted
    count for loop which counts from E100_PHY_REGS (0x1C) down to 0, and
    assigns the slots 1 + E100_PHY_REGS - i. The first loop iteration will
    fill in [1] and the final loop iteration will fill in [1 + 0x1C]. This
    is actually one more than the supposed number of PHY registers.
    
    The memory dump buffer is then filled into the space at
    [2 + E100_PHY_REGS] which will cause that memcpy to assign 4 bytes past
    the total size.
    
    The end result is that we overrun the total buffer size allocated by the
    kernel, which could lead to a panic or other issues due to memory
    corruption.
    
    It is difficult to determine the actual total number of registers
    here. The only 8255x datasheet I could find indicates there are 28 total
    MDI registers. However, we're reading 29 here, and reading them in
    reverse!
    
    In addition, the ethtool e100 register dump interface appears to read
    the first PHY register to determine if the device is in MDI or MDIx
    mode. This doesn't appear to be documented anywhere within the 8255x
    datasheet. I can only assume it must be in register 28 (the extra
    register we're reading here).
    
    Lets not change any of the intended meaning of what we copy here. Just
    extend the space by 4 bytes to account for the extra register and
    continue copying the data out in the same order.
    
    Change the E100_PHY_REGS value to be the correct total (29) so that the
    total register dump size is calculated properly. Fix the offset for
    where we copy the dump buffer so that it doesn't overrun the total size.
    
    Re-write the for loop to use counting up instead of the convoluted
    down-counting. Correct the mdio_read offset to use the 0-based register
    offsets, but maintain the bizarre reverse ordering so that we have the
    ABI expected by applications like ethtool. This requires and additional
    subtraction of 1. It seems a bit odd but it makes the flow of assignment
    into the register buffer easier to follow.
    
    Fixes: 1da177e4 ("Linux-2.6.12-rc2")
    Reported-by: default avatarFelicitas Hetzelt <felicitashetzelt@gmail.com>
    Signed-off-by: default avatarJacob Keller <jacob.e.keller@intel.com>
    Tested-by: default avatarJacob Keller <jacob.e.keller@intel.com>
    Signed-off-by: default avatarTony Nguyen <anthony.l.nguyen@intel.com>
    51032e6f
e100.c 90.5 KB