linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org
Subject: Re: [heads-up] mknod() broken on nfs4
Date: Wed, 22 Jun 2011 01:23:59 +0100	[thread overview]
Message-ID: <20110622002359.GC11521@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20110621235900.GB11521@ZenIV.linux.org.uk>

On Wed, Jun 22, 2011 at 12:59:00AM +0100, Al Viro wrote:
> 	Try mknod(path, 0777, 0); with path leading into nfs4.  It
> leads to call of nfs_open_create(), with nd->intent.open.file being
> uninitialized.  Note that LOOKUP_CREATE is set and so's LOOKUP_EXCL,
> but LOOKUP_OPEN isn't.  So nfs_atomic_lookup() falls through to
> nfs_lookup(), which sees that we are doing exclusive create and just
> does d_instantiate(dentry, NULL) and do nothing else.  And then
> we hit ->create()...
> 
> 	Results are ugly - random errors (often -EINVAL or -ENOENT)
> and possibility of memory corruption if we manage to generate a request
> that won't fail on server.  
> 
> 	The really interesting question is what should we pass in
> NFS_PROTO(dir)->create() in open_flags.  I suspect that you are
> checking the wrong flag there (LOOKUP_CREATE instead of LOOKUP_OPEN),
> but I'm not sure what *should* be passed when LOOKUP_OPEN is not
> there...

Argh...  Alas, it's not that simple.  Even though the code in nfs_open_create()
and nfs4_proc_create() seems to imply that passing NULL as ctx is OK and
expected, in reality that blows up since we end up with NULL cred passed
to nfs4_do_open(), which oopses on attempt to do get_rpccred(NULL) from
nfs4_get_state_owner().

Folks, how is that code supposed to work?  lookup_instantiate_filp() should
*not* be called by vfs_create() triggered by mknod().  And I don't see any
codepath in nfs_open_create() that would not step into that.  ctx == NULL
is the only thing that would skip it and it definitely isn't survivable
by nfs4_proc_create().  Moreover, we need the rpc_cred to come from somewhere
and nfs4_proc_create() needs to get it from us.

BTW, AFAICS fuse will oops in such situation as well...

  reply	other threads:[~2011-06-22  0:24 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-21 23:59 [heads-up] mknod() broken on nfs4 Al Viro
2011-06-22  0:23 ` Al Viro [this message]
     [not found]   ` <20110622002359.GC11521-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2011-06-22 23:00     ` Trond Myklebust
     [not found]       ` <1308783620.25875.30.camel-SyLVLa/KEI9HwK5hSS5vWB2eb7JE58TQ@public.gmane.org>
2011-06-22 23:19         ` Al Viro
2011-06-23  2:48           ` Al Viro
2011-06-23  5:37           ` Al Viro

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=20110622002359.GC11521@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=Trond.Myklebust@netapp.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    /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).