From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from fieldses.org ([174.143.236.118]:40588 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751181Ab1ADDIG (ORCPT ); Mon, 3 Jan 2011 22:08:06 -0500 Date: Mon, 3 Jan 2011 22:08:05 -0500 From: "J. Bruce Fields" To: linux-nfs@vger.kernel.org Cc: Neil Brown Subject: Re: [PATCH] svcrpc: modifying positive sunrpc cache entries is racy Message-ID: <20110104030805.GA3194@fieldses.org> References: <20101229204752.GC12218@fieldses.org> <20110103222605.GA24811@fieldses.org> Content-Type: text/plain; charset=us-ascii In-Reply-To: <20110103222605.GA24811@fieldses.org> Sender: linux-nfs-owner@vger.kernel.org List-ID: MIME-Version: 1.0 On Mon, Jan 03, 2011 at 05:26:05PM -0500, J. Bruce Fields wrote: > On Wed, Dec 29, 2010 at 03:47:52PM -0500, bfields wrote: > > From: J. Bruce Fields > > > > 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. > > Or the following seems simpler. > > (And I was thinking it was necessary to ensure that the right thing > happened to the cached xprt->xpt_auth_cache entry--though on a second > look I see that sunrpc_cache_update also expires the replaced entry > immediately. Still, this seems simpler if it also works.) Eh, on third thoughts: we probably do want a real negative entry created in the cache, so I think the original patch was correct! --b. > > --b. > > commit 20771de3185bf0031aa767d3b19f3e744a465a0c > Author: J. Bruce Fields > Date: Fri Dec 24 14:03:38 2010 -0500 > > svcrpc: modifying valid sunrpc cache entries is racy > > Once a sunrpc cache entry is non-NEGATIVE, we shouldn't attempt to > modify 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. > > Instead, just set the expiry time so it expires immediately. > > The bug should only be seen by users of the legacy nfsd interfaces. > > Signed-off-by: J. Bruce Fields > > diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c > index a04ac91..5edc147 100644 > --- a/net/sunrpc/svcauth_unix.c > +++ b/net/sunrpc/svcauth_unix.c > @@ -401,8 +401,7 @@ struct auth_domain *auth_unix_lookup(struct net *net, struct in6_addr *addr) > return NULL; > > if ((ipm->m_client->addr_changes - ipm->m_add_change) >0) { > - if (test_and_set_bit(CACHE_NEGATIVE, &ipm->h.flags) == 0) > - auth_domain_put(&ipm->m_client->h); > + ipm->h.expiry_time = 0; > rv = NULL; > } else { > rv = &ipm->m_client->h; > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html