diff mbox

[net] sctp: fix rcu usage on proc listing

Message ID c68813736c92f28f78f1c8cc8f362e4c964f7899.1453240529.git.marcelo.leitner@gmail.com
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Marcelo Leitner Jan. 19, 2016, 9:59 p.m. UTC
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

commit 39f66a7dce32 ("sctp: apply rhashtable api to sctp procfs")
removed a rcu_read_lock() too much at sctp_remaddr_seq_show().

Fixes: 39f66a7dce32 ("sctp: apply rhashtable api to sctp procfs")
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/proc.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Marcelo Ricardo Leitner Jan. 19, 2016, 10:02 p.m. UTC | #1
[just cc'ing linux-sctp@vger.kernel.org]

On Tue, Jan 19, 2016 at 07:59:03PM -0200, Marcelo Ricardo Leitner wrote:
> From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> 
> commit 39f66a7dce32 ("sctp: apply rhashtable api to sctp procfs")
> removed a rcu_read_lock() too much at sctp_remaddr_seq_show().
> 
> Fixes: 39f66a7dce32 ("sctp: apply rhashtable api to sctp procfs")
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
>  net/sctp/proc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/sctp/proc.c b/net/sctp/proc.c
> index dfa7eeccb5373053f460f1026a58e4a2ba015a69..4fdf4387910696492ba568b1096dd12e8e7221e2 100644
> --- a/net/sctp/proc.c
> +++ b/net/sctp/proc.c
> @@ -491,6 +491,7 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v)
>  	tsp = (struct sctp_transport *)v;
>  	assoc = tsp->asoc;
>  
> +	rcu_read_lock();
>  	list_for_each_entry_rcu(tsp, &assoc->peer.transport_addr_list,
>  				transports) {
>  		if (tsp->dead)
> @@ -543,6 +544,7 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v)
>  
>  		seq_printf(seq, "\n");
>  	}
> +	rcu_read_unlock();
>  
>  	return 0;
>  }
> -- 
> 2.5.0
>
Marcelo Ricardo Leitner Jan. 20, 2016, 1:07 a.m. UTC | #2
On Tue, Jan 19, 2016 at 08:02:30PM -0200, Marcelo Ricardo Leitner wrote:
> > --- a/net/sctp/proc.c
> > +++ b/net/sctp/proc.c
> > @@ -491,6 +491,7 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v)
> >  	tsp = (struct sctp_transport *)v;
> >  	assoc = tsp->asoc;
> >  
> > +	rcu_read_lock();

should have used rcu_read_lock_bh() here, will send a v2 tomorrow.

> >  	list_for_each_entry_rcu(tsp, &assoc->peer.transport_addr_list,
> >  				transports) {
> >  		if (tsp->dead)
> > @@ -543,6 +544,7 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v)
> >  
> >  		seq_printf(seq, "\n");
> >  	}
> > +	rcu_read_unlock();
> >  
> >  	return 0;
> >  }
> > -- 
> > 2.5.0
> > 
>
Marcelo Ricardo Leitner Jan. 21, 2016, 1:05 p.m. UTC | #3
On Tue, Jan 19, 2016 at 11:07:20PM -0200, Marcelo Ricardo Leitner wrote:
> On Tue, Jan 19, 2016 at 08:02:30PM -0200, Marcelo Ricardo Leitner wrote:
> > > --- a/net/sctp/proc.c
> > > +++ b/net/sctp/proc.c
> > > @@ -491,6 +491,7 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v)
> > >  	tsp = (struct sctp_transport *)v;
> > >  	assoc = tsp->asoc;
> > >  
> > > +	rcu_read_lock();
> 
> should have used rcu_read_lock_bh() here, will send a v2 tomorrow.

Please just drop this one, no change is actually needed.
rhashtable_walk_start() already puts the rcu_read_lock() in there and
disabling bh is not necessary. Thanks.

> > >  	list_for_each_entry_rcu(tsp, &assoc->peer.transport_addr_list,
> > >  				transports) {
> > >  		if (tsp->dead)
> > > @@ -543,6 +544,7 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v)
> > >  
> > >  		seq_printf(seq, "\n");
> > >  	}
> > > +	rcu_read_unlock();
> > >  
> > >  	return 0;
> > >  }
> > > -- 
> > > 2.5.0
> > > 
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
diff mbox

Patch

diff --git a/net/sctp/proc.c b/net/sctp/proc.c
index dfa7eeccb5373053f460f1026a58e4a2ba015a69..4fdf4387910696492ba568b1096dd12e8e7221e2 100644
--- a/net/sctp/proc.c
+++ b/net/sctp/proc.c
@@ -491,6 +491,7 @@  static int sctp_remaddr_seq_show(struct seq_file *seq, void *v)
 	tsp = (struct sctp_transport *)v;
 	assoc = tsp->asoc;
 
+	rcu_read_lock();
 	list_for_each_entry_rcu(tsp, &assoc->peer.transport_addr_list,
 				transports) {
 		if (tsp->dead)
@@ -543,6 +544,7 @@  static int sctp_remaddr_seq_show(struct seq_file *seq, void *v)
 
 		seq_printf(seq, "\n");
 	}
+	rcu_read_unlock();
 
 	return 0;
 }