Message ID | 20200611100735.24184-1-tuong.t.lien@dektech.com.au |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [net] tipc: fix kernel WARNING in tipc_msg_append() | expand |
From: Tuong Lien <tuong.t.lien@dektech.com.au> Date: Thu, 11 Jun 2020 17:07:35 +0700 > syzbot found the following issue: > > WARNING: CPU: 0 PID: 6808 at include/linux/thread_info.h:150 check_copy_size include/linux/thread_info.h:150 [inline] > WARNING: CPU: 0 PID: 6808 at include/linux/thread_info.h:150 copy_from_iter include/linux/uio.h:144 [inline] > WARNING: CPU: 0 PID: 6808 at include/linux/thread_info.h:150 tipc_msg_append+0x49a/0x5e0 net/tipc/msg.c:242 > Kernel panic - not syncing: panic_on_warn set ... > > This happens after commit 5e9eeccc58f3 ("tipc: fix NULL pointer > dereference in streaming") that tried to build at least one buffer even > when the message data length is zero... However, it now exposes another > bug that the 'mss' can be zero and the 'cpy' will be negative, thus the > above kernel WARNING will appear! > The zero value of 'mss' is never expected because it means Nagle is not > enabled for the socket (actually the socket type was 'SOCK_SEQPACKET'), > so the function 'tipc_msg_append()' must not be called at all. But that > was in this particular case since the message data length was zero, and > the 'send <= maxnagle' check became true. > > We resolve the issue by explicitly checking if Nagle is enabled for the > socket, i.e. 'maxnagle != 0' before calling the 'tipc_msg_append()'. We > also reinforce the function to against such a negative values if any. > > Reported-by: syzbot+75139a7d2605236b0b7f@syzkaller.appspotmail.com > Fixes: c0bceb97db9e ("tipc: add smart nagle feature") > Acked-by: Jon Maloy <jmaloy@redhat.com> > Signed-off-by: Tuong Lien <tuong.t.lien@dektech.com.au> Applied and queued up for v5.5+ -stable.
diff --git a/net/tipc/msg.c b/net/tipc/msg.c index 046e4cb3acea..01b64869a173 100644 --- a/net/tipc/msg.c +++ b/net/tipc/msg.c @@ -238,14 +238,14 @@ int tipc_msg_append(struct tipc_msg *_hdr, struct msghdr *m, int dlen, hdr = buf_msg(skb); curr = msg_blocks(hdr); mlen = msg_size(hdr); - cpy = min_t(int, rem, mss - mlen); + cpy = min_t(size_t, rem, mss - mlen); if (cpy != copy_from_iter(skb->data + mlen, cpy, &m->msg_iter)) return -EFAULT; msg_set_size(hdr, mlen + cpy); skb_put(skb, cpy); rem -= cpy; total += msg_blocks(hdr) - curr; - } while (rem); + } while (rem > 0); return total - accounted; } diff --git a/net/tipc/socket.c b/net/tipc/socket.c index 26123f4177fd..a94f38333698 100644 --- a/net/tipc/socket.c +++ b/net/tipc/socket.c @@ -1574,7 +1574,8 @@ static int __tipc_sendstream(struct socket *sock, struct msghdr *m, size_t dlen) break; send = min_t(size_t, dlen - sent, TIPC_MAX_USER_MSG_SIZE); blocks = tsk->snd_backlog; - if (tsk->oneway++ >= tsk->nagle_start && send <= maxnagle) { + if (tsk->oneway++ >= tsk->nagle_start && maxnagle && + send <= maxnagle) { rc = tipc_msg_append(hdr, m, send, maxnagle, txq); if (unlikely(rc < 0)) break;