| Message ID | 20180328172304.7123-1-roland@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Mar 28, 2018 at 10:23:04AM -0700, Roland Dreier wrote: > From: Roland Dreier <roland@purestorage.com> > > If userspace passes a sockaddr with sa_family == AF_IB to the ucma join IP > multicast command, the kernel will memcpy() past the end of the cmd.addr buffer, > because sockaddr_ib is bigger than sockaddr_in6. > > Fix this by returning EINVAL if the addr_size we get back is bigger than the > address buffers we're copying between. > > Reported-and-tested-by: syzbot+6800425d54ed3ed8135d@syzkaller.appspotmail.com > Signed-off-by: Roland Dreier <roland@purestorage.com> > #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git master > > drivers/infiniband/core/ucma.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c > index e5a1e7d81326..7684fd54318a 100644 > +++ b/drivers/infiniband/core/ucma.c > @@ -1427,7 +1427,8 @@ static ssize_t ucma_join_ip_multicast(struct ucma_file *file, > join_cmd.uid = cmd.uid; > join_cmd.id = cmd.id; > join_cmd.addr_size = rdma_addr_size((struct sockaddr *) &cmd.addr); Ugh, this is a systemic mistake. Looks like every place in the uapi that uses struct sockaddr_in6 has this bug. Eg look at ucma_bind_ip I think I'd prefer to fix this via revising the API to size_t rdma_addr_size_in6(struct sockaddr_in6 *addr) size_t rdma_addr_size_kss(struct struct __kernel_sockaddr_storage *addr) And getting rid of all the dangerous (struct sockaddr *) casting at all the various call sites. The function will return 0 if the given storage is too small for the requested AF, or the AF is not supported. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, syzbot has tested the proposed patch and the reproducer did not trigger crash: Reported-and-tested-by: syzbot+6800425d54ed3ed8135d@syzkaller.appspotmail.com Note: the tag will also help syzbot to understand when the bug is fixed. Tested on git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git/master commit 3eb2ce825ea1ad89d20f7a3b5780df850e4be274 (Sun Mar 25 22:44:30 2018 +0000) Linux 4.16-rc7 compiler: gcc (GCC) 7.1.1 20170620 Patch: https://syzkaller.appspot.com/x/patch.diff?id=6405865665986560 Kernel config: https://syzkaller.appspot.com/x/.config?id=-2340295454854568752 --- There is no WARRANTY for the result, to the extent permitted by applicable law. Except when otherwise stated in writing syzbot provides the result "AS IS" without warranty of any kind, either expressed or implied, but not limited to, the implied warranties of merchantability and fittness for a particular purpose. The entire risk as to the quality of the result is with you. Should the result prove defective, you assume the cost of all necessary servicing, repair or correction. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Ugh, this is a systemic mistake. Looks like every place in the uapi that uses > struct sockaddr_in6 has this bug. Eg look at ucma_bind_ip > > I think I'd prefer to fix this via revising the API to > > size_t rdma_addr_size_in6(struct sockaddr_in6 *addr) > size_t rdma_addr_size_kss(struct struct __kernel_sockaddr_storage *addr) > > And getting rid of all the dangerous (struct sockaddr *) casting at > all the various call sites. > > The function will return 0 if the given storage is too small for the > requested AF, or the AF is not supported. Good idea. I'll code that up. - R. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c index e5a1e7d81326..7684fd54318a 100644 --- a/drivers/infiniband/core/ucma.c +++ b/drivers/infiniband/core/ucma.c @@ -1427,7 +1427,8 @@ static ssize_t ucma_join_ip_multicast(struct ucma_file *file, join_cmd.uid = cmd.uid; join_cmd.id = cmd.id; join_cmd.addr_size = rdma_addr_size((struct sockaddr *) &cmd.addr); - if (!join_cmd.addr_size) + if (!join_cmd.addr_size || + join_cmd.addr_size > min(sizeof(cmd.addr), sizeof(join_cmd.addr))) return -EINVAL; join_cmd.join_flags = RDMA_MC_JOIN_FLAG_FULLMEMBER;