From 0da9a0c2638c8476b4a5021841912f249e3187dc Mon Sep 17 00:00:00 2001 From: Tommi Rantala Date: Thu, 15 Nov 2012 03:49:05 +0000 Subject: sctp: fix /proc/net/sctp/ memory leak Commit 13d782f ("sctp: Make the proc files per network namespace.") changed the /proc/net/sctp/ struct file_operations opener functions to use single_open_net() and seq_open_net(). Avoid leaking memory by using single_release_net() and seq_release_net() as the release functions. Discovered with Trinity (the syscall fuzzer). Signed-off-by: Tommi Rantala Acked-by: Neil Horman Cc: "Eric W. Biederman" Signed-off-by: David S. Miller --- net/sctp/proc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'net/sctp/proc.c') diff --git a/net/sctp/proc.c b/net/sctp/proc.c index c3bea269faf4..9966e7b16451 100644 --- a/net/sctp/proc.c +++ b/net/sctp/proc.c @@ -102,7 +102,7 @@ static const struct file_operations sctp_snmp_seq_fops = { .open = sctp_snmp_seq_open, .read = seq_read, .llseek = seq_lseek, - .release = single_release, + .release = single_release_net, }; /* Set up the proc fs entry for 'snmp' object. */ @@ -251,7 +251,7 @@ static const struct file_operations sctp_eps_seq_fops = { .open = sctp_eps_seq_open, .read = seq_read, .llseek = seq_lseek, - .release = seq_release, + .release = seq_release_net, }; /* Set up the proc fs entry for 'eps' object. */ @@ -372,7 +372,7 @@ static const struct file_operations sctp_assocs_seq_fops = { .open = sctp_assocs_seq_open, .read = seq_read, .llseek = seq_lseek, - .release = seq_release, + .release = seq_release_net, }; /* Set up the proc fs entry for 'assocs' object. */ @@ -517,7 +517,7 @@ static const struct file_operations sctp_remaddr_seq_fops = { .open = sctp_remaddr_seq_open, .read = seq_read, .llseek = seq_lseek, - .release = seq_release, + .release = seq_release_net, }; int __net_init sctp_remaddr_proc_init(struct net *net) -- cgit From 0b0fe913bf6d551642eb8892ed90be7358906379 Mon Sep 17 00:00:00 2001 From: Thomas Graf Date: Thu, 6 Dec 2012 09:25:04 +0000 Subject: sctp: proc: protect bind_addr->address_list accesses with rcu_read_lock() address_list is protected via the socket lock or RCU. Since we don't want to take the socket lock for each assoc we dump in procfs a RCU read-side critical section must be entered. V2: Skip local addresses marked as dead Cc: Vlad Yasevich Cc: Neil Horman Signed-off-by: Thomas Graf Acked-by: Vlad Yasevich Acked-by: Neil Horman Signed-off-by: David S. Miller --- net/sctp/proc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'net/sctp/proc.c') diff --git a/net/sctp/proc.c b/net/sctp/proc.c index 9966e7b16451..06b05ee17d8d 100644 --- a/net/sctp/proc.c +++ b/net/sctp/proc.c @@ -139,7 +139,11 @@ static void sctp_seq_dump_local_addrs(struct seq_file *seq, struct sctp_ep_commo primary = &peer->saddr; } - list_for_each_entry(laddr, &epb->bind_addr.address_list, list) { + rcu_read_lock(); + list_for_each_entry_rcu(laddr, &epb->bind_addr.address_list, list) { + if (!laddr->valid) + continue; + addr = &laddr->a; af = sctp_get_af_specific(addr->sa.sa_family); if (primary && af->cmp_addr(addr, primary)) { @@ -147,6 +151,7 @@ static void sctp_seq_dump_local_addrs(struct seq_file *seq, struct sctp_ep_commo } af->seq_dump_addr(seq, addr); } + rcu_read_unlock(); } /* Dump remote addresses of an association. */ -- cgit From 45122ca26ced7fae41049326a3797a73f961db2e Mon Sep 17 00:00:00 2001 From: Thomas Graf Date: Thu, 6 Dec 2012 09:25:05 +0000 Subject: sctp: Add RCU protection to assoc->transport_addr_list peer.transport_addr_list is currently only protected by sk_sock which is inpractical to acquire for procfs dumping purposes. This patch adds RCU protection allowing for the procfs readers to enter RCU read-side critical sections. Modification of the list continues to be serialized via sk_lock. V2: Use list_del_rcu() in sctp_association_free() to be safe Skip transports marked dead when dumping for procfs Cc: Vlad Yasevich Cc: Neil Horman Signed-off-by: Thomas Graf Acked-by: Vlad Yasevich Acked-by: Neil Horman Signed-off-by: David S. Miller --- net/sctp/proc.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'net/sctp/proc.c') diff --git a/net/sctp/proc.c b/net/sctp/proc.c index 06b05ee17d8d..8c19e97262ca 100644 --- a/net/sctp/proc.c +++ b/net/sctp/proc.c @@ -162,15 +162,20 @@ static void sctp_seq_dump_remote_addrs(struct seq_file *seq, struct sctp_associa struct sctp_af *af; primary = &assoc->peer.primary_addr; - list_for_each_entry(transport, &assoc->peer.transport_addr_list, + rcu_read_lock(); + list_for_each_entry_rcu(transport, &assoc->peer.transport_addr_list, transports) { addr = &transport->ipaddr; + if (transport->dead) + continue; + af = sctp_get_af_specific(addr->sa.sa_family); if (af->cmp_addr(addr, primary)) { seq_printf(seq, "*"); } af->seq_dump_addr(seq, addr); } + rcu_read_unlock(); } static void * sctp_eps_seq_start(struct seq_file *seq, loff_t *pos) @@ -441,12 +446,16 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v) head = &sctp_assoc_hashtable[hash]; sctp_local_bh_disable(); read_lock(&head->lock); + rcu_read_lock(); sctp_for_each_hentry(epb, node, &head->chain) { if (!net_eq(sock_net(epb->sk), seq_file_net(seq))) continue; assoc = sctp_assoc(epb); - list_for_each_entry(tsp, &assoc->peer.transport_addr_list, + list_for_each_entry_rcu(tsp, &assoc->peer.transport_addr_list, transports) { + if (tsp->dead) + continue; + /* * The remote address (ADDR) */ @@ -492,6 +501,7 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v) } } + rcu_read_unlock(); read_unlock(&head->lock); sctp_local_bh_enable(); -- cgit From b67bfe0d42cac56c512dd5da4b1b347a23f4b70a Mon Sep 17 00:00:00 2001 From: Sasha Levin Date: Wed, 27 Feb 2013 17:06:00 -0800 Subject: hlist: drop the node parameter from iterators I'm not sure why, but the hlist for each entry iterators were conceived list_for_each_entry(pos, head, member) The hlist ones were greedy and wanted an extra parameter: hlist_for_each_entry(tpos, pos, head, member) Why did they need an extra pos parameter? I'm not quite sure. Not only they don't really need it, it also prevents the iterator from looking exactly like the list iterator, which is unfortunate. Besides the semantic patch, there was some manual work required: - Fix up the actual hlist iterators in linux/list.h - Fix up the declaration of other iterators based on the hlist ones. - A very small amount of places were using the 'node' parameter, this was modified to use 'obj->member' instead. - Coccinelle didn't handle the hlist_for_each_entry_safe iterator properly, so those had to be fixed up manually. The semantic patch which is mostly the work of Peter Senna Tschudin is here: @@ iterator name hlist_for_each_entry, hlist_for_each_entry_continue, hlist_for_each_entry_from, hlist_for_each_entry_rcu, hlist_for_each_entry_rcu_bh, hlist_for_each_entry_continue_rcu_bh, for_each_busy_worker, ax25_uid_for_each, ax25_for_each, inet_bind_bucket_for_each, sctp_for_each_hentry, sk_for_each, sk_for_each_rcu, sk_for_each_from, sk_for_each_safe, sk_for_each_bound, hlist_for_each_entry_safe, hlist_for_each_entry_continue_rcu, nr_neigh_for_each, nr_neigh_for_each_safe, nr_node_for_each, nr_node_for_each_safe, for_each_gfn_indirect_valid_sp, for_each_gfn_sp, for_each_host; type T; expression a,c,d,e; identifier b; statement S; @@ -T b; <+... when != b ( hlist_for_each_entry(a, - b, c, d) S | hlist_for_each_entry_continue(a, - b, c) S | hlist_for_each_entry_from(a, - b, c) S | hlist_for_each_entry_rcu(a, - b, c, d) S | hlist_for_each_entry_rcu_bh(a, - b, c, d) S | hlist_for_each_entry_continue_rcu_bh(a, - b, c) S | for_each_busy_worker(a, c, - b, d) S | ax25_uid_for_each(a, - b, c) S | ax25_for_each(a, - b, c) S | inet_bind_bucket_for_each(a, - b, c) S | sctp_for_each_hentry(a, - b, c) S | sk_for_each(a, - b, c) S | sk_for_each_rcu(a, - b, c) S | sk_for_each_from -(a, b) +(a) S + sk_for_each_from(a) S | sk_for_each_safe(a, - b, c, d) S | sk_for_each_bound(a, - b, c) S | hlist_for_each_entry_safe(a, - b, c, d, e) S | hlist_for_each_entry_continue_rcu(a, - b, c) S | nr_neigh_for_each(a, - b, c) S | nr_neigh_for_each_safe(a, - b, c, d) S | nr_node_for_each(a, - b, c) S | nr_node_for_each_safe(a, - b, c, d) S | - for_each_gfn_sp(a, c, d, b) S + for_each_gfn_sp(a, c, d) S | - for_each_gfn_indirect_valid_sp(a, c, d, b) S + for_each_gfn_indirect_valid_sp(a, c, d) S | for_each_host(a, - b, c) S | for_each_host_safe(a, - b, c, d) S | for_each_mesh_entry(a, - b, c, d) S ) ...+> [akpm@linux-foundation.org: drop bogus change from net/ipv4/raw.c] [akpm@linux-foundation.org: drop bogus hunk from net/ipv6/raw.c] [akpm@linux-foundation.org: checkpatch fixes] [akpm@linux-foundation.org: fix warnings] [akpm@linux-foudnation.org: redo intrusive kvm changes] Tested-by: Peter Senna Tschudin Acked-by: Paul E. McKenney Signed-off-by: Sasha Levin Cc: Wu Fengguang Cc: Marcelo Tosatti Cc: Gleb Natapov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- net/sctp/proc.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'net/sctp/proc.c') diff --git a/net/sctp/proc.c b/net/sctp/proc.c index 8c19e97262ca..ab3bba8cb0a8 100644 --- a/net/sctp/proc.c +++ b/net/sctp/proc.c @@ -213,7 +213,6 @@ static int sctp_eps_seq_show(struct seq_file *seq, void *v) struct sctp_ep_common *epb; struct sctp_endpoint *ep; struct sock *sk; - struct hlist_node *node; int hash = *(loff_t *)v; if (hash >= sctp_ep_hashsize) @@ -222,7 +221,7 @@ static int sctp_eps_seq_show(struct seq_file *seq, void *v) head = &sctp_ep_hashtable[hash]; sctp_local_bh_disable(); read_lock(&head->lock); - sctp_for_each_hentry(epb, node, &head->chain) { + sctp_for_each_hentry(epb, &head->chain) { ep = sctp_ep(epb); sk = epb->sk; if (!net_eq(sock_net(sk), seq_file_net(seq))) @@ -321,7 +320,6 @@ static int sctp_assocs_seq_show(struct seq_file *seq, void *v) struct sctp_ep_common *epb; struct sctp_association *assoc; struct sock *sk; - struct hlist_node *node; int hash = *(loff_t *)v; if (hash >= sctp_assoc_hashsize) @@ -330,7 +328,7 @@ static int sctp_assocs_seq_show(struct seq_file *seq, void *v) head = &sctp_assoc_hashtable[hash]; sctp_local_bh_disable(); read_lock(&head->lock); - sctp_for_each_hentry(epb, node, &head->chain) { + sctp_for_each_hentry(epb, &head->chain) { assoc = sctp_assoc(epb); sk = epb->sk; if (!net_eq(sock_net(sk), seq_file_net(seq))) @@ -436,7 +434,6 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v) struct sctp_hashbucket *head; struct sctp_ep_common *epb; struct sctp_association *assoc; - struct hlist_node *node; struct sctp_transport *tsp; int hash = *(loff_t *)v; @@ -447,7 +444,7 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v) sctp_local_bh_disable(); read_lock(&head->lock); rcu_read_lock(); - sctp_for_each_hentry(epb, node, &head->chain) { + sctp_for_each_hentry(epb, &head->chain) { if (!net_eq(sock_net(epb->sk), seq_file_net(seq))) continue; assoc = sctp_assoc(epb); -- cgit