redonkable
/
alistair23-linux
1
0
Fork 0
Code Releases Activity

sctp: fix refcount bug in sctp_wfree 

We should iterate over the datamsgs to move
all chunks(skbs) to newsk.

The following case cause the bug:
for the trouble SKB, it was in outq->transmitted list

sctp_outq_sack
        sctp_check_transmitted
                SKB was moved to outq->sacked list
        then throw away the sack queue
                SKB was deleted from outq->sacked
(but it was held by datamsg at sctp_datamsg_to_asoc
So, sctp_wfree was not called here)

then migrate happened

        sctp_for_each_tx_datachunk(
        sctp_clear_owner_w);
        sctp_assoc_migrate();
        sctp_for_each_tx_datachunk(
        sctp_set_owner_w);
SKB was not in the outq, and was not changed to newsk

finally

__sctp_outq_teardown
        sctp_chunk_put (for another skb)
                sctp_datamsg_put
                        __kfree_skb(msg->frag_list)
                                sctp_wfree (for SKB)
	SKB->sk was still oldsk (skb->sk != asoc->base.sk).

Reported-and-tested-by: syzbot+cea71eec5d6de256d54d@syzkaller.appspotmail.com
Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
Acked-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
alistair/sensors
Qiujun Huang 2020-03-27 11:07:51 +08:00 committed by David S. Miller
parent fbe4e0c1b2
commit 5c3e82fe15
1 changed files with 23 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
net/sctp/socket.c
View File

@ -147,29 +147,44 @@ static void sctp_clear_owner_w(struct sctp_chunk *chunk)
skb_orphan(chunk->skb);
}
#define traverse_and_process() \
do { \
msg = chunk->msg; \
if (msg == prev_msg) \
continue; \
list_for_each_entry(c, &msg->chunks, frag_list) { \
if ((clear && asoc->base.sk == c->skb->sk) || \
(!clear && asoc->base.sk != c->skb->sk)) \
cb(c); \
} \
prev_msg = msg; \
} while (0)
static void sctp_for_each_tx_datachunk(struct sctp_association *asoc,
bool clear,
void (*cb)(struct sctp_chunk *))
{
struct sctp_datamsg *msg, *prev_msg = NULL;
struct sctp_outq *q = &asoc->outqueue;
struct sctp_chunk *chunk, *c;
struct sctp_transport *t;
struct sctp_chunk *chunk;
list_for_each_entry(t, &asoc->peer.transport_addr_list, transports)
list_for_each_entry(chunk, &t->transmitted, transmitted_list)
cb(chunk);
traverse_and_process();
list_for_each_entry(chunk, &q->retransmit, transmitted_list)
cb(chunk);
traverse_and_process();
list_for_each_entry(chunk, &q->sacked, transmitted_list)
cb(chunk);
traverse_and_process();
list_for_each_entry(chunk, &q->abandoned, transmitted_list)
cb(chunk);
traverse_and_process();
list_for_each_entry(chunk, &q->out_chunk_list, list)
cb(chunk);
traverse_and_process();
}
static void sctp_for_each_rx_skb(struct sctp_association *asoc, struct sock *sk,
@ -9574,9 +9589,9 @@ static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
* paths won't try to lock it and then oldsk.
*/
lock_sock_nested(newsk, SINGLE_DEPTH_NESTING);
sctp_for_each_tx_datachunk(assoc, sctp_clear_owner_w);
sctp_for_each_tx_datachunk(assoc, true, sctp_clear_owner_w);
sctp_assoc_migrate(assoc, newsk);
sctp_for_each_tx_datachunk(assoc, sctp_set_owner_w);
sctp_for_each_tx_datachunk(assoc, false, sctp_set_owner_w);
/* If the association on the newsk is already closed before accept()
* is called, set RCV_SHUTDOWN flag.