Re: [PATCH] svcrpc: modifying positive sunrpc cache entries is racy

["J. Bruce Fields" <bfields@fieldses.org>]

On Wed, Dec 29, 2010 at 03:47:52PM -0500, bfields wrote:
> From: J. Bruce Fields <bfields@redhat.com>
> 
> Once a sunrpc cache entry is non-NEGATIVE, we should be replacing it
> (and allowing any concurrent users to destroy it on last put) instead of
> trying to update it in place.
> 
> Otherwise someone referencing the ip_map we're modifying here could try
> to use the m_client just as we're putting the last reference.
> 
> The bug should only be seen by users of the legacy nfsd interfaces.
> 
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
>  net/sunrpc/svcauth_unix.c |   18 ++++++++++++++++--
>  1 files changed, 16 insertions(+), 2 deletions(-)
> 
> Intended to apply for 2.6.38 if this looks right....

Also noticed while trying to track down an rhel5 oops in
svcauth_unix_set_client():

	- cache_check() can set an entry negative in place, which if
	  nothing else must cause a leak in some cases.  (Because when
	  the entry is eventually destroyed, it will be assumed to not
	  have any contents.)  I suppose the fix is again to try to
	  adding a new negative entry instead.

	- since cache_check() doesn't use any locking, I can't see what
	  guarantees that when it sees the CACHE_VALID bit set and
	  CACHE_NEGATIVE cleared, it must necessarily see the new
	  contents.   I think that'd be fixed by a wmb() before setting
	  those bits and a rmb() after checking them.  I don't know if
	  it's actually possible to hit that bug....

--b.