linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Evgeniy Polyakov <zbr@ioremap.net>
To: David Howells <dhowells@redhat.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
	Trond Myklebust <trond.myklebust@fys.uio.no>,
	linux-fsdevel@vger.kernel.org
Subject: Re: NFS/credentials leak in 2.6.29-rc1
Date: Wed, 21 Jan 2009 15:37:28 +0300	[thread overview]
Message-ID: <20090121123728.GA1739@ioremap.net> (raw)
In-Reply-To: <21428.1232540589@redhat.com>

On Wed, Jan 21, 2009 at 12:23:09PM +0000, David Howells (dhowells@redhat.com) wrote:
> J. Bruce Fields <bfields@fieldses.org> 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):

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 <nop,nop,timestamp 125462 37402358>
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 <nop,nop,timestamp 37402358 125463>
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

  reply	other threads:[~2009-01-21 12:37 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-20 11:46 NFS/credentials leak in 2.6.29-rc1 Evgeniy Polyakov
2009-01-20 13:37 ` Trond Myklebust
2009-01-20 13:49   ` Evgeniy Polyakov
2009-01-20 15:11 ` J. Bruce Fields
2009-01-20 15:23   ` Evgeniy Polyakov
2009-01-20 23:53     ` J. Bruce Fields
2009-01-21 12:23     ` David Howells
2009-01-21 12:37       ` Evgeniy Polyakov [this message]
2009-01-21 22:39         ` J. Bruce Fields
2009-01-21 22:46           ` Evgeniy Polyakov
2009-01-21 23:18             ` J. Bruce Fields
2009-01-21 23:31               ` Evgeniy Polyakov
2009-01-27  0:49         ` J. Bruce Fields
2009-01-27  9:26           ` Evgeniy Polyakov
2009-01-27 22:07             ` J. Bruce Fields
2009-01-29 14:37               ` Evgeniy Polyakov
2009-01-29 18:52                 ` J. Bruce Fields
2009-01-29 19:00                   ` Evgeniy Polyakov
2009-02-05 13:22         ` David Howells
2009-02-05 17:21           ` J. Bruce Fields
2009-01-21 13:17       ` David Howells
2009-01-21 13:18         ` Evgeniy Polyakov
2009-01-21 22:37       ` J. Bruce Fields
2009-01-22  7:08       ` David Howells
2009-01-22 16:43         ` J. Bruce Fields
2009-01-20 21:44 ` David Howells
2009-01-21 22:42   ` J. Bruce Fields

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090121123728.GA1739@ioremap.net \
    --to=zbr@ioremap.net \
    --cc=bfields@fieldses.org \
    --cc=dhowells@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=trond.myklebust@fys.uio.no \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).