* [PATCH] i2c: don't pass 0 nmsgs to i2c_transfer
@ 2021-12-25 14:28 Pavel Skripkin
2021-12-25 18:24 ` [PATCH v2] " Pavel Skripkin
0 siblings, 1 reply; 6+ messages in thread
From: Pavel Skripkin @ 2021-12-25 14:28 UTC (permalink / raw)
To: wsa, viro
Cc: linux-i2c, linux-kernel, Pavel Skripkin,
syzbot+e417648b303855b91d8a
We should not pass 0 nmsgs to i2c_transfer(), since it will cause
warning. This patch addd missing validion check to prevent passing
0 nmsgs.
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>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2] i2c: don't pass 0 nmsgs to i2c_transfer
2021-12-25 14:28 [PATCH] i2c: don't pass 0 nmsgs to i2c_transfer Pavel Skripkin
@ 2021-12-25 18:24 ` Pavel Skripkin
2021-12-30 22:20 ` Wolfram Sang
0 siblings, 1 reply; 6+ messages in thread
From: Pavel Skripkin @ 2021-12-25 18:24 UTC (permalink / raw)
To: wsa, viro
Cc: linux-i2c, linux-kernel, Pavel Skripkin,
syzbot+e417648b303855b91d8a
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] i2c: don't pass 0 nmsgs to i2c_transfer
2021-12-25 18:24 ` [PATCH v2] " Pavel Skripkin
@ 2021-12-30 22:20 ` Wolfram Sang
2021-12-30 22:26 ` Pavel Skripkin
2021-12-30 22:47 ` [PATCH v3] i2c: validate user data in compat ioctl Pavel Skripkin
0 siblings, 2 replies; 6+ messages in thread
From: Wolfram Sang @ 2021-12-30 22:20 UTC (permalink / raw)
To: Pavel Skripkin; +Cc: viro, linux-i2c, linux-kernel, syzbot+e417648b303855b91d8a
[-- Attachment #1: Type: text/plain, Size: 322 bytes --]
> - 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;
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] i2c: don't pass 0 nmsgs to i2c_transfer
2021-12-30 22:20 ` Wolfram Sang
@ 2021-12-30 22:26 ` Pavel Skripkin
2021-12-30 22:47 ` [PATCH v3] i2c: validate user data in compat ioctl Pavel Skripkin
1 sibling, 0 replies; 6+ messages in thread
From: Pavel Skripkin @ 2021-12-30 22:26 UTC (permalink / raw)
To: Wolfram Sang, viro, linux-i2c, linux-kernel,
syzbot+e417648b303855b91d8a
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3] i2c: validate user data in compat ioctl
2021-12-30 22:20 ` Wolfram Sang
2021-12-30 22:26 ` Pavel Skripkin
@ 2021-12-30 22:47 ` Pavel Skripkin
2021-12-31 13:31 ` Wolfram Sang
1 sibling, 1 reply; 6+ messages in thread
From: Pavel Skripkin @ 2021-12-30 22:47 UTC (permalink / raw)
To: wsa, viro
Cc: linux-i2c, linux-kernel, Pavel Skripkin,
syzbot+e417648b303855b91d8a
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3] i2c: validate user data in compat ioctl
2021-12-30 22:47 ` [PATCH v3] i2c: validate user data in compat ioctl Pavel Skripkin
@ 2021-12-31 13:31 ` Wolfram Sang
0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2021-12-31 13:31 UTC (permalink / raw)
To: Pavel Skripkin; +Cc: viro, linux-i2c, linux-kernel, syzbot+e417648b303855b91d8a
[-- Attachment #1: Type: text/plain, Size: 541 bytes --]
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!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-31 13:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-25 14:28 [PATCH] i2c: don't pass 0 nmsgs to i2c_transfer Pavel Skripkin
2021-12-25 18:24 ` [PATCH v2] " Pavel Skripkin
2021-12-30 22:20 ` Wolfram Sang
2021-12-30 22:26 ` Pavel Skripkin
2021-12-30 22:47 ` [PATCH v3] i2c: validate user data in compat ioctl Pavel Skripkin
2021-12-31 13:31 ` Wolfram Sang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).