2016-06-25 23:20:55

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH] NFS: Fix another OPEN_DOWNGRADE bug

Olga Kornievskaia reports that the following test fails to trigger
an OPEN_DOWNGRADE on the wire, and only triggers the final CLOSE.

fd0 = open(foo, RDRW) -- should be open on the wire for "both"
fd1 = open(foo, RDONLY) -- should be open on the wire for "read"
close(fd0) -- should trigger an open_downgrade
read(fd1)
close(fd1)

The issue is that we're missing a check for whether or not the current
state transitioned from an O_RDWR state as opposed to having transitioned
from a combination of O_RDONLY and O_WRONLY.

Reported-by: Olga Kornievskaia <[email protected]>
Fixes: cd9288ffaea4 ("NFSv4: Fix another bug in the close/open_downgrade code")
Cc: [email protected] # 2.6.33+
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4proc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 406dd3eb68e2..ff416d0e24bc 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2882,12 +2882,11 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
call_close |= is_wronly;
else if (is_wronly)
calldata->arg.fmode |= FMODE_WRITE;
+ if (calldata->arg.fmode != (FMODE_READ|FMODE_WRITE))
+ call_close |= is_rdwr;
} else if (is_rdwr)
calldata->arg.fmode |= FMODE_READ|FMODE_WRITE;

- if (calldata->arg.fmode == 0)
- call_close |= is_rdwr;
-
if (!nfs4_valid_open_stateid(state))
call_close = 0;
spin_unlock(&state->owner->so_lock);
--
2.7.4



2016-06-27 21:12:49

by Olga Kornievskaia

[permalink] [raw]
Subject: Re: [PATCH] NFS: Fix another OPEN_DOWNGRADE bug

On Sat, Jun 25, 2016 at 7:19 PM, Trond Myklebust
<[email protected]> wrote:
> Olga Kornievskaia reports that the following test fails to trigger
> an OPEN_DOWNGRADE on the wire, and only triggers the final CLOSE.
>
> fd0 = open(foo, RDRW) -- should be open on the wire for "both"
> fd1 = open(foo, RDONLY) -- should be open on the wire for "read"
> close(fd0) -- should trigger an open_downgrade
> read(fd1)
> close(fd1)
>
> The issue is that we're missing a check for whether or not the current
> state transitioned from an O_RDWR state as opposed to having transitioned
> from a combination of O_RDONLY and O_WRONLY.
>
> Reported-by: Olga Kornievskaia <[email protected]>
> Fixes: cd9288ffaea4 ("NFSv4: Fix another bug in the close/open_downgrade code")
> Cc: [email protected] # 2.6.33+
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/nfs4proc.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 406dd3eb68e2..ff416d0e24bc 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2882,12 +2882,11 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
> call_close |= is_wronly;
> else if (is_wronly)
> calldata->arg.fmode |= FMODE_WRITE;
> + if (calldata->arg.fmode != (FMODE_READ|FMODE_WRITE))
> + call_close |= is_rdwr;
> } else if (is_rdwr)
> calldata->arg.fmode |= FMODE_READ|FMODE_WRITE;
>
> - if (calldata->arg.fmode == 0)
> - call_close |= is_rdwr;
> -
> if (!nfs4_valid_open_stateid(state))
> call_close = 0;
> spin_unlock(&state->owner->so_lock);
> --
> 2.7.4
>

Thanks Trond. With this patch, in my test I see OPEN_DOWNGRADE sent.