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().
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().
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
