[PATCH RESEND v2] HID: cp2112: Add parameter validation to data length

Deepak Sharma posted 1 patch 5 days, 7 hours ago
There is a newer version of this series
drivers/hid/hid-cp2112.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
[PATCH RESEND v2] HID: cp2112: Add parameter validation to data length
Posted by Deepak Sharma 5 days, 7 hours ago
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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7617e19c8a59edfbd879
Tested-by: syzbot+7617e19c8a59edfbd879@syzkaller.appspotmail.com
Signed-off-by: Deepak Sharma <deepak.sharma.472935@gmail.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
Re: [PATCH RESEND v2] HID: cp2112: Add parameter validation to data length
Posted by Deepak Sharma 5 days, 7 hours ago
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@gmail.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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7617e19c8a59edfbd879
> Tested-by: syzbot+7617e19c8a59edfbd879@syzkaller.appspotmail.com
> Signed-off-by: Deepak Sharma <deepak.sharma.472935@gmail.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
>