Re: [PATCH nf-next] netfilter: conntrack: don't acquire lock during seq_printf

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Apr 14, 2016 at 01:16:56PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > >  net/netfilter/nf_conntrack_proto_sctp.c | 8 +-------
> > >  net/netfilter/nf_conntrack_proto_tcp.c  | 8 +-------
> > >  2 files changed, 2 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
> > > index 9578a7c..1d7ab96 100644
> > > --- a/net/netfilter/nf_conntrack_proto_sctp.c
> > > +++ b/net/netfilter/nf_conntrack_proto_sctp.c
> > > @@ -191,13 +191,7 @@ static void sctp_print_tuple(struct seq_file *s,
> > >  /* Print out the private part of the conntrack. */
> > >  static void sctp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
> > >  {
> > > -	enum sctp_conntrack state;
> > > -
> > > -	spin_lock_bh(&ct->lock);
> > > -	state = ct->proto.sctp.state;
> > 
> > Don't we need at least READ_ONCE() here?
> 
> Why?
> 
> seq_printf(s, "%s ", sctp_conntrack_names[ct->proto.sctp.state]);
> 
> I think thats fine, where do you see a problem?

ct->proto.sctp.state may be modified from another cpu while reading
this, is this read guaranteed to be atomic in any arch?