All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Qiujun Huang <hqjagain@gmail.com>,
	Marcelo Ricardo Leitner <mleitner@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	syzbot+cea71eec5d6de256d54d@syzkaller.appspotmail.com
Subject: [PATCH 5.6 05/29] sctp: fix refcount bug in sctp_wfree
Date: Tue,  7 Apr 2020 12:22:02 +0200	[thread overview]
Message-ID: <20200407101452.619901149@linuxfoundation.org> (raw)
In-Reply-To: <20200407101452.046058399@linuxfoundation.org>

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
@@ -147,29 +147,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,
@@ -9574,9 +9589,9 @@ static int sctp_sock_migrate(struct sock
 	 * 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.



  parent reply	other threads:[~2020-04-07 10:26 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-07 10:21 [PATCH 5.6 00/29] 5.6.3-rc1 review Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.6 01/29] ipv4: fix a RCU-list lock in fib_triestat_seq_show Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.6 02/29] net: dsa: ksz: Select KSZ protocol tag Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 03/29] net, ip_tunnel: fix interface lookup with no key Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 04/29] sctp: fix possibly using a bad saddr with a given dst Greg Kroah-Hartman
2020-04-07 10:22 ` Greg Kroah-Hartman [this message]
2020-04-07 10:22 ` [PATCH 5.6 06/29] net: macb: Fix handling of fixed-link node Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 07/29] net: fix fraglist segmentation reference count leak Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 08/29] udp: initialize is_flist with 0 in udp_gro_receive Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 09/29] padata: fix uninitialized return value in padata_replace() Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 10/29] brcmfmac: abort and release host after error Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 11/29] bpf: Fix tnum constraints for 32-bit comparisons Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 12/29] XArray: Fix xa_find_next for large multi-index entries Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 13/29] drm/bridge: analogix-anx6345: Avoid duplicate -supply suffix Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 14/29] drm/i915/display: Fix mode private_flags comparison at atomic_check Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 15/29] misc: rtsx: set correct pcr_ops for rts522A Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 16/29] misc: pci_endpoint_test: Fix to support > 10 pci-endpoint-test devices Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 17/29] misc: pci_endpoint_test: Avoid using module parameter to determine irqtype Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 18/29] PCI: sysfs: Revert "rescan" file renames Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 19/29] coresight: do not use the BIT() macro in the UAPI header Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 20/29] mei: me: add cedar fork device ids Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 21/29] nvmem: release the write-protect pin Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 22/29] nvmem: check for NULL reg_read and reg_write before dereferencing Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 23/29] nvmem: sprd: Fix the block lock operation Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 24/29] extcon: axp288: Add wakeup support Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 25/29] power: supply: axp288_charger: Add special handling for HP Pavilion x2 10 Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 26/29] Revert "ALSA: uapi: Drop asound.h inclusion from asoc.h" Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 27/29] Revert "dm: always call blk_queue_split() in dm_process_bio()" Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 28/29] ALSA: hda/ca0132 - Add Recon3Di quirk to handle integrated sound on EVGA X99 Classified motherboard Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.6 29/29] soc: mediatek: knows_txdone needs to be set in Mediatek CMDQ helper Greg Kroah-Hartman
2020-04-07 12:37 ` [PATCH 5.6 00/29] 5.6.3-rc1 review Jon Hunter
2020-04-07 12:37   ` Jon Hunter
     [not found]   ` <dd65ddb4-478c-c022-542c-5e0b44ab8962-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2020-04-07 14:49     ` Greg Kroah-Hartman
2020-04-07 14:49       ` Greg Kroah-Hartman
2020-04-07 13:51 ` Daniel Díaz
2020-04-07 14:43   ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200407101452.619901149@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=hqjagain@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mleitner@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=syzbot+cea71eec5d6de256d54d@syzkaller.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.