lkml.org 
[lkml]   [2020]   [Apr]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.19 03/54] sctp: fix refcount bug in sctp_wfree
    Date
    From: Qiujun Huang <hqjagain@gmail.com>

    [ Upstream commit 5c3e82fe159622e46e91458c1a6509c321a62820 ]

    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>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    net/sctp/socket.c | 31 +++++++++++++++++++++++--------
    1 file changed, 23 insertions(+), 8 deletions(-)

    --- a/net/sctp/socket.c
    +++ b/net/sctp/socket.c
    @@ -165,29 +165,44 @@ static void sctp_clear_owner_w(struct sc
    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,
    @@ -8899,9 +8914,9 @@ static void sctp_sock_migrate(struct soc
    * 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.

    \
     
     \ /
      Last update: 2020-04-11 14:15    [W:4.451 / U:0.152 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site