linux-kernel - Re: [PATCH RFC 0/5] rcu doc updates for whatisRCU and checklist

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20181006224729.GA7130@thunk.org>
Date:   Sat, 6 Oct 2018 18:47:29 -0400
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     Joel Fernandes <joel@...lfernandes.org>
Cc:     "Paul E. McKenney" <paulmck@...ux.ibm.com>,
        linux-kernel@...r.kernel.org, Jonathan Corbet <corbet@....net>,
        Josh Triplett <josh@...htriplett.org>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        linux-doc@...r.kernel.org,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Steven Rostedt <rostedt@...dmis.org>, pantin@...gle.com
Subject: Re: [PATCH RFC 0/5] rcu doc updates for whatisRCU and checklist

On Fri, Oct 05, 2018 at 09:40:57PM -0700, Joel Fernandes wrote:
> 
> Could you annotate this pointer (sbi->s_qf_names) with __rcu so it can be
> checked by sparse for proper usage? Its also point #16 in the checklist.txt
> RCU document. I enclosed a diff to do this below.

Sure.

> I also saw a bunch of places in super.c where the pointer isn't accessed from
> an rcu read section or rcu_dereference, but it was a quick look so sorry if I
> missed something. If its true, then are you planning to convert these to use
> rcu_dereference and wrapped by an rcu_read_lock/unlock as well?

These are places where we are holding s_umount, so there's no way
s_qf_names can change out from under us.  So the conversion should be
to use rcu_dereference_protected().

> > +		to_free[i] = rcu_dereference_protected(sbi->s_qf_names[i],
> > +						       &sb->s_umount);
> 
> Also should this be the following?
> 		to_free[i] = rcu_dereference_protected(sbi->s_qf_names[i],
> 					       lockdep_is_held(&sb->s_umount));

Yep, good catch, thanks.

						- Ted