On 4/30/21 5:42 AM, Trond Myklebust wrote:
On Fri, 2021-04-30 at 01:09 -0400, Dai Ngo wrote:
Currently can_open_cached accesses the openstate's flags without the
so_lock and also does not update the flags of the cached state. This
results in the openstate's flags be out of sync which can cause the
file to be closed prematurely.
This patch adds the missing so_lock around the call to
can_open_cached
and also updates the openstate's flags if the cached openstate is
used.
Signed-off-by: Dai Ngo <dai.ngo@xxxxxxxxxx>
---
fs/nfs/nfs4proc.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index c65c4b41e2c1..2464e77c51f9 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2410,9 +2410,15 @@ static void nfs4_open_prepare(struct rpc_task
*task, void *calldata)
if (data->state != NULL) {
struct nfs_delegation *delegation;
+ spin_lock(&data->state->owner->so_lock);
if (can_open_cached(data->state, data->o_arg.fmode,
- data->o_arg.open_flags,
claim))
+ data->o_arg.open_flags, claim)) {
+ update_open_stateflags(data->state, data-
o_arg.fmode);
+ spin_unlock(&data->state->owner->so_lock);
goto out_no_action;
+ }
+ spin_unlock(&data->state->owner->so_lock);
+
rcu_read_lock();
delegation = nfs4_get_valid_delegation(data->state-
inode);
if (can_open_delegated(delegation, data->o_arg.fmode,
claim))
This is going to introduce stateid leaks. The actual update of the open
state flags happens in nfs4_try_open_cached(), which is called from
nfs4_opendata_to_nfs4_state().
Right, the actual update is done by _nfs4_opendata_to_nfs4_state called
from _nfs4_do_open/_nfs4_open_and_get_state. I missed the check of
data->cancelled in nfs4_open_release and just keying in on rpc_done not
set path which skips the call to nfs4_opendata_to_nfs4_state.
Thanks Trond!
-Dai
While we could put spinlocks around the call to can_open_cached() here,
there is little point in doing so, since this is just a read-only
advisory check. The real check is performed, as I said, in
nfs4_try_open_cached().
