[PATCH] i2c-core: Expand buffers used in i2c_smbus_xfer_emulated()

[Joel Stanley]

On Fri, Mar 10, 2017 at 10:51 PM, Andrew Jeffery <andrew at aj.id.au> wrote:
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -2894,16 +2894,26 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter *adapter, u16 addr,
>                                    char read_write, u8 command, int size,
>                                    union i2c_smbus_data *data)
>  {
> -       /* So we need to generate a series of msgs. In the case of writing, we
> -         need to use only one message; when reading, we need two. We initialize
> -         most things with sane defaults, to keep the code below somewhat
> -         simpler. */
> -       unsigned char msgbuf0[I2C_SMBUS_BLOCK_MAX+3];
> -       unsigned char msgbuf1[I2C_SMBUS_BLOCK_MAX+2];
> +       /*
> +        * So we need to generate a series of msgs. In the case of writing, we
> +        * need to use only one message; when reading, we need two. We initialize
> +        * most things with sane defaults, to keep the code below somewhat
> +        * simpler.
> +        *
> +        * Also, allocate 256 byte buffers as protection against suspect
> +        * hardware like the UCD90xx, where on occasion the returned block
> +        * length is 0xff[1].
> +        *
> +        * [1] https://github.com/openbmc/openbmc/issues/998
> +        */
> +       unsigned char *msgbuf0 = kmalloc(0x100, GFP_KERNEL);
> +       unsigned char *msgbuf1 = kmalloc(0x100, GFP_KERNEL);

kzalloc?


> @@ -3056,11 +3077,17 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter *adapter, u16 addr,
>                         break;
>                 case I2C_SMBUS_BLOCK_DATA:
>                 case I2C_SMBUS_BLOCK_PROC_CALL:
> -                       for (i = 0; i < msgbuf1[0] + 1; i++)
> +                       for (i = 0;
> +                            i < min((size_t)(msgbuf1[0] + 1), sizeof(*data));
> +                            i++)

Lets assume msgbuf1[0] + 1 > sizeof(*data) means junk in the buffer
and return an error instead of a buffer of junk.

If you want to print a debug message there it might be useful for
future debugging.

>                                 data->block[i] = msgbuf1[i];
>                         break;
>                 }
> -       return 0;
> +out:
> +       kfree(msgbuf0);
> +       kfree(msgbuf1);

Do we cleanup in all paths? Is there any value in using devm_ apis?

Cheers,

Joel