Re: [PATCH 3.0.y, 3.2.y] NFSv4: Revalidate uid/gid after open

[Greg KH <greg@kroah.com>]

On Fri, May 11, 2012 at 04:20:20AM -0500, Jonathan Nieder wrote:
> This is a shorter (and more appropriate for stable kernels) analog to
> the following upstream commit:
> 
> commit 6926afd1925a54a13684ebe05987868890665e2b
> Author: Trond Myklebust <Trond.Myklebust@netapp.com>
> Date:   Sat Jan 7 13:22:46 2012 -0500
> 
>     NFSv4: Save the owner/group name string when doing open
> 
>     ...so that we can do the uid/gid mapping outside the asynchronous RPC
>     context.
>     This fixes a bug in the current NFSv4 atomic open code where the client
>     isn't able to determine what the true uid/gid fields of the file are,
>     (because the asynchronous nature of the OPEN call denies it the ability
>     to do an upcall) and so fills them with default values, marking the
>     inode as needing revalidation.
>     Unfortunately, in some cases, the VFS will do some additional sanity
>     checks on the file, and may override the server's decision to allow
>     the open because it sees the wrong owner/group fields.
> 
>     Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> 
> Without this patch, logging into two different machines with home
> directories mounted over NFS4 and then running "vim" and typing ":q"
> in each reliably produces the following error on the second machine:
> 
> 	E137: Viminfo file is not writable: /users/system/rtheys/.viminfo
> 
> This regression was introduced by 80e52aced138 ("NFSv4: Don't do
> idmapper upcalls for asynchronous RPC calls", merged during the 2.6.32
> cycle) --- after the OPEN call, .viminfo has the default values for
> st_uid and st_gid (0xfffffffe) cached because we do not want to let
> rpciod wait for an idmapper upcall to fill them in.
> 
> The fix used in mainline is to save the owner and group as strings and
> perform the upcall in _nfs4_proc_open outside the rpciod context,
> which takes about 600 lines.  For stable, we can do something similar
> with a one-liner: make open check for the stale fields and make a
> (synchronous) GETATTR call to fill them when needed.
> 
> Trond dictated the patch, I typed it in, and Rik tested it.
> 
> Addresses http://bugs.debian.org/659111 and
>           https://bugzilla.redhat.com/789298
> 
> Reported-by: Rik Theys <Rik.Theys@esat.kuleuven.be>
> Explained-by: David Flyn <davidf@rd.bbc.co.uk>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Tested-by: Rik Theys <Rik.Theys@esat.kuleuven.be>
> ---
> Hi Ben and Greg,
> 
> Please consider this patch for inclusion in 3.0.y and 3.2.y kernels.
> 3.3.y doesn't need it since 6926afd1925a was merged during the 3.3
> merge window.

Wonderful work, now applied to the 3.0-stable tree, thanks for doing
this.

greg k-h