From mboxrd@z Thu Jan 1 00:00:00 1970 From: "J. Bruce Fields" Subject: Re: NFS/credentials leak in 2.6.29-rc1 Date: Wed, 21 Jan 2009 17:39:19 -0500 Message-ID: <20090121223919.GG4295@fieldses.org> References: <20090120235341.GA29017@fieldses.org> <20090120114649.GA15832@ioremap.net> <20090120151125.GB24266@fieldses.org> <20090120152304.GA28592@ioremap.net> <21428.1232540589@redhat.com> <20090121123728.GA1739@ioremap.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: David Howells , Trond Myklebust , linux-fsdevel@vger.kernel.org To: Evgeniy Polyakov Return-path: Received: from mail.fieldses.org ([141.211.133.115]:40407 "EHLO pickle.fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753483AbZAUWjV (ORCPT ); Wed, 21 Jan 2009 17:39:21 -0500 Content-Disposition: inline In-Reply-To: <20090121123728.GA1739@ioremap.net> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed, Jan 21, 2009 at 03:37:28PM +0300, Evgeniy Polyakov wrote: > On Wed, Jan 21, 2009 at 12:23:09PM +0000, David Howells (dhowells@redhat.com) wrote: > > J. Bruce Fields wrote: > > > > > - Finally, we put_cred(override_creds(new)). That modifies > > > current->cred again, putting the old value and getting the > > > new. > > > > > > Hm. But that last part's not OK; aren't we still holding our own > > > reference to new, in addition to the one that override_creds() just > > > took? So I think we need the following? > > > > Yes, you're right. override_creds() takes an extra ref on the argument it is > > passed, thus leaving the caller with their original reference intact. > > > > So really, you don't want to call override_creds() as that will cost you an > > extra atomic_inc() and atomic_dec_and_test(). I recommend you replace: > > > > put_cred(override_creds(new)); > > > > with: > > > > revert_creds(new); > > > > I think that should do the right thing. It may look a bit odd, but it'll be > > quicker. If you object to using revert_creds)( because of the name, we can > > come up with an alternative name. > > With additional put_cred, i.e.: > > put_cred(override_creds(new)); > put_cred(new); > return 0; > > I got following fun tcpdump and failed mount (it stuck, but can be interrupted): I'm really bad at reading tcpdump. Where's the problem here? It looks like every call gets a reply. --b. > > 15:34:41.253911 IP 77.88.20.183.1835336279 > 77.88.20.182.2049: 44 null > 15:34:41.253916 IP 77.88.20.182.2049 > 77.88.20.183.728: . ack 44 win 88 > 15:34:41.254103 IP 77.88.20.182.2049 > 77.88.20.183.1835336279: reply ok 28 null > 15:34:41.254229 IP 77.88.20.183.728 > 77.88.20.182.2049: . ack 29 win 89 > 15:34:41.254238 IP 77.88.20.183.1852113495 > 77.88.20.182.2049: 44 null > 15:34:41.254271 IP 77.88.20.182.2049 > 77.88.20.183.1852113495: reply ok 28 null > 15:34:41.254378 IP 77.88.20.183.1868890711 > 77.88.20.182.2049: 100 fsinfo [|nfs] > 15:34:41.254411 IP 77.88.20.182.2049 > 77.88.20.183.1868890711: reply ok 36 fsinfo [|nfs] > 15:34:41.254528 IP 77.88.20.183.1885667927 > 77.88.20.182.2049: 100 fsinfo [|nfs] > 15:34:41.254555 IP 77.88.20.182.2049 > 77.88.20.183.1885667927: reply ok 36 fsinfo [|nfs] > > But no corruption in the dmesg (like oops or bug). > > -- > Evgeniy Polyakov