In Commit 94153e36e709e ("tipc: use existing sk_write_queue for
outgoing packet chain"), we assume that we fill & empty the socket's
sk_write_queue within the same lock_sock() session.
This is not true if the link is congested. During congestion, the
socket lock is released while we wait for the congestion to cease.
This implementation causes a nullptr exception, if the user space
program has several threads accessing the same socket descriptor.
Consider two threads of the same program performing the following:
Thread1 Thread2
-------------------- ----------------------
Enter tipc_sendmsg() Enter tipc_sendmsg()
lock_sock() lock_sock()
Enter tipc_link_xmit(), ret=ELINKCONG spin on socket lock..
sk_wait_event() :
release_sock() grab socket lock
: Enter tipc_link_xmit(), ret=0
: release_sock()
Wakeup after congestion
lock_sock()
skb = skb_peek(pktchain);
!! TIPC_SKB_CB(skb)->wakeup_pending = tsk->link_cong;
In this case, the second thread transmits the buffers belonging to both
thread1 and thread2 successfully. When the first thread wakeup after the
congestion it assumes that the pktchain is intact and operates on the skb's
in it, which leads to nullptr exception as follows:
[2102.439969] BUG: unable to handle kernel NULL pointer dereference at
00000000000000d0
[2102.440074] IP: [<ffffffffa005f330>] __tipc_link_xmit+0x2b0/0x4d0 [tipc]
[2102.440074] PGD 3fa3f067 PUD 3fa6b067 PMD 0
[2102.440074] Oops: 0000 [#1] SMP
[2102.440074] CPU: 2 PID: 244 Comm: sender Not tainted 3.12.28 #1
[2102.440074] RIP: 0010:[<ffffffffa005f330>] [<ffffffffa005f330>]
__tipc_link_xmit+0x2b0/0x4d0 [tipc]
[...]
[2102.440074] Call Trace:
[2102.440074] [<ffffffff8163f0b9>] ? schedule+0x29/0x70
[2102.440074] [<ffffffffa006a756>] ? tipc_node_unlock+0x46/0x170 [tipc]
[2102.440074] [<ffffffffa005f761>] tipc_link_xmit+0x51/0xf0 [tipc]
[2102.440074] [<ffffffffa006d8ae>] tipc_send_stream+0x11e/0x4f0 [tipc]
[2102.440074] [<ffffffff8106b150>] ? __wake_up_sync+0x20/0x20
[2102.440074] [<ffffffffa006dc9c>] tipc_send_packet+0x1c/0x20 [tipc]
[2102.440074] [<ffffffff81502478>] sock_sendmsg+0xa8/0xd0
[2102.440074] [<ffffffff81507895>] ? release_sock+0x145/0x170
[2102.440074] [<ffffffff815030d8>] ___sys_sendmsg+0x3d8/0x3e0
[2102.440074] [<ffffffff816426ae>] ? _raw_spin_unlock+0xe/0x10
[2102.440074] [<ffffffff81115c2a>] ? handle_mm_fault+0x6ca/0x9d0
[2102.440074] [<ffffffff8107dd65>] ? set_next_entity+0x85/0xa0
[2102.440074] [<ffffffff816426de>] ? _raw_spin_unlock_irq+0xe/0x20
[2102.440074] [<ffffffff8107463c>] ? finish_task_switch+0x5c/0xc0
[2102.440074] [<ffffffff8163ea8c>] ? __schedule+0x34c/0x950
[2102.440074] [<ffffffff81504e12>] __sys_sendmsg+0x42/0x80
[2102.440074] [<ffffffff81504e62>] SyS_sendmsg+0x12/0x20
[2102.440074] [<ffffffff8164aed2>] system_call_fastpath+0x16/0x1b
In this commit, we maintain the buffer list in the stack during congestion
and use sk_write_queue otherwise.
Fixes:
94153e36e709e ("tipc: use existing sk_write_queue for outgoing packet chain")
Signed-off-by: Parthasarathy Bhuvaragan <[email protected]>
---
net/tipc/socket.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 69c29050f14a..bb5875c18c98 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -674,6 +674,7 @@ static int tipc_sendmcast(struct socket *sock, struct
tipc_name_seq *seq,
struct net *net = sock_net(sk);
struct tipc_msg *mhdr = &tsk->phdr;
struct sk_buff_head *pktchain = &sk->sk_write_queue;
+ struct sk_buff_head tmpq;
struct iov_iter save = msg->msg_iter;
uint mtu;
int rc;
@@ -687,6 +688,11 @@ static int tipc_sendmcast(struct socket *sock, struct
tipc_name_seq *seq,
msg_set_nameupper(mhdr, seq->upper);
msg_set_hdr_sz(mhdr, MCAST_H_SIZE);
+ if (likely(!skb_queue_empty(pktchain))) {
+ skb_queue_head_init(&tmpq);
+ pktchain = &tmpq;
+ }
+
new_mtu:
mtu = tipc_bcast_get_mtu(net);
rc = tipc_msg_build(mhdr, msg, 0, dsz, mtu, pktchain);
@@ -864,6 +870,7 @@ static int __tipc_sendmsg(struct socket *sock, struct
msghdr *m, size_t dsz)
struct tipc_msg *mhdr = &tsk->phdr;
u32 dnode, dport;
struct sk_buff_head *pktchain = &sk->sk_write_queue;
+ struct sk_buff_head tmpq;
struct sk_buff *skb;
struct tipc_name_seq *seq;
struct iov_iter save;
@@ -924,6 +931,11 @@ static int __tipc_sendmsg(struct socket *sock, struct
msghdr *m, size_t dsz)
msg_set_hdr_sz(mhdr, BASIC_H_SIZE);
}
+ if (likely(!skb_queue_empty(pktchain))) {
+ skb_queue_head_init(&tmpq);
+ pktchain = &tmpq;
+ }
+
save = m->msg_iter;
new_mtu:
mtu = tipc_node_get_mtu(net, dnode, tsk->portid);
@@ -1017,6 +1029,7 @@ static int __tipc_send_stream(struct socket *sock, struct
msghdr *m, size_t dsz)
struct tipc_sock *tsk = tipc_sk(sk);
struct tipc_msg *mhdr = &tsk->phdr;
struct sk_buff_head *pktchain = &sk->sk_write_queue;
+ struct sk_buff_head tmpq;
DECLARE_SOCKADDR(struct sockaddr_tipc *, dest, m->msg_name);
u32 portid = tsk->portid;
int rc = -EINVAL;
@@ -1045,6 +1058,11 @@ static int __tipc_send_stream(struct socket *sock,
struct msghdr *m, size_t dsz)
timeo = sock_sndtimeo(sk, m->msg_flags & MSG_DONTWAIT);
dnode = tsk_peer_node(tsk);
+ if (likely(!skb_queue_empty(pktchain))) {
+ skb_queue_head_init(&tmpq);
+ pktchain = &tmpq;
+ }
+
next:
save = m->msg_iter;
mtu = tsk->max_pkt;
@@ -1052,6 +1070,7 @@ next:
rc = tipc_msg_build(mhdr, m, sent, send, mtu, pktchain);
if (unlikely(rc < 0))
return rc;
+
do {
if (likely(!tsk_conn_cong(tsk))) {
rc = tipc_node_xmit(net, pktchain, dnode, portid);
--
2.1.4
------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
_______________________________________________
tipc-discussion mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
