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, Ji Jianwen <jiji@redhat.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	Hannes Frederic Sowa <hannes@stressinduktion.org>,
	Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 4.0 09/55] sctp: fix ASCONF list handling
Date: Wed,  8 Jul 2015 00:34:44 -0700	[thread overview]
Message-ID: <20150708073239.371076747@linuxfoundation.org> (raw)
In-Reply-To: <20150708073238.785749886@linuxfoundation.org>

4.0-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

[ Upstream commit 2d45a02d0166caf2627fe91897c6ffc3b19514c4 ]

->auto_asconf_splist is per namespace and mangled by functions like
sctp_setsockopt_auto_asconf() which doesn't guarantee any serialization.

Also, the call to inet_sk_copy_descendant() was backuping
->auto_asconf_list through the copy but was not honoring
->do_auto_asconf, which could lead to list corruption if it was
different between both sockets.

This commit thus fixes the list handling by using ->addr_wq_lock
spinlock to protect the list. A special handling is done upon socket
creation and destruction for that. Error handlig on sctp_init_sock()
will never return an error after having initialized asconf, so
sctp_destroy_sock() can be called without addrq_wq_lock. The lock now
will be take on sctp_close_sock(), before locking the socket, so we
don't do it in inverse order compared to sctp_addr_wq_timeout_handler().

Instead of taking the lock on sctp_sock_migrate() for copying and
restoring the list values, it's preferred to avoid rewritting it by
implementing sctp_copy_descendant().

Issue was found with a test application that kept flipping sysctl
default_auto_asconf on and off, but one could trigger it by issuing
simultaneous setsockopt() calls on multiple sockets or by
creating/destroying sockets fast enough. This is only triggerable
locally.

Fixes: 9f7d653b67ae ("sctp: Add Auto-ASCONF support (core).")
Reported-by: Ji Jianwen <jiji@redhat.com>
Suggested-by: Neil Horman <nhorman@tuxdriver.com>
Suggested-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/net/netns/sctp.h   |    1 +
 include/net/sctp/structs.h |    4 ++++
 net/sctp/socket.c          |   43 ++++++++++++++++++++++++++++++++-----------
 3 files changed, 37 insertions(+), 11 deletions(-)

--- a/include/net/netns/sctp.h
+++ b/include/net/netns/sctp.h
@@ -31,6 +31,7 @@ struct netns_sctp {
 	struct list_head addr_waitq;
 	struct timer_list addr_wq_timer;
 	struct list_head auto_asconf_splist;
+	/* Lock that protects both addr_waitq and auto_asconf_splist */
 	spinlock_t addr_wq_lock;
 
 	/* Lock that protects the local_addr_list writers */
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -223,6 +223,10 @@ struct sctp_sock {
 	atomic_t pd_mode;
 	/* Receive to here while partial delivery is in effect. */
 	struct sk_buff_head pd_lobby;
+
+	/* These must be the last fields, as they will skipped on copies,
+	 * like on accept and peeloff operations
+	 */
 	struct list_head auto_asconf_list;
 	int do_auto_asconf;
 };
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1533,8 +1533,10 @@ static void sctp_close(struct sock *sk,
 
 	/* Supposedly, no process has access to the socket, but
 	 * the net layers still may.
+	 * Also, sctp_destroy_sock() needs to be called with addr_wq_lock
+	 * held and that should be grabbed before socket lock.
 	 */
-	local_bh_disable();
+	spin_lock_bh(&net->sctp.addr_wq_lock);
 	bh_lock_sock(sk);
 
 	/* Hold the sock, since sk_common_release() will put sock_put()
@@ -1544,7 +1546,7 @@ static void sctp_close(struct sock *sk,
 	sk_common_release(sk);
 
 	bh_unlock_sock(sk);
-	local_bh_enable();
+	spin_unlock_bh(&net->sctp.addr_wq_lock);
 
 	sock_put(sk);
 
@@ -3587,6 +3589,7 @@ static int sctp_setsockopt_auto_asconf(s
 	if ((val && sp->do_auto_asconf) || (!val && !sp->do_auto_asconf))
 		return 0;
 
+	spin_lock_bh(&sock_net(sk)->sctp.addr_wq_lock);
 	if (val == 0 && sp->do_auto_asconf) {
 		list_del(&sp->auto_asconf_list);
 		sp->do_auto_asconf = 0;
@@ -3595,6 +3598,7 @@ static int sctp_setsockopt_auto_asconf(s
 		    &sock_net(sk)->sctp.auto_asconf_splist);
 		sp->do_auto_asconf = 1;
 	}
+	spin_unlock_bh(&sock_net(sk)->sctp.addr_wq_lock);
 	return 0;
 }
 
@@ -4128,18 +4132,28 @@ static int sctp_init_sock(struct sock *s
 	local_bh_disable();
 	percpu_counter_inc(&sctp_sockets_allocated);
 	sock_prot_inuse_add(net, sk->sk_prot, 1);
+
+	/* Nothing can fail after this block, otherwise
+	 * sctp_destroy_sock() will be called without addr_wq_lock held
+	 */
 	if (net->sctp.default_auto_asconf) {
+		spin_lock(&sock_net(sk)->sctp.addr_wq_lock);
 		list_add_tail(&sp->auto_asconf_list,
 		    &net->sctp.auto_asconf_splist);
 		sp->do_auto_asconf = 1;
-	} else
+		spin_unlock(&sock_net(sk)->sctp.addr_wq_lock);
+	} else {
 		sp->do_auto_asconf = 0;
+	}
+
 	local_bh_enable();
 
 	return 0;
 }
 
-/* Cleanup any SCTP per socket resources.  */
+/* Cleanup any SCTP per socket resources. Must be called with
+ * sock_net(sk)->sctp.addr_wq_lock held if sp->do_auto_asconf is true
+ */
 static void sctp_destroy_sock(struct sock *sk)
 {
 	struct sctp_sock *sp;
@@ -7202,6 +7216,19 @@ void sctp_copy_sock(struct sock *newsk,
 	newinet->mc_list = NULL;
 }
 
+static inline void sctp_copy_descendant(struct sock *sk_to,
+					const struct sock *sk_from)
+{
+	int ancestor_size = sizeof(struct inet_sock) +
+			    sizeof(struct sctp_sock) -
+			    offsetof(struct sctp_sock, auto_asconf_list);
+
+	if (sk_from->sk_family == PF_INET6)
+		ancestor_size += sizeof(struct ipv6_pinfo);
+
+	__inet_sk_copy_descendant(sk_to, sk_from, ancestor_size);
+}
+
 /* Populate the fields of the newsk from the oldsk and migrate the assoc
  * and its messages to the newsk.
  */
@@ -7216,7 +7243,6 @@ static void sctp_sock_migrate(struct soc
 	struct sk_buff *skb, *tmp;
 	struct sctp_ulpevent *event;
 	struct sctp_bind_hashbucket *head;
-	struct list_head tmplist;
 
 	/* Migrate socket buffer sizes and all the socket level options to the
 	 * new socket.
@@ -7224,12 +7250,7 @@ static void sctp_sock_migrate(struct soc
 	newsk->sk_sndbuf = oldsk->sk_sndbuf;
 	newsk->sk_rcvbuf = oldsk->sk_rcvbuf;
 	/* Brute force copy old sctp opt. */
-	if (oldsp->do_auto_asconf) {
-		memcpy(&tmplist, &newsp->auto_asconf_list, sizeof(tmplist));
-		inet_sk_copy_descendant(newsk, oldsk);
-		memcpy(&newsp->auto_asconf_list, &tmplist, sizeof(tmplist));
-	} else
-		inet_sk_copy_descendant(newsk, oldsk);
+	sctp_copy_descendant(newsk, oldsk);
 
 	/* Restore the ep value that was overwritten with the above structure
 	 * copy.



  parent reply	other threads:[~2015-07-08  8:05 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-08  7:34 [PATCH 4.0 00/55] 4.0.8-stable review Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 01/55] config: Enable NEED_DMA_MAP_STATE by default when SWIOTLB is selected Greg Kroah-Hartman
2015-07-08  7:34   ` Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 02/55] netfilter: nft_rbtree: fix locking Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 03/55] ARM: clk-imx6q: refine satas parent Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 04/55] KVM: nSVM: Check for NRIPS support before updating control field Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 05/55] sparc: Use GFP_ATOMIC in ldc_alloc_exp_dring() as it can be called in softirq context Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 06/55] bridge: fix multicast router rlist endless loop Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 07/55] net: igb: fix the start time for periodic output signals Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 08/55] net: dont wait for order-3 page allocation Greg Kroah-Hartman
2015-07-08  7:34 ` Greg Kroah-Hartman [this message]
2015-07-08  7:34 ` [PATCH 4.0 10/55] bridge: fix br_stp_set_bridge_priority race conditions Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 11/55] packet: read num_members once in packet_rcv_fanout() Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 12/55] packet: avoid out of bounds read in round robin fanout Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 13/55] neigh: do not modify unlinked entries Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 14/55] tcp: Do not call tcp_fastopen_reset_cipher from interrupt context Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 15/55] ip: report the original address of ICMP messages Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 16/55] net/mlx4_en: Release TX QP when destroying TX ring Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 17/55] net/mlx4_en: Wake TX queues only when theres enough room Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 18/55] net/mlx4_en: Fix wrong csum complete report when rxvlan offload is disabled Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 19/55] mlx4: Disable HA for SRIOV PF RoCE devices Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 20/55] net: phy: fix phy link up when limiting speed via device tree Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 21/55] bnx2x: fix lockdep splat Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 22/55] sctp: Fix race between OOTB responce and route removal Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 23/55] amd-xgbe: Add the __GFP_NOWARN flag to Rx buffer allocation Greg Kroah-Hartman
2015-07-08  7:34 ` [PATCH 4.0 24/55] net: mvneta: introduce compatible string "marvell, armada-xp-neta" Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 25/55] ARM: mvebu: update Ethernet compatible string for Armada XP Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 26/55] net: mvneta: disable IP checksum with jumbo frames for Armada 370 Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 27/55] usb: gadget: f_fs: fix check in read operation Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 28/55] usb: gadget: f_fs: add extra check before unregister_gadget_item Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 29/55] crypto: talitos - avoid memleak in talitos_alg_alloc() Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 30/55] Revert "crypto: talitos - convert to use be16_add_cpu()" Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 31/55] iommu/arm-smmu: Fix broken ATOS check Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 32/55] iommu/amd: Handle large pages correctly in free_pagetable Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 33/55] mmc: sdhci: fix low memory corruption Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 34/55] intel_pstate: set BYT MSR with wrmsrl_on_cpu() Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 35/55] selinux: fix setting of security labels on NFS Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 36/55] arm: KVM: force execution of HCPTR access on VM exit Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 37/55] ARM: kvm: psci: fix handling of unimplemented functions Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 38/55] ARM: tegra20: Store CPU "resettable" status in IRAM Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 39/55] ARM: mvebu: fix suspend to RAM on big-endian configurations Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 40/55] tick/idle/powerpc: Do not register idle states with CPUIDLE_FLAG_TIMER_STOP set in periodic mode Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 41/55] powerpc/perf: Fix book3s kernel to userspace backtraces Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 42/55] x86/PCI: Use host bridge _CRS info on systems with >32 bit addressing Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 43/55] x86/PCI: Use host bridge _CRS info on Foxconn K8M890-8237A Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 44/55] KVM: mips: use id_to_memslot correctly Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 45/55] MIPS: Fix KVM guest fixmap address Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 46/55] KVM: s390: fix external call injection without sigp interpretation Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 47/55] s390/kdump: fix REGSET_VX_LOW vector register ELF notes Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 48/55] KVM: s390: virtio-ccw: dont overwrite config space values Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 51/55] fs: Fix S_NOSEC handling Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 52/55] fs/ufs: revert "ufs: fix deadlocks introduced by sb mutex merge" Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 53/55] fs/ufs: restore s_lock mutex Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 54/55] vfs: Remove incorrect debugging WARN in prepend_path Greg Kroah-Hartman
2015-07-08  7:35 ` [PATCH 4.0 55/55] vfs: Ignore unlocked mounts in fs_fully_visible Greg Kroah-Hartman
2015-07-08 14:09 ` [PATCH 4.0 00/55] 4.0.8-stable review Guenter Roeck
2015-07-08 16:34 ` Shuah Khan
2015-07-09  4:21 ` Sudip Mukherjee
2015-07-10 17:31   ` Greg Kroah-Hartman
2015-07-11  9:47     ` Sudip Mukherjee
2015-07-11 11:03       ` Sudip Mukherjee
2015-07-11 12:51         ` Sudip Mukherjee
2015-07-11 14:39       ` Greg Kroah-Hartman
2015-07-10 16:05 ` Kevin Hilman
2015-07-10 17:34   ` Greg Kroah-Hartman
2015-07-10 18:53     ` Kevin Hilman

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=20150708073239.371076747@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=hannes@stressinduktion.org \
    --cc=jiji@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcelo.leitner@gmail.com \
    --cc=nhorman@tuxdriver.com \
    --cc=stable@vger.kernel.org \
    /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.