* [PATCH v2 1/2] NFSv4: Fix problems with close in the presence of a delegation
@ 2014-08-26 18:30 Trond Myklebust
2014-08-26 18:30 ` [PATCH v2 2/2] NFSv4: Don't clear the open state when we just did an OPEN_DOWNGRADE Trond Myklebust
2014-08-26 20:11 ` [PATCH v2 1/2] NFSv4: Fix problems with close in the presence of a delegation Anna Schumaker
0 siblings, 2 replies; 6+ messages in thread
From: Trond Myklebust @ 2014-08-26 18:30 UTC (permalink / raw
To: linux-nfs; +Cc: James Drews, jrs
In the presence of delegations, we can no longer assume that the
state->n_rdwr, state->n_rdonly, state->n_wronly reflect the open
stateid share mode, and so we need to need to calculate the initial
value for calldata->arg.fmode using the state->flags.
Reported-by: James Drews <drews@engr.wisc.edu>
Fixes: 88069f77e1ac5 (NFSv41: Fix a potential state leakage when...)
Cc: stable@vger.kernel.org # 2.6.33+
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
fs/nfs/nfs4proc.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 75ae8d22f067..7d67d5b332d4 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2601,6 +2601,7 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
struct nfs4_closedata *calldata = data;
struct nfs4_state *state = calldata->state;
struct inode *inode = calldata->inode;
+ bool is_rdonly, is_wronly, is_rdwr;
int call_close = 0;
dprintk("%s: begin!\n", __func__);
@@ -2608,24 +2609,30 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
goto out_wait;
task->tk_msg.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_OPEN_DOWNGRADE];
- calldata->arg.fmode = FMODE_READ|FMODE_WRITE;
spin_lock(&state->owner->so_lock);
+ is_rdwr = test_bit(NFS_O_RDWR_STATE, &state->flags);
+ is_rdonly = test_bit(NFS_O_RDONLY_STATE, &state->flags);
+ is_wronly = test_bit(NFS_O_WRONLY_STATE, &state->flags);
+ spin_unlock(&state->owner->so_lock);
+ /* Calculate the current open share mode */
+ calldata->arg.fmode = 0;
+ if (is_rdonly || is_rdwr)
+ calldata->arg.fmode |= FMODE_READ;
+ if (is_wronly || is_rdwr)
+ calldata->arg.fmode |= FMODE_WRITE;
/* Calculate the change in open mode */
if (state->n_rdwr == 0) {
if (state->n_rdonly == 0) {
- call_close |= test_bit(NFS_O_RDONLY_STATE, &state->flags);
- call_close |= test_bit(NFS_O_RDWR_STATE, &state->flags);
+ call_close |= is_rdonly || is_rdwr;
calldata->arg.fmode &= ~FMODE_READ;
}
if (state->n_wronly == 0) {
- call_close |= test_bit(NFS_O_WRONLY_STATE, &state->flags);
- call_close |= test_bit(NFS_O_RDWR_STATE, &state->flags);
+ call_close |= is_wronly || is_rdwr;
calldata->arg.fmode &= ~FMODE_WRITE;
}
}
if (!nfs4_valid_open_stateid(state))
call_close = 0;
- spin_unlock(&state->owner->so_lock);
if (!call_close) {
/* Note: exit _without_ calling nfs4_close_done */
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] NFSv4: Don't clear the open state when we just did an OPEN_DOWNGRADE
2014-08-26 18:30 [PATCH v2 1/2] NFSv4: Fix problems with close in the presence of a delegation Trond Myklebust
@ 2014-08-26 18:30 ` Trond Myklebust
2014-08-26 20:11 ` [PATCH v2 1/2] NFSv4: Fix problems with close in the presence of a delegation Anna Schumaker
1 sibling, 0 replies; 6+ messages in thread
From: Trond Myklebust @ 2014-08-26 18:30 UTC (permalink / raw
To: linux-nfs; +Cc: James Drews, jrs
If we did an OPEN_DOWNGRADE, then the right thing to do on success, is
to apply the new open mode to the struct nfs4_state. Instead, we were
unconditionally clearing the state, making it appear to our state
machinery as if we had just performed a CLOSE.
Fixes: 226056c5c312b (NFSv4: Use correct locking when updating nfs4_state...)
Cc: stable@vger.kernel.org # 3.15+
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
fs/nfs/nfs4proc.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7d67d5b332d4..fe63691822bc 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2560,6 +2560,7 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
struct nfs4_closedata *calldata = data;
struct nfs4_state *state = calldata->state;
struct nfs_server *server = NFS_SERVER(calldata->inode);
+ nfs4_stateid *res_stateid = NULL;
dprintk("%s: begin!\n", __func__);
if (!nfs4_sequence_done(task, &calldata->res.seq_res))
@@ -2570,12 +2571,12 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
*/
switch (task->tk_status) {
case 0:
- if (calldata->roc)
+ res_stateid = &calldata->res.stateid;
+ if (calldata->arg.fmode == 0 && calldata->roc)
pnfs_roc_set_barrier(state->inode,
calldata->roc_barrier);
- nfs_clear_open_stateid(state, &calldata->res.stateid, 0);
renew_lease(server, calldata->timestamp);
- goto out_release;
+ break;
case -NFS4ERR_ADMIN_REVOKED:
case -NFS4ERR_STALE_STATEID:
case -NFS4ERR_OLD_STATEID:
@@ -2589,7 +2590,7 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
goto out_release;
}
}
- nfs_clear_open_stateid(state, NULL, calldata->arg.fmode);
+ nfs_clear_open_stateid(state, res_stateid, calldata->arg.fmode);
out_release:
nfs_release_seqid(calldata->arg.seqid);
nfs_refresh_inode(calldata->inode, calldata->res.fattr);
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] NFSv4: Fix problems with close in the presence of a delegation
2014-08-26 18:30 [PATCH v2 1/2] NFSv4: Fix problems with close in the presence of a delegation Trond Myklebust
2014-08-26 18:30 ` [PATCH v2 2/2] NFSv4: Don't clear the open state when we just did an OPEN_DOWNGRADE Trond Myklebust
@ 2014-08-26 20:11 ` Anna Schumaker
2014-08-26 20:15 ` Trond Myklebust
1 sibling, 1 reply; 6+ messages in thread
From: Anna Schumaker @ 2014-08-26 20:11 UTC (permalink / raw
To: Trond Myklebust, linux-nfs; +Cc: James Drews, jrs
On 08/26/2014 02:30 PM, Trond Myklebust wrote:
> In the presence of delegations, we can no longer assume that the
> state->n_rdwr, state->n_rdonly, state->n_wronly reflect the open
> stateid share mode, and so we need to need to calculate the initial
Nit: Remove the duplicate "need to" (above).
> value for calldata->arg.fmode using the state->flags.
>
> Reported-by: James Drews <drews@engr.wisc.edu>
> Fixes: 88069f77e1ac5 (NFSv41: Fix a potential state leakage when...)
> Cc: stable@vger.kernel.org # 2.6.33+
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> ---
> fs/nfs/nfs4proc.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 75ae8d22f067..7d67d5b332d4 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2601,6 +2601,7 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
> struct nfs4_closedata *calldata = data;
> struct nfs4_state *state = calldata->state;
> struct inode *inode = calldata->inode;
> + bool is_rdonly, is_wronly, is_rdwr;
> int call_close = 0;
>
> dprintk("%s: begin!\n", __func__);
> @@ -2608,24 +2609,30 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
> goto out_wait;
>
> task->tk_msg.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_OPEN_DOWNGRADE];
> - calldata->arg.fmode = FMODE_READ|FMODE_WRITE;
> spin_lock(&state->owner->so_lock);
> + is_rdwr = test_bit(NFS_O_RDWR_STATE, &state->flags);
> + is_rdonly = test_bit(NFS_O_RDONLY_STATE, &state->flags);
> + is_wronly = test_bit(NFS_O_WRONLY_STATE, &state->flags);
> + spin_unlock(&state->owner->so_lock);
> + /* Calculate the current open share mode */
> + calldata->arg.fmode = 0;
> + if (is_rdonly || is_rdwr)
> + calldata->arg.fmode |= FMODE_READ;
> + if (is_wronly || is_rdwr)
> + calldata->arg.fmode |= FMODE_WRITE;
> /* Calculate the change in open mode */
> if (state->n_rdwr == 0) {
Do we need the owner lock for reading state->n_rdwr, n_rdonly, and n_wronly?
Anna
> if (state->n_rdonly == 0) {
> - call_close |= test_bit(NFS_O_RDONLY_STATE, &state->flags);
> - call_close |= test_bit(NFS_O_RDWR_STATE, &state->flags);
> + call_close |= is_rdonly || is_rdwr;
> calldata->arg.fmode &= ~FMODE_READ;
> }
> if (state->n_wronly == 0) {
> - call_close |= test_bit(NFS_O_WRONLY_STATE, &state->flags);
> - call_close |= test_bit(NFS_O_RDWR_STATE, &state->flags);
> + call_close |= is_wronly || is_rdwr;
> calldata->arg.fmode &= ~FMODE_WRITE;
> }
> }
> if (!nfs4_valid_open_stateid(state))
> call_close = 0;
> - spin_unlock(&state->owner->so_lock);
>
> if (!call_close) {
> /* Note: exit _without_ calling nfs4_close_done */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] NFSv4: Fix problems with close in the presence of a delegation
2014-08-26 20:11 ` [PATCH v2 1/2] NFSv4: Fix problems with close in the presence of a delegation Anna Schumaker
@ 2014-08-26 20:15 ` Trond Myklebust
2014-08-26 20:21 ` Anna Schumaker
0 siblings, 1 reply; 6+ messages in thread
From: Trond Myklebust @ 2014-08-26 20:15 UTC (permalink / raw
To: Anna Schumaker; +Cc: Linux NFS Mailing List, James Drews, jrs
On Tue, Aug 26, 2014 at 4:11 PM, Anna Schumaker
<Anna.Schumaker@netapp.com> wrote:
> On 08/26/2014 02:30 PM, Trond Myklebust wrote:
>> In the presence of delegations, we can no longer assume that the
>> state->n_rdwr, state->n_rdonly, state->n_wronly reflect the open
>> stateid share mode, and so we need to need to calculate the initial
> Nit: Remove the duplicate "need to" (above).
>
>> value for calldata->arg.fmode using the state->flags.
>>
>> Reported-by: James Drews <drews@engr.wisc.edu>
>> Fixes: 88069f77e1ac5 (NFSv41: Fix a potential state leakage when...)
>> Cc: stable@vger.kernel.org # 2.6.33+
>> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
>> ---
>> fs/nfs/nfs4proc.c | 19 +++++++++++++------
>> 1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 75ae8d22f067..7d67d5b332d4 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -2601,6 +2601,7 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
>> struct nfs4_closedata *calldata = data;
>> struct nfs4_state *state = calldata->state;
>> struct inode *inode = calldata->inode;
>> + bool is_rdonly, is_wronly, is_rdwr;
>> int call_close = 0;
>>
>> dprintk("%s: begin!\n", __func__);
>> @@ -2608,24 +2609,30 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
>> goto out_wait;
>>
>> task->tk_msg.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_OPEN_DOWNGRADE];
>> - calldata->arg.fmode = FMODE_READ|FMODE_WRITE;
>> spin_lock(&state->owner->so_lock);
>> + is_rdwr = test_bit(NFS_O_RDWR_STATE, &state->flags);
>> + is_rdonly = test_bit(NFS_O_RDONLY_STATE, &state->flags);
>> + is_wronly = test_bit(NFS_O_WRONLY_STATE, &state->flags);
>> + spin_unlock(&state->owner->so_lock);
>> + /* Calculate the current open share mode */
>> + calldata->arg.fmode = 0;
>> + if (is_rdonly || is_rdwr)
>> + calldata->arg.fmode |= FMODE_READ;
>> + if (is_wronly || is_rdwr)
>> + calldata->arg.fmode |= FMODE_WRITE;
>> /* Calculate the change in open mode */
>> if (state->n_rdwr == 0) {
> Do we need the owner lock for reading state->n_rdwr, n_rdonly, and n_wronly?
Duh, yes, you're right. I'll revert that part of the change.
> Anna
>> if (state->n_rdonly == 0) {
>> - call_close |= test_bit(NFS_O_RDONLY_STATE, &state->flags);
>> - call_close |= test_bit(NFS_O_RDWR_STATE, &state->flags);
>> + call_close |= is_rdonly || is_rdwr;
>> calldata->arg.fmode &= ~FMODE_READ;
>> }
>> if (state->n_wronly == 0) {
>> - call_close |= test_bit(NFS_O_WRONLY_STATE, &state->flags);
>> - call_close |= test_bit(NFS_O_RDWR_STATE, &state->flags);
>> + call_close |= is_wronly || is_rdwr;
>> calldata->arg.fmode &= ~FMODE_WRITE;
>> }
>> }
>> if (!nfs4_valid_open_stateid(state))
>> call_close = 0;
>> - spin_unlock(&state->owner->so_lock);
>>
>> if (!call_close) {
>> /* Note: exit _without_ calling nfs4_close_done */
>
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] NFSv4: Fix problems with close in the presence of a delegation
2014-08-26 20:15 ` Trond Myklebust
@ 2014-08-26 20:21 ` Anna Schumaker
[not found] ` <OFAD58A859.9600390A-ON88257D40.0070BE69-88257D40.0070FF36@LocalDomain>
0 siblings, 1 reply; 6+ messages in thread
From: Anna Schumaker @ 2014-08-26 20:21 UTC (permalink / raw
To: Trond Myklebust; +Cc: Linux NFS Mailing List, James Drews, jrs
On 08/26/2014 04:15 PM, Trond Myklebust wrote:
> On Tue, Aug 26, 2014 at 4:11 PM, Anna Schumaker
> <Anna.Schumaker@netapp.com> wrote:
>> On 08/26/2014 02:30 PM, Trond Myklebust wrote:
>>> In the presence of delegations, we can no longer assume that the
>>> state->n_rdwr, state->n_rdonly, state->n_wronly reflect the open
>>> stateid share mode, and so we need to need to calculate the initial
>> Nit: Remove the duplicate "need to" (above).
>>
>>> value for calldata->arg.fmode using the state->flags.
>>>
>>> Reported-by: James Drews <drews@engr.wisc.edu>
>>> Fixes: 88069f77e1ac5 (NFSv41: Fix a potential state leakage when...)
>>> Cc: stable@vger.kernel.org # 2.6.33+
>>> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
>>> ---
>>> fs/nfs/nfs4proc.c | 19 +++++++++++++------
>>> 1 file changed, 13 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>> index 75ae8d22f067..7d67d5b332d4 100644
>>> --- a/fs/nfs/nfs4proc.c
>>> +++ b/fs/nfs/nfs4proc.c
>>> @@ -2601,6 +2601,7 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
>>> struct nfs4_closedata *calldata = data;
>>> struct nfs4_state *state = calldata->state;
>>> struct inode *inode = calldata->inode;
>>> + bool is_rdonly, is_wronly, is_rdwr;
>>> int call_close = 0;
>>>
>>> dprintk("%s: begin!\n", __func__);
>>> @@ -2608,24 +2609,30 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
>>> goto out_wait;
>>>
>>> task->tk_msg.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_OPEN_DOWNGRADE];
>>> - calldata->arg.fmode = FMODE_READ|FMODE_WRITE;
>>> spin_lock(&state->owner->so_lock);
>>> + is_rdwr = test_bit(NFS_O_RDWR_STATE, &state->flags);
>>> + is_rdonly = test_bit(NFS_O_RDONLY_STATE, &state->flags);
>>> + is_wronly = test_bit(NFS_O_WRONLY_STATE, &state->flags);
>>> + spin_unlock(&state->owner->so_lock);
>>> + /* Calculate the current open share mode */
>>> + calldata->arg.fmode = 0;
>>> + if (is_rdonly || is_rdwr)
>>> + calldata->arg.fmode |= FMODE_READ;
>>> + if (is_wronly || is_rdwr)
>>> + calldata->arg.fmode |= FMODE_WRITE;
>>> /* Calculate the change in open mode */
>>> if (state->n_rdwr == 0) {
>> Do we need the owner lock for reading state->n_rdwr, n_rdonly, and n_wronly?
>
> Duh, yes, you're right. I'll revert that part of the change.
I could be wrong! I checked for other uses, and nfs4proc.c:can_open_cached() reads these values without a lock.
Anna
>
>> Anna
>>> if (state->n_rdonly == 0) {
>>> - call_close |= test_bit(NFS_O_RDONLY_STATE, &state->flags);
>>> - call_close |= test_bit(NFS_O_RDWR_STATE, &state->flags);
>>> + call_close |= is_rdonly || is_rdwr;
>>> calldata->arg.fmode &= ~FMODE_READ;
>>> }
>>> if (state->n_wronly == 0) {
>>> - call_close |= test_bit(NFS_O_WRONLY_STATE, &state->flags);
>>> - call_close |= test_bit(NFS_O_RDWR_STATE, &state->flags);
>>> + call_close |= is_wronly || is_rdwr;
>>> calldata->arg.fmode &= ~FMODE_WRITE;
>>> }
>>> }
>>> if (!nfs4_valid_open_stateid(state))
>>> call_close = 0;
>>> - spin_unlock(&state->owner->so_lock);
>>>
>>> if (!call_close) {
>>> /* Note: exit _without_ calling nfs4_close_done */
>>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: NFSv4.2 Linux client
[not found] ` <OFAD58A859.9600390A-ON88257D40.0070BE69-88257D40.0070FF36@LocalDomain>
@ 2014-08-26 22:43 ` Marc Eshel
0 siblings, 0 replies; 6+ messages in thread
From: Marc Eshel @ 2014-08-26 22:43 UTC (permalink / raw
To: Marc Eshel; +Cc: Anna Schumaker, Linux NFS Mailing List, Trond Myklebust
Hi Anna,
Any plans to update the Linux client with the latest changes to the
NFSv4.2 draft?
Marc.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-08-26 22:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-26 18:30 [PATCH v2 1/2] NFSv4: Fix problems with close in the presence of a delegation Trond Myklebust
2014-08-26 18:30 ` [PATCH v2 2/2] NFSv4: Don't clear the open state when we just did an OPEN_DOWNGRADE Trond Myklebust
2014-08-26 20:11 ` [PATCH v2 1/2] NFSv4: Fix problems with close in the presence of a delegation Anna Schumaker
2014-08-26 20:15 ` Trond Myklebust
2014-08-26 20:21 ` Anna Schumaker
[not found] ` <OFAD58A859.9600390A-ON88257D40.0070BE69-88257D40.0070FF36@LocalDomain>
2014-08-26 22:43 ` NFSv4.2 Linux client Marc Eshel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.