On Fri, Mar 27, 2020 at 10:35 AM Marcelo Ricardo Leitner
[off-list ref] wrote:

On Fri, Mar 27, 2020 at 09:28:32AM +0800, Qiujun Huang wrote:

quoted

We should iterate over the datamsgs to modify

Just two small things now.
s/modify/move/  , it's more accurate.

Get it.

But mainly because...

...

quoted

Reported-and-tested-by:syzbot+cea71eec5d6de256d54d@syzkaller.appspotmail.com

checkpatch.pl is warning that there should be an empty space after the
':' here.

forgot that.

Otherwise, looks very good.

Btw, I learned a lot about syzbot new features with your tests, thanks :-)

So do I, thanks.

quoted

Signed-off-by: Qiujun Huang <redacted>
---
 net/sctp/socket.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 1b56fc440606..f68076713162 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c

@@ -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.
--

2.17.1