Commit ea010070 authored by shamir rabinovitch's avatar shamir rabinovitch Committed by David S. Miller

net/rds: fix warn in rds_message_alloc_sgs

redundant copy_from_user in rds_sendmsg system call expose rds
to issue where rds_rdma_extra_size walk the rds iovec and and
calculate the number pf pages (sgs) it need to add to the tail of
rds message and later rds_cmsg_rdma_args copy the rds iovec again
and re calculate the same number and get different result causing
WARN_ON in rds_message_alloc_sgs.

fix this by doing the copy_from_user only once per rds_sendmsg
system call.

When issue occur the below dump is seen:

WARNING: CPU: 0 PID: 19789 at net/rds/message.c:316 rds_message_alloc_sgs+0x10c/0x160 net/rds/message.c:316
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 19789 Comm: syz-executor827 Not tainted 4.19.0-next-20181030+ #101
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+0x244/0x39d lib/dump_stack.c:113
 panic+0x2ad/0x55c kernel/panic.c:188
 __warn.cold.8+0x20/0x45 kernel/panic.c:540
 report_bug+0x254/0x2d0 lib/bug.c:186
 fixup_bug arch/x86/kernel/traps.c:178 [inline]
 do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
 do_invalid_op+0x36/0x40 arch/x86/kernel/traps.c:290
 invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:969
RIP: 0010:rds_message_alloc_sgs+0x10c/0x160 net/rds/message.c:316
Code: c0 74 04 3c 03 7e 6c 44 01 ab 78 01 00 00 e8 2b 9e 35 fa 4c 89 e0 48 83 c4 08 5b 41 5c 41 5d 41 5e 41 5f 5d c3 e8 14 9e 35 fa <0f> 0b 31 ff 44 89 ee e8 18 9f 35 fa 45 85 ed 75 1b e8 fe 9d 35 fa
RSP: 0018:ffff8801c51b7460 EFLAGS: 00010293
RAX: ffff8801bc412080 RBX: ffff8801d7bf4040 RCX: ffffffff8749c9e6
RDX: 0000000000000000 RSI: ffffffff8749ca5c RDI: 0000000000000004
RBP: ffff8801c51b7490 R08: ffff8801bc412080 R09: ffffed003b5c5b67
R10: ffffed003b5c5b67 R11: ffff8801dae2db3b R12: 0000000000000000
R13: 000000000007165c R14: 000000000007165c R15: 0000000000000005
 rds_cmsg_rdma_args+0x82d/0x1510 net/rds/rdma.c:623
 rds_cmsg_send net/rds/send.c:971 [inline]
 rds_sendmsg+0x19a2/0x3180 net/rds/send.c:1273
 sock_sendmsg_nosec net/socket.c:622 [inline]
 sock_sendmsg+0xd5/0x120 net/socket.c:632
 ___sys_sendmsg+0x7fd/0x930 net/socket.c:2117
 __sys_sendmsg+0x11d/0x280 net/socket.c:2155
 __do_sys_sendmsg net/socket.c:2164 [inline]
 __se_sys_sendmsg net/socket.c:2162 [inline]
 __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2162
 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x44a859
Code: e8 dc e6 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 6b cb fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f1d4710ada8 EFLAGS: 00000297 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000006dcc28 RCX: 000000000044a859
RDX: 0000000000000000 RSI: 0000000020001600 RDI: 0000000000000003
RBP: 00000000006dcc20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000297 R12: 00000000006dcc2c
R13: 646e732f7665642f R14: 00007f1d4710b9c0 R15: 00000000006dcd2c
Kernel Offset: disabled
Rebooting in 86400 seconds..

Reported-by: syzbot+26de17458aeda9d305d8@syzkaller.appspotmail.com
Acked-by: default avatarSantosh Shilimkar <santosh.shilimkar@oracle.com>
Signed-off-by: default avatarshamir rabinovitch <shamir.rabinovitch@oracle.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent c6f4075e
...@@ -517,9 +517,10 @@ static int rds_rdma_pages(struct rds_iovec iov[], int nr_iovecs) ...@@ -517,9 +517,10 @@ static int rds_rdma_pages(struct rds_iovec iov[], int nr_iovecs)
return tot_pages; return tot_pages;
} }
int rds_rdma_extra_size(struct rds_rdma_args *args) int rds_rdma_extra_size(struct rds_rdma_args *args,
struct rds_iov_vector *iov)
{ {
struct rds_iovec vec; struct rds_iovec *vec;
struct rds_iovec __user *local_vec; struct rds_iovec __user *local_vec;
int tot_pages = 0; int tot_pages = 0;
unsigned int nr_pages; unsigned int nr_pages;
...@@ -530,13 +531,23 @@ int rds_rdma_extra_size(struct rds_rdma_args *args) ...@@ -530,13 +531,23 @@ int rds_rdma_extra_size(struct rds_rdma_args *args)
if (args->nr_local == 0) if (args->nr_local == 0)
return -EINVAL; return -EINVAL;
/* figure out the number of pages in the vector */ iov->iov = kcalloc(args->nr_local,
for (i = 0; i < args->nr_local; i++) { sizeof(struct rds_iovec),
if (copy_from_user(&vec, &local_vec[i], GFP_KERNEL);
if (!iov->iov)
return -ENOMEM;
vec = &iov->iov[0];
if (copy_from_user(vec, local_vec, args->nr_local *
sizeof(struct rds_iovec))) sizeof(struct rds_iovec)))
return -EFAULT; return -EFAULT;
iov->len = args->nr_local;
/* figure out the number of pages in the vector */
for (i = 0; i < args->nr_local; i++, vec++) {
nr_pages = rds_pages_in_vec(&vec); nr_pages = rds_pages_in_vec(vec);
if (nr_pages == 0) if (nr_pages == 0)
return -EINVAL; return -EINVAL;
...@@ -558,15 +569,15 @@ int rds_rdma_extra_size(struct rds_rdma_args *args) ...@@ -558,15 +569,15 @@ int rds_rdma_extra_size(struct rds_rdma_args *args)
* Extract all arguments and set up the rdma_op * Extract all arguments and set up the rdma_op
*/ */
int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm, int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm,
struct cmsghdr *cmsg) struct cmsghdr *cmsg,
struct rds_iov_vector *vec)
{ {
struct rds_rdma_args *args; struct rds_rdma_args *args;
struct rm_rdma_op *op = &rm->rdma; struct rm_rdma_op *op = &rm->rdma;
int nr_pages; int nr_pages;
unsigned int nr_bytes; unsigned int nr_bytes;
struct page **pages = NULL; struct page **pages = NULL;
struct rds_iovec iovstack[UIO_FASTIOV], *iovs = iovstack; struct rds_iovec *iovs;
int iov_size;
unsigned int i, j; unsigned int i, j;
int ret = 0; int ret = 0;
...@@ -586,31 +597,23 @@ int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm, ...@@ -586,31 +597,23 @@ int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm,
goto out_ret; goto out_ret;
} }
/* Check whether to allocate the iovec area */ if (vec->len != args->nr_local) {
iov_size = args->nr_local * sizeof(struct rds_iovec); ret = -EINVAL;
if (args->nr_local > UIO_FASTIOV) {
iovs = sock_kmalloc(rds_rs_to_sk(rs), iov_size, GFP_KERNEL);
if (!iovs) {
ret = -ENOMEM;
goto out_ret; goto out_ret;
} }
}
if (copy_from_user(iovs, (struct rds_iovec __user *)(unsigned long) args->local_vec_addr, iov_size)) { iovs = vec->iov;
ret = -EFAULT;
goto out;
}
nr_pages = rds_rdma_pages(iovs, args->nr_local); nr_pages = rds_rdma_pages(iovs, args->nr_local);
if (nr_pages < 0) { if (nr_pages < 0) {
ret = -EINVAL; ret = -EINVAL;
goto out; goto out_ret;
} }
pages = kcalloc(nr_pages, sizeof(struct page *), GFP_KERNEL); pages = kcalloc(nr_pages, sizeof(struct page *), GFP_KERNEL);
if (!pages) { if (!pages) {
ret = -ENOMEM; ret = -ENOMEM;
goto out; goto out_ret;
} }
op->op_write = !!(args->flags & RDS_RDMA_READWRITE); op->op_write = !!(args->flags & RDS_RDMA_READWRITE);
...@@ -623,7 +626,7 @@ int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm, ...@@ -623,7 +626,7 @@ int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm,
op->op_sg = rds_message_alloc_sgs(rm, nr_pages); op->op_sg = rds_message_alloc_sgs(rm, nr_pages);
if (!op->op_sg) { if (!op->op_sg) {
ret = -ENOMEM; ret = -ENOMEM;
goto out; goto out_pages;
} }
if (op->op_notify || op->op_recverr) { if (op->op_notify || op->op_recverr) {
...@@ -635,7 +638,7 @@ int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm, ...@@ -635,7 +638,7 @@ int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm,
op->op_notifier = kmalloc(sizeof(struct rds_notifier), GFP_KERNEL); op->op_notifier = kmalloc(sizeof(struct rds_notifier), GFP_KERNEL);
if (!op->op_notifier) { if (!op->op_notifier) {
ret = -ENOMEM; ret = -ENOMEM;
goto out; goto out_pages;
} }
op->op_notifier->n_user_token = args->user_token; op->op_notifier->n_user_token = args->user_token;
op->op_notifier->n_status = RDS_RDMA_SUCCESS; op->op_notifier->n_status = RDS_RDMA_SUCCESS;
...@@ -681,7 +684,7 @@ int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm, ...@@ -681,7 +684,7 @@ int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm,
*/ */
ret = rds_pin_pages(iov->addr, nr, pages, !op->op_write); ret = rds_pin_pages(iov->addr, nr, pages, !op->op_write);
if (ret < 0) if (ret < 0)
goto out; goto out_pages;
else else
ret = 0; ret = 0;
...@@ -714,13 +717,11 @@ int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm, ...@@ -714,13 +717,11 @@ int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm,
nr_bytes, nr_bytes,
(unsigned int) args->remote_vec.bytes); (unsigned int) args->remote_vec.bytes);
ret = -EINVAL; ret = -EINVAL;
goto out; goto out_pages;
} }
op->op_bytes = nr_bytes; op->op_bytes = nr_bytes;
out: out_pages:
if (iovs != iovstack)
sock_kfree_s(rds_rs_to_sk(rs), iovs, iov_size);
kfree(pages); kfree(pages);
out_ret: out_ret:
if (ret) if (ret)
......
...@@ -386,6 +386,18 @@ static inline void rds_message_zcopy_queue_init(struct rds_msg_zcopy_queue *q) ...@@ -386,6 +386,18 @@ static inline void rds_message_zcopy_queue_init(struct rds_msg_zcopy_queue *q)
INIT_LIST_HEAD(&q->zcookie_head); INIT_LIST_HEAD(&q->zcookie_head);
} }
struct rds_iov_vector {
struct rds_iovec *iov;
int len;
};
struct rds_iov_vector_arr {
struct rds_iov_vector *vec;
int len;
int indx;
int incr;
};
struct rds_message { struct rds_message {
refcount_t m_refcount; refcount_t m_refcount;
struct list_head m_sock_item; struct list_head m_sock_item;
...@@ -904,13 +916,13 @@ int rds_get_mr(struct rds_sock *rs, char __user *optval, int optlen); ...@@ -904,13 +916,13 @@ int rds_get_mr(struct rds_sock *rs, char __user *optval, int optlen);
int rds_get_mr_for_dest(struct rds_sock *rs, char __user *optval, int optlen); int rds_get_mr_for_dest(struct rds_sock *rs, char __user *optval, int optlen);
int rds_free_mr(struct rds_sock *rs, char __user *optval, int optlen); int rds_free_mr(struct rds_sock *rs, char __user *optval, int optlen);
void rds_rdma_drop_keys(struct rds_sock *rs); void rds_rdma_drop_keys(struct rds_sock *rs);
int rds_rdma_extra_size(struct rds_rdma_args *args); int rds_rdma_extra_size(struct rds_rdma_args *args,
int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm, struct rds_iov_vector *iov);
struct cmsghdr *cmsg);
int rds_cmsg_rdma_dest(struct rds_sock *rs, struct rds_message *rm, int rds_cmsg_rdma_dest(struct rds_sock *rs, struct rds_message *rm,
struct cmsghdr *cmsg); struct cmsghdr *cmsg);
int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm, int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm,
struct cmsghdr *cmsg); struct cmsghdr *cmsg,
struct rds_iov_vector *vec);
int rds_cmsg_rdma_map(struct rds_sock *rs, struct rds_message *rm, int rds_cmsg_rdma_map(struct rds_sock *rs, struct rds_message *rm,
struct cmsghdr *cmsg); struct cmsghdr *cmsg);
void rds_rdma_free_op(struct rm_rdma_op *ro); void rds_rdma_free_op(struct rm_rdma_op *ro);
......
...@@ -876,13 +876,15 @@ static int rds_send_queue_rm(struct rds_sock *rs, struct rds_connection *conn, ...@@ -876,13 +876,15 @@ static int rds_send_queue_rm(struct rds_sock *rs, struct rds_connection *conn,
* rds_message is getting to be quite complicated, and we'd like to allocate * rds_message is getting to be quite complicated, and we'd like to allocate
* it all in one go. This figures out how big it needs to be up front. * it all in one go. This figures out how big it needs to be up front.
*/ */
static int rds_rm_size(struct msghdr *msg, int num_sgs) static int rds_rm_size(struct msghdr *msg, int num_sgs,
struct rds_iov_vector_arr *vct)
{ {
struct cmsghdr *cmsg; struct cmsghdr *cmsg;
int size = 0; int size = 0;
int cmsg_groups = 0; int cmsg_groups = 0;
int retval; int retval;
bool zcopy_cookie = false; bool zcopy_cookie = false;
struct rds_iov_vector *iov, *tmp_iov;
for_each_cmsghdr(cmsg, msg) { for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg)) if (!CMSG_OK(msg, cmsg))
...@@ -893,8 +895,24 @@ static int rds_rm_size(struct msghdr *msg, int num_sgs) ...@@ -893,8 +895,24 @@ static int rds_rm_size(struct msghdr *msg, int num_sgs)
switch (cmsg->cmsg_type) { switch (cmsg->cmsg_type) {
case RDS_CMSG_RDMA_ARGS: case RDS_CMSG_RDMA_ARGS:
if (vct->indx >= vct->len) {
vct->len += vct->incr;
tmp_iov =
krealloc(vct->vec,
vct->len *
sizeof(struct rds_iov_vector),
GFP_KERNEL);
if (!tmp_iov) {
vct->len -= vct->incr;
return -ENOMEM;
}
vct->vec = tmp_iov;
}
iov = &vct->vec[vct->indx];
memset(iov, 0, sizeof(struct rds_iov_vector));
vct->indx++;
cmsg_groups |= 1; cmsg_groups |= 1;
retval = rds_rdma_extra_size(CMSG_DATA(cmsg)); retval = rds_rdma_extra_size(CMSG_DATA(cmsg), iov);
if (retval < 0) if (retval < 0)
return retval; return retval;
size += retval; size += retval;
...@@ -951,10 +969,11 @@ static int rds_cmsg_zcopy(struct rds_sock *rs, struct rds_message *rm, ...@@ -951,10 +969,11 @@ static int rds_cmsg_zcopy(struct rds_sock *rs, struct rds_message *rm,
} }
static int rds_cmsg_send(struct rds_sock *rs, struct rds_message *rm, static int rds_cmsg_send(struct rds_sock *rs, struct rds_message *rm,
struct msghdr *msg, int *allocated_mr) struct msghdr *msg, int *allocated_mr,
struct rds_iov_vector_arr *vct)
{ {
struct cmsghdr *cmsg; struct cmsghdr *cmsg;
int ret = 0; int ret = 0, ind = 0;
for_each_cmsghdr(cmsg, msg) { for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg)) if (!CMSG_OK(msg, cmsg))
...@@ -968,7 +987,10 @@ static int rds_cmsg_send(struct rds_sock *rs, struct rds_message *rm, ...@@ -968,7 +987,10 @@ static int rds_cmsg_send(struct rds_sock *rs, struct rds_message *rm,
*/ */
switch (cmsg->cmsg_type) { switch (cmsg->cmsg_type) {
case RDS_CMSG_RDMA_ARGS: case RDS_CMSG_RDMA_ARGS:
ret = rds_cmsg_rdma_args(rs, rm, cmsg); if (ind >= vct->indx)
return -ENOMEM;
ret = rds_cmsg_rdma_args(rs, rm, cmsg, &vct->vec[ind]);
ind++;
break; break;
case RDS_CMSG_RDMA_DEST: case RDS_CMSG_RDMA_DEST:
...@@ -1084,6 +1106,11 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len) ...@@ -1084,6 +1106,11 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
sock_flag(rds_rs_to_sk(rs), SOCK_ZEROCOPY)); sock_flag(rds_rs_to_sk(rs), SOCK_ZEROCOPY));
int num_sgs = ceil(payload_len, PAGE_SIZE); int num_sgs = ceil(payload_len, PAGE_SIZE);
int namelen; int namelen;
struct rds_iov_vector_arr vct = {0};
int ind;
/* expect 1 RDMA CMSG per rds_sendmsg. can still grow if more needed. */
vct.incr = 1;
/* Mirror Linux UDP mirror of BSD error message compatibility */ /* Mirror Linux UDP mirror of BSD error message compatibility */
/* XXX: Perhaps MSG_MORE someday */ /* XXX: Perhaps MSG_MORE someday */
...@@ -1220,7 +1247,7 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len) ...@@ -1220,7 +1247,7 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
num_sgs = iov_iter_npages(&msg->msg_iter, INT_MAX); num_sgs = iov_iter_npages(&msg->msg_iter, INT_MAX);
} }
/* size of rm including all sgs */ /* size of rm including all sgs */
ret = rds_rm_size(msg, num_sgs); ret = rds_rm_size(msg, num_sgs, &vct);
if (ret < 0) if (ret < 0)
goto out; goto out;
...@@ -1270,7 +1297,7 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len) ...@@ -1270,7 +1297,7 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
rm->m_conn_path = cpath; rm->m_conn_path = cpath;
/* Parse any control messages the user may have included. */ /* Parse any control messages the user may have included. */
ret = rds_cmsg_send(rs, rm, msg, &allocated_mr); ret = rds_cmsg_send(rs, rm, msg, &allocated_mr, &vct);
if (ret) { if (ret) {
/* Trigger connection so that its ready for the next retry */ /* Trigger connection so that its ready for the next retry */
if (ret == -EAGAIN) if (ret == -EAGAIN)
...@@ -1348,9 +1375,18 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len) ...@@ -1348,9 +1375,18 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
if (ret) if (ret)
goto out; goto out;
rds_message_put(rm); rds_message_put(rm);
for (ind = 0; ind < vct.indx; ind++)
kfree(vct.vec[ind].iov);
kfree(vct.vec);
return payload_len; return payload_len;
out: out:
for (ind = 0; ind < vct.indx; ind++)
kfree(vct.vec[ind].iov);
kfree(vct.vec);
/* If the user included a RDMA_MAP cmsg, we allocated a MR on the fly. /* If the user included a RDMA_MAP cmsg, we allocated a MR on the fly.
* If the sendmsg goes through, we keep the MR. If it fails with EAGAIN * If the sendmsg goes through, we keep the MR. If it fails with EAGAIN
* or in any other way, we need to destroy the MR again */ * or in any other way, we need to destroy the MR again */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment