We should not pass 0 nmsgs to i2c_transfer(), since it will cause
warning. This patch adds missing validation check to prevent passing
0 nmsgs to i2c_transfer().

Reported-and-tested-by: syzbot+e417648b303855b91d8a@syzkaller.appspotmail.com
Fixes: 7d5cb45655f2 ("i2c compat ioctls: move to ->compat_ioctl()")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---

Changes in v2:
	- Fixed typos in commit message

---
 drivers/i2c/i2c-dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index bce0e8bb7852..3b54efa4b1ec 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -535,7 +535,7 @@ static long compat_i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned lo
 				   sizeof(rdwr_arg)))
 			return -EFAULT;
 
-		if (rdwr_arg.nmsgs > I2C_RDWR_IOCTL_MAX_MSGS)
+		if (!rdwr_arg.nmsgs || rdwr_arg.nmsgs > I2C_RDWR_IOCTL_MAX_MSGS)
 			return -EINVAL;
 
 		rdwr_pa = kmalloc_array(rdwr_arg.nmsgs, sizeof(struct i2c_msg),
-- 
2.34.1

> -		if (rdwr_arg.nmsgs > I2C_RDWR_IOCTL_MAX_MSGS)
> +		if (!rdwr_arg.nmsgs || rdwr_arg.nmsgs > I2C_RDWR_IOCTL_MAX_MSGS)
>  			return -EINVAL;

Shouldn't we check the msgs pointer as well while we are here? Like in
the non-compat IOCTL code:

443  if (!rdwr_arg.msgs || rdwr_arg.nmsgs == 0)
444          return -EINVAL;

On 12/31/21 01:20, Wolfram Sang wrote:
>> -		if (rdwr_arg.nmsgs > I2C_RDWR_IOCTL_MAX_MSGS)
>> +		if (!rdwr_arg.nmsgs || rdwr_arg.nmsgs > I2C_RDWR_IOCTL_MAX_MSGS)
>>  			return -EINVAL;
> 
> Shouldn't we check the msgs pointer as well while we are here? Like in
> the non-compat IOCTL code:
> 
> 443  if (!rdwr_arg.msgs || rdwr_arg.nmsgs == 0)
> 444          return -EINVAL;
> 
> 

Sure! Will prepare v2, thanks for review


With regards,
Pavel Skripkin

Wrong user data may cause warning in i2c_transfer(), ex: zero msgs.
Userspace should not be able to trigger warnings, so this patch adds
validation checks for user data in compact ioctl to prevent reported
warnings

Reported-and-tested-by: syzbot+e417648b303855b91d8a@syzkaller.appspotmail.com
Fixes: 7d5cb45655f2 ("i2c compat ioctls: move to ->compat_ioctl()")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---

Changes in v3
	- Add rdwr_arg.nmsgs == 0 check as Wolfram suggested

Changes in v2:
	- Fixed typos in commit message

---
 drivers/i2c/i2c-dev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index bce0e8bb7852..cf5d049342ea 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -535,6 +535,9 @@ static long compat_i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned lo
 				   sizeof(rdwr_arg)))
 			return -EFAULT;
 
+		if (!rdwr_arg.msgs || rdwr_arg.nmsgs == 0)
+			return -EINVAL;
+
 		if (rdwr_arg.nmsgs > I2C_RDWR_IOCTL_MAX_MSGS)
 			return -EINVAL;
 
-- 
2.34.1

On Fri, Dec 31, 2021 at 01:47:50AM +0300, Pavel Skripkin wrote:
> Wrong user data may cause warning in i2c_transfer(), ex: zero msgs.
> Userspace should not be able to trigger warnings, so this patch adds
> validation checks for user data in compact ioctl to prevent reported
> warnings
> 
> Reported-and-tested-by: syzbot+e417648b303855b91d8a@syzkaller.appspotmail.com
> Fixes: 7d5cb45655f2 ("i2c compat ioctls: move to ->compat_ioctl()")
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>

Applied to for-current, thanks!