[PATCH 2.6] I2C: Prevent buffer overflow on SMBus block read in i2c-viapro

From: Jean Delvare (khali_at_linux-fr.org)
Date: 01/25/05

  • Next message: Aurélien Jarno: "[PATCH 2.6] I2C: New chip driver: sis5595" 
    Date:	Tue, 25 Jan 2005 23:03:48 +0100
To: Greg KH <greg@kroah.com>, Linus Torvalds <torvalds@osdl.org>

    Hi Greg, Linus, all,

    I just hit a buffer overflow while playing around with i2cdump and
    i2c-viapro through i2c-dev. This is caused by a missing length check on
    a buffer operation when doing a SMBus block read in the i2c-viapro
    driver. The problem was already known and had been fixed upon report by
    Sergey Vlasov back in August 2003 in lm_sensors (2.4 kernel version of
    the driver) but for some reason it was never ported to the 2.6 kernel
    version.

    I am not a security expert but I would guess that such a buffer overflow
    could possibly be used to run arbitrary code in kernel space from user
    space through i2c-dev. The severity obviously depends on the permisions
    set on the i2c device files in /dev. Maybe it wouldn't be a bad idea to
    push this patch upstream rather sooner than later.

    While I was at it, I also changed a similar size check (for SMBus block
    write this time) in the same driver to use the correct constant
    I2C_SMBUS_BLOCK_MAX instead of its current numerical value. This doesn't
    change a thing at the moment but prevents another potential buffer
    overflow in case the value of I2C_SMBUS_BLOCK_MAX were to be changed in
    the future (admittedly unlikely though).

    Thanks.

    Signed-off-by: Jean Delvare <khali@linux-fr.org>

    --- linux-2.6.11-rc1-bk8/drivers/i2c/busses/i2c-viapro.c.orig 2005-01-21 20:05:05.000000000 +0100
    +++ linux-2.6.11-rc1-bk8/drivers/i2c/busses/i2c-viapro.c 2005-01-25 21:45:01.000000000 +0100
    @@ -233,8 +233,8 @@
                             len = data->block[0];
                             if (len < 0)
                                     len = 0;
    - if (len > 32)
    - len = 32;
    + if (len > I2C_SMBUS_BLOCK_MAX)
    + len = I2C_SMBUS_BLOCK_MAX;
                             outb_p(len, SMBHSTDAT0);
                             i = inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */
                             for (i = 1; i <= len; i++)
    @@ -268,6 +268,8 @@
                     break;
             case VT596_BLOCK_DATA:
                     data->block[0] = inb_p(SMBHSTDAT0);
    + if (data->block[0] > I2C_SMBUS_BLOCK_MAX)
    + data->block[0] = I2C_SMBUS_BLOCK_MAX;
                     i = inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */
                     for (i = 1; i <= data->block[0]; i++)
                             data->block[i] = inb_p(SMBBLKDAT); 

    -- 
Jean Delvare
http://khali.linux-fr.org/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
  • Next message: Aurélien Jarno: "[PATCH 2.6] I2C: New chip driver: sis5595"

    Relevant Pages
    Loading