linux-kernel - Re: [PATCH RESEND v2] HID: cp2112: Add parameter validation to data length

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CABbzaOUQ04seRWn3ik2fnoMddc5uNfzNVfOuNcC+i+dT43isYg@mail.gmail.com>
Date: Fri, 26 Sep 2025 19:47:17 +0530
From: Deepak Sharma <deepak.sharma.472935@...il.com>
To: jikos@...nel.org, bentiss@...nel.org
Cc: linux-input@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-kernel-mentees@...ts.linux.dev, skhan@...uxfoundation.org, 
	david.hunter.linux@...il.com, 
	syzbot+7617e19c8a59edfbd879@...kaller.appspotmail.com
Subject: Re: [PATCH RESEND v2] HID: cp2112: Add parameter validation to data length

Please ignore this patch. I did this patch quite a while ago and put
the change log mistakenly into the commit message. Will send a v3
fixing it. Sorry for being clumsy with that

Thanks,
Deepak Sharma

On Fri, Sep 26, 2025 at 7:39 PM Deepak Sharma
<deepak.sharma.472935@...il.com> wrote:
>
> This is v2 for the earlier patch, where a few bounds check were
> unnecessarily strict. This patch also removes the use of magic numbers
>
> Syzkaller reported a stack OOB access in cp2112_write_req caused by lack
> of parameter validation for the user input in I2C SMBUS ioctl codeflow
> in the report
>
> I2C device drivers are "responsible for checking all the parameters that
> come from user-space for validity" as specified at Documentation/i2c/dev-interface
>
> Add the parameter validation for the data->block[0] to be bounded by
> I2C_SMBUS_BLOCK_MAX + the additional compatibility padding
>
> Reported-by: syzbot+7617e19c8a59edfbd879@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7617e19c8a59edfbd879
> Tested-by: syzbot+7617e19c8a59edfbd879@...kaller.appspotmail.com
> Signed-off-by: Deepak Sharma <deepak.sharma.472935@...il.com>
> ---
>  drivers/hid/hid-cp2112.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> index 482f62a78c41..13dcd2470d92 100644
> --- a/drivers/hid/hid-cp2112.c
> +++ b/drivers/hid/hid-cp2112.c
> @@ -689,7 +689,14 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr,
>                         count = cp2112_write_read_req(buf, addr, read_length,
>                                                       command, NULL, 0);
>                 } else {
> -                       count = cp2112_write_req(buf, addr, command,
> +                       /* Copy starts from data->block[1] so the length can
> +                        * be at max I2C_SMBUS_CLOCK_MAX + 1
> +                        */
> +
> +                       if (data->block[0] > I2C_SMBUS_BLOCK_MAX + 1)
> +                               count = -EINVAL;
> +                       else
> +                               count = cp2112_write_req(buf, addr, command,
>                                                  data->block + 1,
>                                                  data->block[0]);
>                 }
> @@ -700,7 +707,14 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr,
>                                                       I2C_SMBUS_BLOCK_MAX,
>                                                       command, NULL, 0);
>                 } else {
> -                       count = cp2112_write_req(buf, addr, command,
> +                       /* data_length here is data->block[0] + 1
> +                        * so make sure that the data->block[0] is
> +                        * less than or equals I2C_SMBUS_BLOCK_MAX + 1
> +                       */
> +                       if (data->block[0] > I2C_SMBUS_BLOCK_MAX + 1)
> +                               count = -EINVAL;
> +                       else
> +                               count = cp2112_write_req(buf, addr, command,
>                                                  data->block,
>                                                  data->block[0] + 1);
>                 }
> @@ -709,7 +723,14 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr,
>                 size = I2C_SMBUS_BLOCK_DATA;
>                 read_write = I2C_SMBUS_READ;
>
> -               count = cp2112_write_read_req(buf, addr, I2C_SMBUS_BLOCK_MAX,
> +               /* data_length is data->block[0] + 1, so
> +                * so data->block[0] should be less than or
> +                * equal to the I2C_SMBUS_BLOCK_MAX + 1
> +               */
> +               if (data->block[0] > I2C_SMBUS_BLOCK_MAX + 1)
> +                       count = -EINVAL;
> +               else
> +                       count = cp2112_write_read_req(buf, addr, I2C_SMBUS_BLOCK_MAX,
>                                               command, data->block,
>                                               data->block[0] + 1);
>                 break;
> --
> 2.51.0
>