On Fri, 2010-09-24 at 15:14 -0400, Jeff Layton wrote:
> alloc_nfs_open_context creates a nfs_open_context struct, but doesn't
> initialize the list_head embedded in it. In newer kernels, this is
> harmless since the ctx is always put on a list soon after creation. In
> an older RHEL4 kernel however, it's possible (though unlikely) for a ctx
> to be passed to put_nfs_open_context without ever being put on a list,
> which can cause an oops.
>
> Since it's hard to predict how this code will change in the future,
> let's go ahead and initialize the list.
>
> Reported-by: Paul Bunyan <pbunyan@xxxxxxxxxx>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/nfs/inode.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 7d2d6c7..3185fc2 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -638,6 +638,7 @@ static struct nfs_open_context *alloc_nfs_open_context(struct path *path, struct
> ctx->dir_cookie = 0;
> nfs_init_lock_context(&ctx->lock_context);
> ctx->lock_context.open_context = ctx;
> + INIT_LIST_HEAD(&ctx->list);
> }
> return ctx;
> }
Hi Jeff,
I'm really too keen to merge stuff into 2.6.37 with the sole
justification being that it is needed to keep kernel 2.6.9 in sync.
That said, you appear to be in luck today: I found a reason to apply
something like the above in 2.6.37 too. Please see the patch below, that
appears to need to be applied to the nfs-for-2.6.37 branch.
Cheers
Trond
-------------------------------------------------------------------------------------------
commit 2128d32b4fbb85bba129ca7b6d4c8e118d4a325b
Author: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
Date: Mon Sep 27 15:51:20 2010 -0400
NFS: Really fix put_nfs_open_context()
In nfs_open_revalidate(), if the open_context() call returns an inode that
is not the same as dentry->d_inode, then we will call
put_nfs_open_context() with a valid dentry->d_inode, but without the
context being part of the nfsi->open_files list.
In this case too, we want to just skip the list removal, but we do want to
call the ->close_context() callback in order to close the NFSv4 state.
Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index cc22ef5..fca63b9 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -635,6 +635,7 @@ struct nfs_open_context *alloc_nfs_open_context(struct path *path, struct rpc_cr
ctx->dir_cookie = 0;
nfs_init_lock_context(&ctx->lock_context);
ctx->lock_context.open_context = ctx;
+ INIT_LIST_HEAD(&ctx->list);
}
return ctx;
}
@@ -650,14 +651,15 @@ static void __put_nfs_open_context(struct nfs_open_context *ctx, int is_sync)
{
struct inode *inode = ctx->path.dentry->d_inode;
- if (inode) {
+ if (!list_empty(&ctx->list)) {
if (!atomic_dec_and_lock(&ctx->lock_context.count, &inode->i_lock))
return;
list_del(&ctx->list);
spin_unlock(&inode->i_lock);
- NFS_PROTO(inode)->close_context(ctx, is_sync);
} else if (!atomic_dec_and_test(&ctx->lock_context.count))
return;
+ if (inode != NULL)
+ NFS_PROTO(inode)->close_context(ctx, is_sync);
if (ctx->cred != NULL)
put_rpccred(ctx->cred);
path_put(&ctx->path);
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
