[PATCH 1/1] NFSv4: open context can be NULL in nfs_put_open_context

[a***@public.gmane.org]

From: Andy Adamson <andros-HgOvQuBEEgTQT0dZR+***@public.gmane.org>

If a pgio error is caught before the nfs_pgio_header args->context is set
with get_nfs_open_context, the context can be NULL.

Signed-off-by: Andy Adamson <andros-HgOvQuBEEgTQT0dZR+***@public.gmane.org>
---
fs/nfs/inode.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 141c9f4..a2b148e 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -798,6 +798,9 @@ static void __put_nfs_open_context(struct nfs_open_context *ctx, int is_sync)
struct inode *inode = ctx->dentry->d_inode;
struct super_block *sb = ctx->dentry->d_sb;

+ if (ctx == NULL)
+ return;
+
if (!list_empty(&ctx->list)) {
if (!atomic_dec_and_lock(&ctx->lock_context.count, &inode->i_lock))
return;

[Weston Andros Adamson]

Post by a***@public.gmane.org
If a pgio error is caught before the nfs_pgio_header args->context is set
with get_nfs_open_context, the context can be NULL.

It looks like all three calls to nfs_pgio_error would hit this!

The fix looks good to me.

-dros

Post by a***@public.gmane.org
---
fs/nfs/inode.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 141c9f4..a2b148e 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -798,6 +798,9 @@ static void __put_nfs_open_context(struct nfs_open_context *ctx, int is_sync)
struct inode *inode = ctx->dentry->d_inode;
struct super_block *sb = ctx->dentry->d_sb;
+ if (ctx == NULL)
+ return;
+
if (!list_empty(&ctx->list)) {
if (!atomic_dec_and_lock(&ctx->lock_context.count, &inode->i_lock))
return;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Trond Myklebust]

Resending to list... Apologies for the duplicate...

On Mon, Oct 13, 2014 at 11:24 AM, Trond Myklebust

Hi Andy,

Post by a***@public.gmane.org
If a pgio error is caught before the nfs_pgio_header args->context is set
with get_nfs_open_context, the context can be NULL.
---
fs/nfs/inode.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 141c9f4..a2b148e 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -798,6 +798,9 @@ static void __put_nfs_open_context(struct
nfs_open_context *ctx, int is_sync)
struct inode *inode = ctx->dentry->d_inode;
struct super_block *sb = ctx->dentry->d_sb;
+ if (ctx == NULL)
+ return;
+
if (!list_empty(&ctx->list)) {
if (!atomic_dec_and_lock(&ctx->lock_context.count, &inode->i_lock))
return;
--
1.8.3.1

AFAICS, there is only one callsite where we need to check the context, so we
can avoid the check on all calls to put_nfs_open_context. Please see PATCH
1/2 that I just posted in response to an off-list bug report from Steve
Dickson.
Cheers
Trond
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData

--
Trond Myklebust

Linux NFS client maintainer, PrimaryData

trond.myklebust-7I+n7zu2hftEKMMhf/***@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Weston Andros Adamson]

Post by Trond Myklebust
Resending to list... Apologies for the duplicate...
On Mon, Oct 13, 2014 at 11:24 AM, Trond Myklebust

Hi Andy,

Post by a***@public.gmane.org
If a pgio error is caught before the nfs_pgio_header args->context is set
with get_nfs_open_context, the context can be NULL.
---
fs/nfs/inode.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 141c9f4..a2b148e 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -798,6 +798,9 @@ static void __put_nfs_open_context(struct
nfs_open_context *ctx, int is_sync)
struct inode *inode = ctx->dentry->d_inode;
struct super_block *sb = ctx->dentry->d_sb;
+ if (ctx == NULL)
+ return;
+
if (!list_empty(&ctx->list)) {
if (!atomic_dec_and_lock(&ctx->lock_context.count, &inode->i_lock))
return;
--
1.8.3.1

AFAICS, there is only one callsite where we need to check the context, so we
can avoid the check on all calls to put_nfs_open_context. Please see PATCH
1/2 that I just posted in response to an off-list bug report from Steve
Dickson.

Thanks Trond,

Both patches should fix the OOPS, but I think your reasoning to avoid the check in
all calls to put_nfs_open_context makes sense.

-dros

Post by Trond Myklebust

Cheers
Trond
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData

--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html