* [PATCH] RDMA/cm: Add min length checks to user structure copies
@ 2020-07-24 13:19 Jason Gunthorpe
2020-07-24 13:45 ` Leon Romanovsky
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jason Gunthorpe @ 2020-07-24 13:19 UTC (permalink / raw)
To: Leon Romanovsky, linux-rdma
Cc: syzbot+086ab5ca9eafd2379aa6, syzbot+7446526858b83c8828b2
These are missing throughout ucma, it harmlessly copies garbage from
userspace, but in this new code which uses min to compute the copy length
it can result in uninitialized stack memory. Check for minimum length at
the very start.
BUG: KMSAN: uninit-value in ucma_connect+0x2aa/0xab0 drivers/infiniband/core/ucma.c:1091
CPU: 0 PID: 8457 Comm: syz-executor069 Not tainted 5.8.0-rc5-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1df/0x240 lib/dump_stack.c:118
kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:121
__msan_warning+0x58/0xa0 mm/kmsan/kmsan_instr.c:215
ucma_connect+0x2aa/0xab0 drivers/infiniband/core/ucma.c:1091
ucma_write+0x5c5/0x630 drivers/infiniband/core/ucma.c:1764
do_loop_readv_writev fs/read_write.c:737 [inline]
do_iter_write+0x710/0xdc0 fs/read_write.c:1020
vfs_writev fs/read_write.c:1091 [inline]
do_writev+0x42d/0x8f0 fs/read_write.c:1134
__do_sys_writev fs/read_write.c:1207 [inline]
__se_sys_writev+0x9b/0xb0 fs/read_write.c:1204
__x64_sys_writev+0x4a/0x70 fs/read_write.c:1204
do_syscall_64+0xb0/0x150 arch/x86/entry/common.c:386
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Fixes: 34e2ab57a911 ("RDMA/ucma: Extend ucma_connect to receive ECE parameters")
Fixes: 0cb15372a615 ("RDMA/cma: Connect ECE to rdma_accept")
Reported-by: syzbot+086ab5ca9eafd2379aa6@syzkaller.appspotmail.com
Reported-by: syzbot+7446526858b83c8828b2@syzkaller.appspotmail.com
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
drivers/infiniband/core/ucma.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index 5b87eee8ccc8b6..d03dacaef78805 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -1084,6 +1084,8 @@ static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf,
size_t in_size;
int ret;
+ if (in_len < offsetofend(typeof(cmd), reserved))
+ return -EINVAL;
in_size = min_t(size_t, in_len, sizeof(cmd));
if (copy_from_user(&cmd, inbuf, in_size))
return -EFAULT;
@@ -1141,6 +1143,8 @@ static ssize_t ucma_accept(struct ucma_file *file, const char __user *inbuf,
size_t in_size;
int ret;
+ if (in_len < offsetofend(typeof(cmd), reserved))
+ return -EINVAL;
in_size = min_t(size_t, in_len, sizeof(cmd));
if (copy_from_user(&cmd, inbuf, in_size))
return -EFAULT;
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] RDMA/cm: Add min length checks to user structure copies
2020-07-24 13:19 [PATCH] RDMA/cm: Add min length checks to user structure copies Jason Gunthorpe
@ 2020-07-24 13:45 ` Leon Romanovsky
2020-07-24 13:52 ` Jason Gunthorpe
2020-07-24 19:33 ` Leon Romanovsky
2020-07-27 14:57 ` Jason Gunthorpe
2 siblings, 1 reply; 6+ messages in thread
From: Leon Romanovsky @ 2020-07-24 13:45 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-rdma, syzbot+086ab5ca9eafd2379aa6, syzbot+7446526858b83c8828b2
On Fri, Jul 24, 2020 at 10:19:29AM -0300, Jason Gunthorpe wrote:
> These are missing throughout ucma, it harmlessly copies garbage from
> userspace, but in this new code which uses min to compute the copy length
> it can result in uninitialized stack memory. Check for minimum length at
> the very start.
>
> BUG: KMSAN: uninit-value in ucma_connect+0x2aa/0xab0 drivers/infiniband/core/ucma.c:1091
> CPU: 0 PID: 8457 Comm: syz-executor069 Not tainted 5.8.0-rc5-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1df/0x240 lib/dump_stack.c:118
> kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:121
> __msan_warning+0x58/0xa0 mm/kmsan/kmsan_instr.c:215
> ucma_connect+0x2aa/0xab0 drivers/infiniband/core/ucma.c:1091
> ucma_write+0x5c5/0x630 drivers/infiniband/core/ucma.c:1764
> do_loop_readv_writev fs/read_write.c:737 [inline]
> do_iter_write+0x710/0xdc0 fs/read_write.c:1020
> vfs_writev fs/read_write.c:1091 [inline]
> do_writev+0x42d/0x8f0 fs/read_write.c:1134
> __do_sys_writev fs/read_write.c:1207 [inline]
> __se_sys_writev+0x9b/0xb0 fs/read_write.c:1204
> __x64_sys_writev+0x4a/0x70 fs/read_write.c:1204
> do_syscall_64+0xb0/0x150 arch/x86/entry/common.c:386
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> Fixes: 34e2ab57a911 ("RDMA/ucma: Extend ucma_connect to receive ECE parameters")
> Fixes: 0cb15372a615 ("RDMA/cma: Connect ECE to rdma_accept")
> Reported-by: syzbot+086ab5ca9eafd2379aa6@syzkaller.appspotmail.com
> Reported-by: syzbot+7446526858b83c8828b2@syzkaller.appspotmail.com
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
> drivers/infiniband/core/ucma.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index 5b87eee8ccc8b6..d03dacaef78805 100644
> --- a/drivers/infiniband/core/ucma.c
> +++ b/drivers/infiniband/core/ucma.c
> @@ -1084,6 +1084,8 @@ static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf,
> size_t in_size;
> int ret;
>
> + if (in_len < offsetofend(typeof(cmd), reserved))
> + return -EINVAL;
This check wasn't before the patches citied in Fixes lines. This is why
I didn't add them while extended ucma_*.
> in_size = min_t(size_t, in_len, sizeof(cmd));
> if (copy_from_user(&cmd, inbuf, in_size))
> return -EFAULT;
> @@ -1141,6 +1143,8 @@ static ssize_t ucma_accept(struct ucma_file *file, const char __user *inbuf,
> size_t in_size;
> int ret;
>
> + if (in_len < offsetofend(typeof(cmd), reserved))
> + return -EINVAL;
> in_size = min_t(size_t, in_len, sizeof(cmd));
> if (copy_from_user(&cmd, inbuf, in_size))
> return -EFAULT;
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RDMA/cm: Add min length checks to user structure copies
2020-07-24 13:45 ` Leon Romanovsky
@ 2020-07-24 13:52 ` Jason Gunthorpe
2020-07-24 19:33 ` Leon Romanovsky
0 siblings, 1 reply; 6+ messages in thread
From: Jason Gunthorpe @ 2020-07-24 13:52 UTC (permalink / raw)
To: Leon Romanovsky
Cc: linux-rdma, syzbot+086ab5ca9eafd2379aa6, syzbot+7446526858b83c8828b2
On Fri, Jul 24, 2020 at 04:45:45PM +0300, Leon Romanovsky wrote:
> On Fri, Jul 24, 2020 at 10:19:29AM -0300, Jason Gunthorpe wrote:
> > These are missing throughout ucma, it harmlessly copies garbage from
> > userspace, but in this new code which uses min to compute the copy length
> > it can result in uninitialized stack memory. Check for minimum length at
> > the very start.
> >
> > BUG: KMSAN: uninit-value in ucma_connect+0x2aa/0xab0 drivers/infiniband/core/ucma.c:1091
> > CPU: 0 PID: 8457 Comm: syz-executor069 Not tainted 5.8.0-rc5-syzkaller #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> > Call Trace:
> > __dump_stack lib/dump_stack.c:77 [inline]
> > dump_stack+0x1df/0x240 lib/dump_stack.c:118
> > kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:121
> > __msan_warning+0x58/0xa0 mm/kmsan/kmsan_instr.c:215
> > ucma_connect+0x2aa/0xab0 drivers/infiniband/core/ucma.c:1091
> > ucma_write+0x5c5/0x630 drivers/infiniband/core/ucma.c:1764
> > do_loop_readv_writev fs/read_write.c:737 [inline]
> > do_iter_write+0x710/0xdc0 fs/read_write.c:1020
> > vfs_writev fs/read_write.c:1091 [inline]
> > do_writev+0x42d/0x8f0 fs/read_write.c:1134
> > __do_sys_writev fs/read_write.c:1207 [inline]
> > __se_sys_writev+0x9b/0xb0 fs/read_write.c:1204
> > __x64_sys_writev+0x4a/0x70 fs/read_write.c:1204
> > do_syscall_64+0xb0/0x150 arch/x86/entry/common.c:386
> > entry_SYSCALL_64_after_hwframe+0x44/0xa9
> >
> > Fixes: 34e2ab57a911 ("RDMA/ucma: Extend ucma_connect to receive ECE parameters")
> > Fixes: 0cb15372a615 ("RDMA/cma: Connect ECE to rdma_accept")
> > Reported-by: syzbot+086ab5ca9eafd2379aa6@syzkaller.appspotmail.com
> > Reported-by: syzbot+7446526858b83c8828b2@syzkaller.appspotmail.com
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > drivers/infiniband/core/ucma.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> > index 5b87eee8ccc8b6..d03dacaef78805 100644
> > +++ b/drivers/infiniband/core/ucma.c
> > @@ -1084,6 +1084,8 @@ static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf,
> > size_t in_size;
> > int ret;
> >
> > + if (in_len < offsetofend(typeof(cmd), reserved))
> > + return -EINVAL;
>
> This check wasn't before the patches citied in Fixes lines. This is why
> I didn't add them while extended ucma_*.
It wasn't a bug before.
> > in_size = min_t(size_t, in_len, sizeof(cmd));
> > if (copy_from_user(&cmd, inbuf, in_size))
> > return -EFAULT;
Because this used to be:
if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
Which always completely filled the stack memory.
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RDMA/cm: Add min length checks to user structure copies
2020-07-24 13:52 ` Jason Gunthorpe
@ 2020-07-24 19:33 ` Leon Romanovsky
0 siblings, 0 replies; 6+ messages in thread
From: Leon Romanovsky @ 2020-07-24 19:33 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-rdma, syzbot+086ab5ca9eafd2379aa6, syzbot+7446526858b83c8828b2
On Fri, Jul 24, 2020 at 10:52:44AM -0300, Jason Gunthorpe wrote:
> On Fri, Jul 24, 2020 at 04:45:45PM +0300, Leon Romanovsky wrote:
> > On Fri, Jul 24, 2020 at 10:19:29AM -0300, Jason Gunthorpe wrote:
> > > These are missing throughout ucma, it harmlessly copies garbage from
> > > userspace, but in this new code which uses min to compute the copy length
> > > it can result in uninitialized stack memory. Check for minimum length at
> > > the very start.
> > >
> > > BUG: KMSAN: uninit-value in ucma_connect+0x2aa/0xab0 drivers/infiniband/core/ucma.c:1091
> > > CPU: 0 PID: 8457 Comm: syz-executor069 Not tainted 5.8.0-rc5-syzkaller #0
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> > > Call Trace:
> > > __dump_stack lib/dump_stack.c:77 [inline]
> > > dump_stack+0x1df/0x240 lib/dump_stack.c:118
> > > kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:121
> > > __msan_warning+0x58/0xa0 mm/kmsan/kmsan_instr.c:215
> > > ucma_connect+0x2aa/0xab0 drivers/infiniband/core/ucma.c:1091
> > > ucma_write+0x5c5/0x630 drivers/infiniband/core/ucma.c:1764
> > > do_loop_readv_writev fs/read_write.c:737 [inline]
> > > do_iter_write+0x710/0xdc0 fs/read_write.c:1020
> > > vfs_writev fs/read_write.c:1091 [inline]
> > > do_writev+0x42d/0x8f0 fs/read_write.c:1134
> > > __do_sys_writev fs/read_write.c:1207 [inline]
> > > __se_sys_writev+0x9b/0xb0 fs/read_write.c:1204
> > > __x64_sys_writev+0x4a/0x70 fs/read_write.c:1204
> > > do_syscall_64+0xb0/0x150 arch/x86/entry/common.c:386
> > > entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > >
> > > Fixes: 34e2ab57a911 ("RDMA/ucma: Extend ucma_connect to receive ECE parameters")
> > > Fixes: 0cb15372a615 ("RDMA/cma: Connect ECE to rdma_accept")
> > > Reported-by: syzbot+086ab5ca9eafd2379aa6@syzkaller.appspotmail.com
> > > Reported-by: syzbot+7446526858b83c8828b2@syzkaller.appspotmail.com
> > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > > drivers/infiniband/core/ucma.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> > > index 5b87eee8ccc8b6..d03dacaef78805 100644
> > > +++ b/drivers/infiniband/core/ucma.c
> > > @@ -1084,6 +1084,8 @@ static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf,
> > > size_t in_size;
> > > int ret;
> > >
> > > + if (in_len < offsetofend(typeof(cmd), reserved))
> > > + return -EINVAL;
> >
> > This check wasn't before the patches citied in Fixes lines. This is why
> > I didn't add them while extended ucma_*.
>
> It wasn't a bug before.
>
> > > in_size = min_t(size_t, in_len, sizeof(cmd));
> > > if (copy_from_user(&cmd, inbuf, in_size))
> > > return -EFAULT;
>
> Because this used to be:
>
> if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
>
> Which always completely filled the stack memory.
I see, thanks.
>
> Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RDMA/cm: Add min length checks to user structure copies
2020-07-24 13:19 [PATCH] RDMA/cm: Add min length checks to user structure copies Jason Gunthorpe
2020-07-24 13:45 ` Leon Romanovsky
@ 2020-07-24 19:33 ` Leon Romanovsky
2020-07-27 14:57 ` Jason Gunthorpe
2 siblings, 0 replies; 6+ messages in thread
From: Leon Romanovsky @ 2020-07-24 19:33 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-rdma, syzbot+086ab5ca9eafd2379aa6, syzbot+7446526858b83c8828b2
On Fri, Jul 24, 2020 at 10:19:29AM -0300, Jason Gunthorpe wrote:
> These are missing throughout ucma, it harmlessly copies garbage from
> userspace, but in this new code which uses min to compute the copy length
> it can result in uninitialized stack memory. Check for minimum length at
> the very start.
>
> BUG: KMSAN: uninit-value in ucma_connect+0x2aa/0xab0 drivers/infiniband/core/ucma.c:1091
> CPU: 0 PID: 8457 Comm: syz-executor069 Not tainted 5.8.0-rc5-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1df/0x240 lib/dump_stack.c:118
> kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:121
> __msan_warning+0x58/0xa0 mm/kmsan/kmsan_instr.c:215
> ucma_connect+0x2aa/0xab0 drivers/infiniband/core/ucma.c:1091
> ucma_write+0x5c5/0x630 drivers/infiniband/core/ucma.c:1764
> do_loop_readv_writev fs/read_write.c:737 [inline]
> do_iter_write+0x710/0xdc0 fs/read_write.c:1020
> vfs_writev fs/read_write.c:1091 [inline]
> do_writev+0x42d/0x8f0 fs/read_write.c:1134
> __do_sys_writev fs/read_write.c:1207 [inline]
> __se_sys_writev+0x9b/0xb0 fs/read_write.c:1204
> __x64_sys_writev+0x4a/0x70 fs/read_write.c:1204
> do_syscall_64+0xb0/0x150 arch/x86/entry/common.c:386
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> Fixes: 34e2ab57a911 ("RDMA/ucma: Extend ucma_connect to receive ECE parameters")
> Fixes: 0cb15372a615 ("RDMA/cma: Connect ECE to rdma_accept")
> Reported-by: syzbot+086ab5ca9eafd2379aa6@syzkaller.appspotmail.com
> Reported-by: syzbot+7446526858b83c8828b2@syzkaller.appspotmail.com
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
> drivers/infiniband/core/ucma.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RDMA/cm: Add min length checks to user structure copies
2020-07-24 13:19 [PATCH] RDMA/cm: Add min length checks to user structure copies Jason Gunthorpe
2020-07-24 13:45 ` Leon Romanovsky
2020-07-24 19:33 ` Leon Romanovsky
@ 2020-07-27 14:57 ` Jason Gunthorpe
2 siblings, 0 replies; 6+ messages in thread
From: Jason Gunthorpe @ 2020-07-27 14:57 UTC (permalink / raw)
To: Leon Romanovsky, linux-rdma
Cc: syzbot+086ab5ca9eafd2379aa6, syzbot+7446526858b83c8828b2
On Fri, Jul 24, 2020 at 10:19:29AM -0300, Jason Gunthorpe wrote:
> These are missing throughout ucma, it harmlessly copies garbage from
> userspace, but in this new code which uses min to compute the copy length
> it can result in uninitialized stack memory. Check for minimum length at
> the very start.
>
> BUG: KMSAN: uninit-value in ucma_connect+0x2aa/0xab0 drivers/infiniband/core/ucma.c:1091
> CPU: 0 PID: 8457 Comm: syz-executor069 Not tainted 5.8.0-rc5-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1df/0x240 lib/dump_stack.c:118
> kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:121
> __msan_warning+0x58/0xa0 mm/kmsan/kmsan_instr.c:215
> ucma_connect+0x2aa/0xab0 drivers/infiniband/core/ucma.c:1091
> ucma_write+0x5c5/0x630 drivers/infiniband/core/ucma.c:1764
> do_loop_readv_writev fs/read_write.c:737 [inline]
> do_iter_write+0x710/0xdc0 fs/read_write.c:1020
> vfs_writev fs/read_write.c:1091 [inline]
> do_writev+0x42d/0x8f0 fs/read_write.c:1134
> __do_sys_writev fs/read_write.c:1207 [inline]
> __se_sys_writev+0x9b/0xb0 fs/read_write.c:1204
> __x64_sys_writev+0x4a/0x70 fs/read_write.c:1204
> do_syscall_64+0xb0/0x150 arch/x86/entry/common.c:386
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> Fixes: 34e2ab57a911 ("RDMA/ucma: Extend ucma_connect to receive ECE parameters")
> Fixes: 0cb15372a615 ("RDMA/cma: Connect ECE to rdma_accept")
> Reported-by: syzbot+086ab5ca9eafd2379aa6@syzkaller.appspotmail.com
> Reported-by: syzbot+7446526858b83c8828b2@syzkaller.appspotmail.com
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> drivers/infiniband/core/ucma.c | 4 ++++
> 1 file changed, 4 insertions(+)
Applied to for-rc
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-27 14:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-24 13:19 [PATCH] RDMA/cm: Add min length checks to user structure copies Jason Gunthorpe
2020-07-24 13:45 ` Leon Romanovsky
2020-07-24 13:52 ` Jason Gunthorpe
2020-07-24 19:33 ` Leon Romanovsky
2020-07-24 19:33 ` Leon Romanovsky
2020-07-27 14:57 ` Jason Gunthorpe
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).