aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorXin Long <lucien.xin@gmail.com>2017-10-27 14:13:29 -0400
committerDavid S. Miller <davem@davemloft.net>2017-10-28 23:06:57 -0400
commitd04adf1b355181e737b6b1e23d801b07f0b7c4c0 (patch)
tree7f80cd86a4d400fafa2c9cdd3cce5d3b29f8fe4c
parent151516fab4e82ab5da35fd14c972efc90a1c4aa4 (diff)
sctp: reset owner sk for data chunks on out queues when migrating a sock
Now when migrating sock to another one in sctp_sock_migrate(), it only resets owner sk for the data in receive queues, not the chunks on out queues. It would cause that data chunks length on the sock is not consistent with sk sk_wmem_alloc. When closing the sock or freeing these chunks, the old sk would never be freed, and the new sock may crash due to the overflow sk_wmem_alloc. syzbot found this issue with this series: r0 = socket$inet_sctp() sendto$inet(r0) listen(r0) accept4(r0) close(r0) Although listen() should have returned error when one TCP-style socket is in connecting (I may fix this one in another patch), it could also be reproduced by peeling off an assoc. This issue is there since very beginning. This patch is to reset owner sk for the chunks on out queues so that sk sk_wmem_alloc has correct value after accept one sock or peeloff an assoc to one sock. Note that when resetting owner sk for chunks on outqueue, it has to sctp_clear_owner_w/skb_orphan chunks before changing assoc->base.sk first and then sctp_set_owner_w them after changing assoc->base.sk, due to that sctp_wfree and it's callees are using assoc->base.sk. Reported-by: Dmitry Vyukov <dvyukov@google.com> Signed-off-by: Xin Long <lucien.xin@gmail.com> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r--net/sctp/socket.c32
1 files changed, 32 insertions, 0 deletions
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 17841ab30798..6f45d1713452 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -170,6 +170,36 @@ static inline void sctp_set_owner_w(struct sctp_chunk *chunk)
170 sk_mem_charge(sk, chunk->skb->truesize); 170 sk_mem_charge(sk, chunk->skb->truesize);
171} 171}
172 172
173static void sctp_clear_owner_w(struct sctp_chunk *chunk)
174{
175 skb_orphan(chunk->skb);
176}
177
178static void sctp_for_each_tx_datachunk(struct sctp_association *asoc,
179 void (*cb)(struct sctp_chunk *))
180
181{
182 struct sctp_outq *q = &asoc->outqueue;
183 struct sctp_transport *t;
184 struct sctp_chunk *chunk;
185
186 list_for_each_entry(t, &asoc->peer.transport_addr_list, transports)
187 list_for_each_entry(chunk, &t->transmitted, transmitted_list)
188 cb(chunk);
189
190 list_for_each_entry(chunk, &q->retransmit, list)
191 cb(chunk);
192
193 list_for_each_entry(chunk, &q->sacked, list)
194 cb(chunk);
195
196 list_for_each_entry(chunk, &q->abandoned, list)
197 cb(chunk);
198
199 list_for_each_entry(chunk, &q->out_chunk_list, list)
200 cb(chunk);
201}
202
173/* Verify that this is a valid address. */ 203/* Verify that this is a valid address. */
174static inline int sctp_verify_addr(struct sock *sk, union sctp_addr *addr, 204static inline int sctp_verify_addr(struct sock *sk, union sctp_addr *addr,
175 int len) 205 int len)
@@ -8212,7 +8242,9 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
8212 * paths won't try to lock it and then oldsk. 8242 * paths won't try to lock it and then oldsk.
8213 */ 8243 */
8214 lock_sock_nested(newsk, SINGLE_DEPTH_NESTING); 8244 lock_sock_nested(newsk, SINGLE_DEPTH_NESTING);
8245 sctp_for_each_tx_datachunk(assoc, sctp_clear_owner_w);
8215 sctp_assoc_migrate(assoc, newsk); 8246 sctp_assoc_migrate(assoc, newsk);
8247 sctp_for_each_tx_datachunk(assoc, sctp_set_owner_w);
8216 8248
8217 /* If the association on the newsk is already closed before accept() 8249 /* If the association on the newsk is already closed before accept()
8218 * is called, set RCV_SHUTDOWN flag. 8250 * is called, set RCV_SHUTDOWN flag.