diff mbox series

[2/3] smb: client: do not defer close open handles to deleted files

Message ID 20240306034353.190039-2-meetakshisetiyaoss@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/3] smb: client: reuse file lease key in compound operations | expand

Commit Message

Meetakshi Setiya March 6, 2024, 3:43 a.m. UTC
From: Meetakshi Setiya <msetiya@microsoft.com>

When a file/dentry has been deleted before closing all its open
handles, currently, closing them can add them to the deferred
close list. This can lead to problems in creating file with the
same name when the file is re-created before the deferred close
completes. This issue was seen while reusing a client's already
existing lease on a file for compound operations and xfstest 591
failed because of the deferred close handle that remained valid
even after the file was deleted and was being reused to create a
file with the same name. The server in this case returns an error
on open with STATUS_DELETE_PENDING. Recreating the file would
fail till the deferred handles are closed (duration specified in
closetimeo).

This patch fixes the issue by flagging all open handles for the
deleted file (file path to be precise) by setting
status_file_deleted to true in the cifsFileInfo structure. As per
the information classes specified in MS-FSCC, SMB2 query info
response from the server has a DeletePending field, set to true
to indicate that deletion has been requested on that file. If
this is the case, flag the open handles for this file too.

When doing close in cifs_close for each of these handles, check the
value of this boolean field and do not defer close these handles
if the corresponding filepath has been deleted.

Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com>
---
 fs/smb/client/cifsglob.h  |  1 +
 fs/smb/client/cifsproto.h |  4 ++++
 fs/smb/client/file.c      |  3 ++-
 fs/smb/client/inode.c     | 28 +++++++++++++++++++++++++---
 fs/smb/client/misc.c      | 34 ++++++++++++++++++++++++++++++++++
 fs/smb/client/smb2inode.c |  9 ++++++++-
 6 files changed, 74 insertions(+), 5 deletions(-)

Comments

Meetakshi Setiya March 6, 2024, 4:06 a.m. UTC | #1
Updated version of the patch
https://lore.kernel.org/all/20240209131552.471765-1-meetakshisetiyaoss@gmail.com/
Includes perf improvement by iterating over cinode's openFileList instead
of tcon openFileList. Also, compare the deleted path with the path of each
open file in the cinode only when the file has hardlinks. Else, all open
handles are from the same file only.

Thanks
Meetakshi

On Wed, Mar 6, 2024 at 9:14 AM <meetakshisetiyaoss@gmail.com> wrote:
>
> From: Meetakshi Setiya <msetiya@microsoft.com>
>
> When a file/dentry has been deleted before closing all its open
> handles, currently, closing them can add them to the deferred
> close list. This can lead to problems in creating file with the
> same name when the file is re-created before the deferred close
> completes. This issue was seen while reusing a client's already
> existing lease on a file for compound operations and xfstest 591
> failed because of the deferred close handle that remained valid
> even after the file was deleted and was being reused to create a
> file with the same name. The server in this case returns an error
> on open with STATUS_DELETE_PENDING. Recreating the file would
> fail till the deferred handles are closed (duration specified in
> closetimeo).
>
> This patch fixes the issue by flagging all open handles for the
> deleted file (file path to be precise) by setting
> status_file_deleted to true in the cifsFileInfo structure. As per
> the information classes specified in MS-FSCC, SMB2 query info
> response from the server has a DeletePending field, set to true
> to indicate that deletion has been requested on that file. If
> this is the case, flag the open handles for this file too.
>
> When doing close in cifs_close for each of these handles, check the
> value of this boolean field and do not defer close these handles
> if the corresponding filepath has been deleted.
>
> Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com>
> ---
>  fs/smb/client/cifsglob.h  |  1 +
>  fs/smb/client/cifsproto.h |  4 ++++
>  fs/smb/client/file.c      |  3 ++-
>  fs/smb/client/inode.c     | 28 +++++++++++++++++++++++++---
>  fs/smb/client/misc.c      | 34 ++++++++++++++++++++++++++++++++++
>  fs/smb/client/smb2inode.c |  9 ++++++++-
>  6 files changed, 74 insertions(+), 5 deletions(-)
>
> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> index 50f7e47c2229..a88c8328b29c 100644
> --- a/fs/smb/client/cifsglob.h
> +++ b/fs/smb/client/cifsglob.h
> @@ -1417,6 +1417,7 @@ struct cifsFileInfo {
>         bool invalidHandle:1;   /* file closed via session abend */
>         bool swapfile:1;
>         bool oplock_break_cancelled:1;
> +       bool status_file_deleted:1; /* file has been deleted */
>         unsigned int oplock_epoch; /* epoch from the lease break */
>         __u32 oplock_level; /* oplock/lease level from the lease break */
>         int count;
> diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
> index ef98c840791f..1f46e0db6e92 100644
> --- a/fs/smb/client/cifsproto.h
> +++ b/fs/smb/client/cifsproto.h
> @@ -296,6 +296,10 @@ extern void cifs_close_all_deferred_files(struct cifs_tcon *cifs_tcon);
>
>  extern void cifs_close_deferred_file_under_dentry(struct cifs_tcon *cifs_tcon,
>                                 const char *path);
> +
> +extern void cifs_mark_open_handles_for_deleted_file(struct inode *inode,
> +                               const char *path);
> +
>  extern struct TCP_Server_Info *
>  cifs_get_tcp_session(struct smb3_fs_context *ctx,
>                      struct TCP_Server_Info *primary_server);
> diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> index b75282c204da..46951f403d31 100644
> --- a/fs/smb/client/file.c
> +++ b/fs/smb/client/file.c
> @@ -483,6 +483,7 @@ struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
>         cfile->uid = current_fsuid();
>         cfile->dentry = dget(dentry);
>         cfile->f_flags = file->f_flags;
> +       cfile->status_file_deleted = false;
>         cfile->invalidHandle = false;
>         cfile->deferred_close_scheduled = false;
>         cfile->tlink = cifs_get_tlink(tlink);
> @@ -1085,7 +1086,7 @@ int cifs_close(struct inode *inode, struct file *file)
>                 if ((cifs_sb->ctx->closetimeo && cinode->oplock == CIFS_CACHE_RHW_FLG)
>                     && cinode->lease_granted &&
>                     !test_bit(CIFS_INO_CLOSE_ON_LOCK, &cinode->flags) &&
> -                   dclose) {
> +                   dclose && !(cfile->status_file_deleted)) {
>                         if (test_and_clear_bit(CIFS_INO_MODIFIED_ATTR, &cinode->flags)) {
>                                 inode_set_mtime_to_ts(inode,
>                                                       inode_set_ctime_current(inode));
> diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> index 3073eac989ea..3242e3b74386 100644
> --- a/fs/smb/client/inode.c
> +++ b/fs/smb/client/inode.c
> @@ -893,6 +893,9 @@ cifs_get_file_info(struct file *filp)
>         struct cifsFileInfo *cfile = filp->private_data;
>         struct cifs_tcon *tcon = tlink_tcon(cfile->tlink);
>         struct TCP_Server_Info *server = tcon->ses->server;
> +       struct dentry *dentry = filp->f_path.dentry;
> +       void *page = alloc_dentry_path();
> +       const unsigned char *path;
>
>         if (!server->ops->query_file_info)
>                 return -ENOSYS;
> @@ -907,7 +910,14 @@ cifs_get_file_info(struct file *filp)
>                         data.symlink = true;
>                         data.reparse.tag = IO_REPARSE_TAG_SYMLINK;
>                 }
> +               path = build_path_from_dentry(dentry, page);
> +               if (IS_ERR(path)) {
> +                       free_dentry_path(page);
> +                       return PTR_ERR(path);
> +               }
>                 cifs_open_info_to_fattr(&fattr, &data, inode->i_sb);
> +               if (fattr.cf_flags & CIFS_FATTR_DELETE_PENDING)
> +                       cifs_mark_open_handles_for_deleted_file(inode, path);
>                 break;
>         case -EREMOTE:
>                 cifs_create_junction_fattr(&fattr, inode->i_sb);
> @@ -937,6 +947,7 @@ cifs_get_file_info(struct file *filp)
>         rc = cifs_fattr_to_inode(inode, &fattr);
>  cgfi_exit:
>         cifs_free_open_info(&data);
> +       free_dentry_path(page);
>         free_xid(xid);
>         return rc;
>  }
> @@ -1075,6 +1086,7 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
>         struct kvec rsp_iov, *iov = NULL;
>         int rsp_buftype = CIFS_NO_BUFFER;
>         u32 tag = data->reparse.tag;
> +       struct inode *inode = NULL;
>         int rc = 0;
>
>         if (!tag && server->ops->query_reparse_point) {
> @@ -1114,8 +1126,12 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
>
>         if (tcon->posix_extensions)
>                 smb311_posix_info_to_fattr(fattr, data, sb);
> -       else
> +       else {
>                 cifs_open_info_to_fattr(fattr, data, sb);
> +               inode = cifs_iget(sb, fattr);
> +               if (inode && fattr->cf_flags & CIFS_FATTR_DELETE_PENDING)
> +                       cifs_mark_open_handles_for_deleted_file(inode, full_path);
> +       }
>  out:
>         fattr->cf_cifstag = data->reparse.tag;
>         free_rsp_buf(rsp_buftype, rsp_iov.iov_base);
> @@ -1170,6 +1186,8 @@ static int cifs_get_fattr(struct cifs_open_info_data *data,
>                                                    full_path, fattr);
>                 } else {
>                         cifs_open_info_to_fattr(fattr, data, sb);
> +                       if (fattr->cf_flags & CIFS_FATTR_DELETE_PENDING)
> +                               cifs_mark_open_handles_for_deleted_file(*inode, full_path);
>                 }
>                 break;
>         case -EREMOTE:
> @@ -1850,16 +1868,20 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
>
>  psx_del_no_retry:
>         if (!rc) {
> -               if (inode)
> +               if (inode) {
> +                       cifs_mark_open_handles_for_deleted_file(inode, full_path);
>                         cifs_drop_nlink(inode);
> +               }
>         } else if (rc == -ENOENT) {
>                 d_drop(dentry);
>         } else if (rc == -EBUSY) {
>                 if (server->ops->rename_pending_delete) {
>                         rc = server->ops->rename_pending_delete(full_path,
>                                                                 dentry, xid);
> -                       if (rc == 0)
> +                       if (rc == 0) {
> +                               cifs_mark_open_handles_for_deleted_file(inode, full_path);
>                                 cifs_drop_nlink(inode);
> +                       }
>                 }
>         } else if ((rc == -EACCES) && (dosattr == 0) && inode) {
>                 attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
> diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
> index 0748d7b757b9..9428a0db7718 100644
> --- a/fs/smb/client/misc.c
> +++ b/fs/smb/client/misc.c
> @@ -853,6 +853,40 @@ cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon, const char *path)
>         free_dentry_path(page);
>  }
>
> +/*
> + * If a dentry has been deleted, all corresponding open handles should know that
> + * so that we do not defer close them.
> + */
> +void cifs_mark_open_handles_for_deleted_file(struct inode *inode,
> +                                            const char *path)
> +{
> +       struct cifsFileInfo *cfile;
> +       void *page;
> +       const char *full_path;
> +       struct cifsInodeInfo *cinode = CIFS_I(inode);
> +
> +       page = alloc_dentry_path();
> +       spin_lock(&cinode->open_file_lock);
> +
> +       /*
> +        * note: we need to construct path from dentry and compare only if the
> +        * inode has any hardlinks. When number of hardlinks is 1, we can just
> +        * mark all open handles since they are going to be from the same file.
> +        */
> +       if (inode->i_nlink > 1) {
> +               list_for_each_entry(cfile, &cinode->openFileList, flist) {
> +                       full_path = build_path_from_dentry(cfile->dentry, page);
> +                       if (!IS_ERR(full_path) && strcmp(full_path, path) == 0)
> +                               cfile->status_file_deleted = true;
> +               }
> +       } else {
> +               list_for_each_entry(cfile, &cinode->openFileList, flist)
> +                       cfile->status_file_deleted = true;
> +       }
> +       spin_unlock(&cinode->open_file_lock);
> +       free_dentry_path(page);
> +}
> +
>  /* parses DFS referral V3 structure
>   * caller is responsible for freeing target_nodes
>   * returns:
> diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> index 69f3442c5b96..429d83d31280 100644
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -561,8 +561,15 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
>                 case SMB2_OP_DELETE:
>                         if (rc)
>                                 trace_smb3_delete_err(xid,  ses->Suid, tcon->tid, rc);
> -                       else
> +                       else {
> +                               /*
> +                                * If dentry (hence, inode) is NULL, lease break is going to
> +                                * take care of degrading leases on handles for deleted files.
> +                                */
> +                               if (inode)
> +                                       cifs_mark_open_handles_for_deleted_file(inode, full_path);
>                                 trace_smb3_delete_done(xid, ses->Suid, tcon->tid);
> +                       }
>                         break;
>                 case SMB2_OP_MKDIR:
>                         if (rc)
> --
> 2.39.2
>
Steve French March 6, 2024, 7:09 a.m. UTC | #2
Merged the three patches into cifs-2.6.git pending testing but I had
to rebase this one.  I also had to rebase one of Paulo's patches "smb:
client: move most of reparse point handling code to common file"

If you see anything wrong with the minor rebase let me know.

On Tue, Mar 5, 2024 at 9:44 PM <meetakshisetiyaoss@gmail.com> wrote:
>
> From: Meetakshi Setiya <msetiya@microsoft.com>
>
> When a file/dentry has been deleted before closing all its open
> handles, currently, closing them can add them to the deferred
> close list. This can lead to problems in creating file with the
> same name when the file is re-created before the deferred close
> completes. This issue was seen while reusing a client's already
> existing lease on a file for compound operations and xfstest 591
> failed because of the deferred close handle that remained valid
> even after the file was deleted and was being reused to create a
> file with the same name. The server in this case returns an error
> on open with STATUS_DELETE_PENDING. Recreating the file would
> fail till the deferred handles are closed (duration specified in
> closetimeo).
>
> This patch fixes the issue by flagging all open handles for the
> deleted file (file path to be precise) by setting
> status_file_deleted to true in the cifsFileInfo structure. As per
> the information classes specified in MS-FSCC, SMB2 query info
> response from the server has a DeletePending field, set to true
> to indicate that deletion has been requested on that file. If
> this is the case, flag the open handles for this file too.
>
> When doing close in cifs_close for each of these handles, check the
> value of this boolean field and do not defer close these handles
> if the corresponding filepath has been deleted.
>
> Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com>
> ---
>  fs/smb/client/cifsglob.h  |  1 +
>  fs/smb/client/cifsproto.h |  4 ++++
>  fs/smb/client/file.c      |  3 ++-
>  fs/smb/client/inode.c     | 28 +++++++++++++++++++++++++---
>  fs/smb/client/misc.c      | 34 ++++++++++++++++++++++++++++++++++
>  fs/smb/client/smb2inode.c |  9 ++++++++-
>  6 files changed, 74 insertions(+), 5 deletions(-)
>
> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> index 50f7e47c2229..a88c8328b29c 100644
> --- a/fs/smb/client/cifsglob.h
> +++ b/fs/smb/client/cifsglob.h
> @@ -1417,6 +1417,7 @@ struct cifsFileInfo {
>         bool invalidHandle:1;   /* file closed via session abend */
>         bool swapfile:1;
>         bool oplock_break_cancelled:1;
> +       bool status_file_deleted:1; /* file has been deleted */
>         unsigned int oplock_epoch; /* epoch from the lease break */
>         __u32 oplock_level; /* oplock/lease level from the lease break */
>         int count;
> diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
> index ef98c840791f..1f46e0db6e92 100644
> --- a/fs/smb/client/cifsproto.h
> +++ b/fs/smb/client/cifsproto.h
> @@ -296,6 +296,10 @@ extern void cifs_close_all_deferred_files(struct cifs_tcon *cifs_tcon);
>
>  extern void cifs_close_deferred_file_under_dentry(struct cifs_tcon *cifs_tcon,
>                                 const char *path);
> +
> +extern void cifs_mark_open_handles_for_deleted_file(struct inode *inode,
> +                               const char *path);
> +
>  extern struct TCP_Server_Info *
>  cifs_get_tcp_session(struct smb3_fs_context *ctx,
>                      struct TCP_Server_Info *primary_server);
> diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> index b75282c204da..46951f403d31 100644
> --- a/fs/smb/client/file.c
> +++ b/fs/smb/client/file.c
> @@ -483,6 +483,7 @@ struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
>         cfile->uid = current_fsuid();
>         cfile->dentry = dget(dentry);
>         cfile->f_flags = file->f_flags;
> +       cfile->status_file_deleted = false;
>         cfile->invalidHandle = false;
>         cfile->deferred_close_scheduled = false;
>         cfile->tlink = cifs_get_tlink(tlink);
> @@ -1085,7 +1086,7 @@ int cifs_close(struct inode *inode, struct file *file)
>                 if ((cifs_sb->ctx->closetimeo && cinode->oplock == CIFS_CACHE_RHW_FLG)
>                     && cinode->lease_granted &&
>                     !test_bit(CIFS_INO_CLOSE_ON_LOCK, &cinode->flags) &&
> -                   dclose) {
> +                   dclose && !(cfile->status_file_deleted)) {
>                         if (test_and_clear_bit(CIFS_INO_MODIFIED_ATTR, &cinode->flags)) {
>                                 inode_set_mtime_to_ts(inode,
>                                                       inode_set_ctime_current(inode));
> diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> index 3073eac989ea..3242e3b74386 100644
> --- a/fs/smb/client/inode.c
> +++ b/fs/smb/client/inode.c
> @@ -893,6 +893,9 @@ cifs_get_file_info(struct file *filp)
>         struct cifsFileInfo *cfile = filp->private_data;
>         struct cifs_tcon *tcon = tlink_tcon(cfile->tlink);
>         struct TCP_Server_Info *server = tcon->ses->server;
> +       struct dentry *dentry = filp->f_path.dentry;
> +       void *page = alloc_dentry_path();
> +       const unsigned char *path;
>
>         if (!server->ops->query_file_info)
>                 return -ENOSYS;
> @@ -907,7 +910,14 @@ cifs_get_file_info(struct file *filp)
>                         data.symlink = true;
>                         data.reparse.tag = IO_REPARSE_TAG_SYMLINK;
>                 }
> +               path = build_path_from_dentry(dentry, page);
> +               if (IS_ERR(path)) {
> +                       free_dentry_path(page);
> +                       return PTR_ERR(path);
> +               }
>                 cifs_open_info_to_fattr(&fattr, &data, inode->i_sb);
> +               if (fattr.cf_flags & CIFS_FATTR_DELETE_PENDING)
> +                       cifs_mark_open_handles_for_deleted_file(inode, path);
>                 break;
>         case -EREMOTE:
>                 cifs_create_junction_fattr(&fattr, inode->i_sb);
> @@ -937,6 +947,7 @@ cifs_get_file_info(struct file *filp)
>         rc = cifs_fattr_to_inode(inode, &fattr);
>  cgfi_exit:
>         cifs_free_open_info(&data);
> +       free_dentry_path(page);
>         free_xid(xid);
>         return rc;
>  }
> @@ -1075,6 +1086,7 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
>         struct kvec rsp_iov, *iov = NULL;
>         int rsp_buftype = CIFS_NO_BUFFER;
>         u32 tag = data->reparse.tag;
> +       struct inode *inode = NULL;
>         int rc = 0;
>
>         if (!tag && server->ops->query_reparse_point) {
> @@ -1114,8 +1126,12 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
>
>         if (tcon->posix_extensions)
>                 smb311_posix_info_to_fattr(fattr, data, sb);
> -       else
> +       else {
>                 cifs_open_info_to_fattr(fattr, data, sb);
> +               inode = cifs_iget(sb, fattr);
> +               if (inode && fattr->cf_flags & CIFS_FATTR_DELETE_PENDING)
> +                       cifs_mark_open_handles_for_deleted_file(inode, full_path);
> +       }
>  out:
>         fattr->cf_cifstag = data->reparse.tag;
>         free_rsp_buf(rsp_buftype, rsp_iov.iov_base);
> @@ -1170,6 +1186,8 @@ static int cifs_get_fattr(struct cifs_open_info_data *data,
>                                                    full_path, fattr);
>                 } else {
>                         cifs_open_info_to_fattr(fattr, data, sb);
> +                       if (fattr->cf_flags & CIFS_FATTR_DELETE_PENDING)
> +                               cifs_mark_open_handles_for_deleted_file(*inode, full_path);
>                 }
>                 break;
>         case -EREMOTE:
> @@ -1850,16 +1868,20 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
>
>  psx_del_no_retry:
>         if (!rc) {
> -               if (inode)
> +               if (inode) {
> +                       cifs_mark_open_handles_for_deleted_file(inode, full_path);
>                         cifs_drop_nlink(inode);
> +               }
>         } else if (rc == -ENOENT) {
>                 d_drop(dentry);
>         } else if (rc == -EBUSY) {
>                 if (server->ops->rename_pending_delete) {
>                         rc = server->ops->rename_pending_delete(full_path,
>                                                                 dentry, xid);
> -                       if (rc == 0)
> +                       if (rc == 0) {
> +                               cifs_mark_open_handles_for_deleted_file(inode, full_path);
>                                 cifs_drop_nlink(inode);
> +                       }
>                 }
>         } else if ((rc == -EACCES) && (dosattr == 0) && inode) {
>                 attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
> diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
> index 0748d7b757b9..9428a0db7718 100644
> --- a/fs/smb/client/misc.c
> +++ b/fs/smb/client/misc.c
> @@ -853,6 +853,40 @@ cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon, const char *path)
>         free_dentry_path(page);
>  }
>
> +/*
> + * If a dentry has been deleted, all corresponding open handles should know that
> + * so that we do not defer close them.
> + */
> +void cifs_mark_open_handles_for_deleted_file(struct inode *inode,
> +                                            const char *path)
> +{
> +       struct cifsFileInfo *cfile;
> +       void *page;
> +       const char *full_path;
> +       struct cifsInodeInfo *cinode = CIFS_I(inode);
> +
> +       page = alloc_dentry_path();
> +       spin_lock(&cinode->open_file_lock);
> +
> +       /*
> +        * note: we need to construct path from dentry and compare only if the
> +        * inode has any hardlinks. When number of hardlinks is 1, we can just
> +        * mark all open handles since they are going to be from the same file.
> +        */
> +       if (inode->i_nlink > 1) {
> +               list_for_each_entry(cfile, &cinode->openFileList, flist) {
> +                       full_path = build_path_from_dentry(cfile->dentry, page);
> +                       if (!IS_ERR(full_path) && strcmp(full_path, path) == 0)
> +                               cfile->status_file_deleted = true;
> +               }
> +       } else {
> +               list_for_each_entry(cfile, &cinode->openFileList, flist)
> +                       cfile->status_file_deleted = true;
> +       }
> +       spin_unlock(&cinode->open_file_lock);
> +       free_dentry_path(page);
> +}
> +
>  /* parses DFS referral V3 structure
>   * caller is responsible for freeing target_nodes
>   * returns:
> diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> index 69f3442c5b96..429d83d31280 100644
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -561,8 +561,15 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
>                 case SMB2_OP_DELETE:
>                         if (rc)
>                                 trace_smb3_delete_err(xid,  ses->Suid, tcon->tid, rc);
> -                       else
> +                       else {
> +                               /*
> +                                * If dentry (hence, inode) is NULL, lease break is going to
> +                                * take care of degrading leases on handles for deleted files.
> +                                */
> +                               if (inode)
> +                                       cifs_mark_open_handles_for_deleted_file(inode, full_path);
>                                 trace_smb3_delete_done(xid, ses->Suid, tcon->tid);
> +                       }
>                         break;
>                 case SMB2_OP_MKDIR:
>                         if (rc)
> --
> 2.39.2
>
>
Meetakshi Setiya March 6, 2024, 7:35 a.m. UTC | #3
Just checked, looks fine to me.
(apologies for the previous mail)

On Wed, Mar 6, 2024 at 12:39 PM Steve French <smfrench@gmail.com> wrote:
>
> Merged the three patches into cifs-2.6.git pending testing but I had
> to rebase this one.  I also had to rebase one of Paulo's patches "smb:
> client: move most of reparse point handling code to common file"
>
> If you see anything wrong with the minor rebase let me know.
>
> On Tue, Mar 5, 2024 at 9:44 PM <meetakshisetiyaoss@gmail.com> wrote:
> >
> > From: Meetakshi Setiya <msetiya@microsoft.com>
> >
> > When a file/dentry has been deleted before closing all its open
> > handles, currently, closing them can add them to the deferred
> > close list. This can lead to problems in creating file with the
> > same name when the file is re-created before the deferred close
> > completes. This issue was seen while reusing a client's already
> > existing lease on a file for compound operations and xfstest 591
> > failed because of the deferred close handle that remained valid
> > even after the file was deleted and was being reused to create a
> > file with the same name. The server in this case returns an error
> > on open with STATUS_DELETE_PENDING. Recreating the file would
> > fail till the deferred handles are closed (duration specified in
> > closetimeo).
> >
> > This patch fixes the issue by flagging all open handles for the
> > deleted file (file path to be precise) by setting
> > status_file_deleted to true in the cifsFileInfo structure. As per
> > the information classes specified in MS-FSCC, SMB2 query info
> > response from the server has a DeletePending field, set to true
> > to indicate that deletion has been requested on that file. If
> > this is the case, flag the open handles for this file too.
> >
> > When doing close in cifs_close for each of these handles, check the
> > value of this boolean field and do not defer close these handles
> > if the corresponding filepath has been deleted.
> >
> > Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com>
> > ---
> >  fs/smb/client/cifsglob.h  |  1 +
> >  fs/smb/client/cifsproto.h |  4 ++++
> >  fs/smb/client/file.c      |  3 ++-
> >  fs/smb/client/inode.c     | 28 +++++++++++++++++++++++++---
> >  fs/smb/client/misc.c      | 34 ++++++++++++++++++++++++++++++++++
> >  fs/smb/client/smb2inode.c |  9 ++++++++-
> >  6 files changed, 74 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> > index 50f7e47c2229..a88c8328b29c 100644
> > --- a/fs/smb/client/cifsglob.h
> > +++ b/fs/smb/client/cifsglob.h
> > @@ -1417,6 +1417,7 @@ struct cifsFileInfo {
> >         bool invalidHandle:1;   /* file closed via session abend */
> >         bool swapfile:1;
> >         bool oplock_break_cancelled:1;
> > +       bool status_file_deleted:1; /* file has been deleted */
> >         unsigned int oplock_epoch; /* epoch from the lease break */
> >         __u32 oplock_level; /* oplock/lease level from the lease break */
> >         int count;
> > diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
> > index ef98c840791f..1f46e0db6e92 100644
> > --- a/fs/smb/client/cifsproto.h
> > +++ b/fs/smb/client/cifsproto.h
> > @@ -296,6 +296,10 @@ extern void cifs_close_all_deferred_files(struct cifs_tcon *cifs_tcon);
> >
> >  extern void cifs_close_deferred_file_under_dentry(struct cifs_tcon *cifs_tcon,
> >                                 const char *path);
> > +
> > +extern void cifs_mark_open_handles_for_deleted_file(struct inode *inode,
> > +                               const char *path);
> > +
> >  extern struct TCP_Server_Info *
> >  cifs_get_tcp_session(struct smb3_fs_context *ctx,
> >                      struct TCP_Server_Info *primary_server);
> > diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> > index b75282c204da..46951f403d31 100644
> > --- a/fs/smb/client/file.c
> > +++ b/fs/smb/client/file.c
> > @@ -483,6 +483,7 @@ struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
> >         cfile->uid = current_fsuid();
> >         cfile->dentry = dget(dentry);
> >         cfile->f_flags = file->f_flags;
> > +       cfile->status_file_deleted = false;
> >         cfile->invalidHandle = false;
> >         cfile->deferred_close_scheduled = false;
> >         cfile->tlink = cifs_get_tlink(tlink);
> > @@ -1085,7 +1086,7 @@ int cifs_close(struct inode *inode, struct file *file)
> >                 if ((cifs_sb->ctx->closetimeo && cinode->oplock == CIFS_CACHE_RHW_FLG)
> >                     && cinode->lease_granted &&
> >                     !test_bit(CIFS_INO_CLOSE_ON_LOCK, &cinode->flags) &&
> > -                   dclose) {
> > +                   dclose && !(cfile->status_file_deleted)) {
> >                         if (test_and_clear_bit(CIFS_INO_MODIFIED_ATTR, &cinode->flags)) {
> >                                 inode_set_mtime_to_ts(inode,
> >                                                       inode_set_ctime_current(inode));
> > diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> > index 3073eac989ea..3242e3b74386 100644
> > --- a/fs/smb/client/inode.c
> > +++ b/fs/smb/client/inode.c
> > @@ -893,6 +893,9 @@ cifs_get_file_info(struct file *filp)
> >         struct cifsFileInfo *cfile = filp->private_data;
> >         struct cifs_tcon *tcon = tlink_tcon(cfile->tlink);
> >         struct TCP_Server_Info *server = tcon->ses->server;
> > +       struct dentry *dentry = filp->f_path.dentry;
> > +       void *page = alloc_dentry_path();
> > +       const unsigned char *path;
> >
> >         if (!server->ops->query_file_info)
> >                 return -ENOSYS;
> > @@ -907,7 +910,14 @@ cifs_get_file_info(struct file *filp)
> >                         data.symlink = true;
> >                         data.reparse.tag = IO_REPARSE_TAG_SYMLINK;
> >                 }
> > +               path = build_path_from_dentry(dentry, page);
> > +               if (IS_ERR(path)) {
> > +                       free_dentry_path(page);
> > +                       return PTR_ERR(path);
> > +               }
> >                 cifs_open_info_to_fattr(&fattr, &data, inode->i_sb);
> > +               if (fattr.cf_flags & CIFS_FATTR_DELETE_PENDING)
> > +                       cifs_mark_open_handles_for_deleted_file(inode, path);
> >                 break;
> >         case -EREMOTE:
> >                 cifs_create_junction_fattr(&fattr, inode->i_sb);
> > @@ -937,6 +947,7 @@ cifs_get_file_info(struct file *filp)
> >         rc = cifs_fattr_to_inode(inode, &fattr);
> >  cgfi_exit:
> >         cifs_free_open_info(&data);
> > +       free_dentry_path(page);
> >         free_xid(xid);
> >         return rc;
> >  }
> > @@ -1075,6 +1086,7 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
> >         struct kvec rsp_iov, *iov = NULL;
> >         int rsp_buftype = CIFS_NO_BUFFER;
> >         u32 tag = data->reparse.tag;
> > +       struct inode *inode = NULL;
> >         int rc = 0;
> >
> >         if (!tag && server->ops->query_reparse_point) {
> > @@ -1114,8 +1126,12 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
> >
> >         if (tcon->posix_extensions)
> >                 smb311_posix_info_to_fattr(fattr, data, sb);
> > -       else
> > +       else {
> >                 cifs_open_info_to_fattr(fattr, data, sb);
> > +               inode = cifs_iget(sb, fattr);
> > +               if (inode && fattr->cf_flags & CIFS_FATTR_DELETE_PENDING)
> > +                       cifs_mark_open_handles_for_deleted_file(inode, full_path);
> > +       }
> >  out:
> >         fattr->cf_cifstag = data->reparse.tag;
> >         free_rsp_buf(rsp_buftype, rsp_iov.iov_base);
> > @@ -1170,6 +1186,8 @@ static int cifs_get_fattr(struct cifs_open_info_data *data,
> >                                                    full_path, fattr);
> >                 } else {
> >                         cifs_open_info_to_fattr(fattr, data, sb);
> > +                       if (fattr->cf_flags & CIFS_FATTR_DELETE_PENDING)
> > +                               cifs_mark_open_handles_for_deleted_file(*inode, full_path);
> >                 }
> >                 break;
> >         case -EREMOTE:
> > @@ -1850,16 +1868,20 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
> >
> >  psx_del_no_retry:
> >         if (!rc) {
> > -               if (inode)
> > +               if (inode) {
> > +                       cifs_mark_open_handles_for_deleted_file(inode, full_path);
> >                         cifs_drop_nlink(inode);
> > +               }
> >         } else if (rc == -ENOENT) {
> >                 d_drop(dentry);
> >         } else if (rc == -EBUSY) {
> >                 if (server->ops->rename_pending_delete) {
> >                         rc = server->ops->rename_pending_delete(full_path,
> >                                                                 dentry, xid);
> > -                       if (rc == 0)
> > +                       if (rc == 0) {
> > +                               cifs_mark_open_handles_for_deleted_file(inode, full_path);
> >                                 cifs_drop_nlink(inode);
> > +                       }
> >                 }
> >         } else if ((rc == -EACCES) && (dosattr == 0) && inode) {
> >                 attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
> > diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
> > index 0748d7b757b9..9428a0db7718 100644
> > --- a/fs/smb/client/misc.c
> > +++ b/fs/smb/client/misc.c
> > @@ -853,6 +853,40 @@ cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon, const char *path)
> >         free_dentry_path(page);
> >  }
> >
> > +/*
> > + * If a dentry has been deleted, all corresponding open handles should know that
> > + * so that we do not defer close them.
> > + */
> > +void cifs_mark_open_handles_for_deleted_file(struct inode *inode,
> > +                                            const char *path)
> > +{
> > +       struct cifsFileInfo *cfile;
> > +       void *page;
> > +       const char *full_path;
> > +       struct cifsInodeInfo *cinode = CIFS_I(inode);
> > +
> > +       page = alloc_dentry_path();
> > +       spin_lock(&cinode->open_file_lock);
> > +
> > +       /*
> > +        * note: we need to construct path from dentry and compare only if the
> > +        * inode has any hardlinks. When number of hardlinks is 1, we can just
> > +        * mark all open handles since they are going to be from the same file.
> > +        */
> > +       if (inode->i_nlink > 1) {
> > +               list_for_each_entry(cfile, &cinode->openFileList, flist) {
> > +                       full_path = build_path_from_dentry(cfile->dentry, page);
> > +                       if (!IS_ERR(full_path) && strcmp(full_path, path) == 0)
> > +                               cfile->status_file_deleted = true;
> > +               }
> > +       } else {
> > +               list_for_each_entry(cfile, &cinode->openFileList, flist)
> > +                       cfile->status_file_deleted = true;
> > +       }
> > +       spin_unlock(&cinode->open_file_lock);
> > +       free_dentry_path(page);
> > +}
> > +
> >  /* parses DFS referral V3 structure
> >   * caller is responsible for freeing target_nodes
> >   * returns:
> > diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> > index 69f3442c5b96..429d83d31280 100644
> > --- a/fs/smb/client/smb2inode.c
> > +++ b/fs/smb/client/smb2inode.c
> > @@ -561,8 +561,15 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> >                 case SMB2_OP_DELETE:
> >                         if (rc)
> >                                 trace_smb3_delete_err(xid,  ses->Suid, tcon->tid, rc);
> > -                       else
> > +                       else {
> > +                               /*
> > +                                * If dentry (hence, inode) is NULL, lease break is going to
> > +                                * take care of degrading leases on handles for deleted files.
> > +                                */
> > +                               if (inode)
> > +                                       cifs_mark_open_handles_for_deleted_file(inode, full_path);
> >                                 trace_smb3_delete_done(xid, ses->Suid, tcon->tid);
> > +                       }
> >                         break;
> >                 case SMB2_OP_MKDIR:
> >                         if (rc)
> > --
> > 2.39.2
> >
> >
>
>
> --
> Thanks,
>
> Steve
Paulo Alcantara March 6, 2024, 2:31 p.m. UTC | #4
meetakshisetiyaoss@gmail.com writes:

> diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> index b75282c204da..46951f403d31 100644
> --- a/fs/smb/client/file.c
> +++ b/fs/smb/client/file.c
> @@ -483,6 +483,7 @@ struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
>  	cfile->uid = current_fsuid();
>  	cfile->dentry = dget(dentry);
>  	cfile->f_flags = file->f_flags;
> +	cfile->status_file_deleted = false;

This is unnecessary as @cfile is kzalloc()'d.

>  	cfile->invalidHandle = false;
>  	cfile->deferred_close_scheduled = false;
>  	cfile->tlink = cifs_get_tlink(tlink);
> @@ -1085,7 +1086,7 @@ int cifs_close(struct inode *inode, struct file *file)
>  		if ((cifs_sb->ctx->closetimeo && cinode->oplock == CIFS_CACHE_RHW_FLG)
>  		    && cinode->lease_granted &&
>  		    !test_bit(CIFS_INO_CLOSE_ON_LOCK, &cinode->flags) &&
> -		    dclose) {
> +		    dclose && !(cfile->status_file_deleted)) {
>  			if (test_and_clear_bit(CIFS_INO_MODIFIED_ATTR, &cinode->flags)) {
>  				inode_set_mtime_to_ts(inode,
>  						      inode_set_ctime_current(inode));
> diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> index 3073eac989ea..3242e3b74386 100644
> --- a/fs/smb/client/inode.c
> +++ b/fs/smb/client/inode.c
> @@ -893,6 +893,9 @@ cifs_get_file_info(struct file *filp)
>  	struct cifsFileInfo *cfile = filp->private_data;
>  	struct cifs_tcon *tcon = tlink_tcon(cfile->tlink);
>  	struct TCP_Server_Info *server = tcon->ses->server;
> +	struct dentry *dentry = filp->f_path.dentry;
> +	void *page = alloc_dentry_path();
> +	const unsigned char *path;
>  
>  	if (!server->ops->query_file_info)
>  		return -ENOSYS;

You're leaking @page if above condition is true.

> @@ -907,7 +910,14 @@ cifs_get_file_info(struct file *filp)
>  			data.symlink = true;
>  			data.reparse.tag = IO_REPARSE_TAG_SYMLINK;
>  		}
> +		path = build_path_from_dentry(dentry, page);
> +		if (IS_ERR(path)) {
> +			free_dentry_path(page);
> +			return PTR_ERR(path);

Now you're leaking @data and @fid if above condition is true.

> +		}
>  		cifs_open_info_to_fattr(&fattr, &data, inode->i_sb);
> +		if (fattr.cf_flags & CIFS_FATTR_DELETE_PENDING)
> +			cifs_mark_open_handles_for_deleted_file(inode, path);
>  		break;
>  	case -EREMOTE:
>  		cifs_create_junction_fattr(&fattr, inode->i_sb);
> @@ -937,6 +947,7 @@ cifs_get_file_info(struct file *filp)
>  	rc = cifs_fattr_to_inode(inode, &fattr);
>  cgfi_exit:
>  	cifs_free_open_info(&data);
> +	free_dentry_path(page);
>  	free_xid(xid);
>  	return rc;
>  }
> @@ -1075,6 +1086,7 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
>  	struct kvec rsp_iov, *iov = NULL;
>  	int rsp_buftype = CIFS_NO_BUFFER;
>  	u32 tag = data->reparse.tag;
> +	struct inode *inode = NULL;
>  	int rc = 0;
>  
>  	if (!tag && server->ops->query_reparse_point) {
> @@ -1114,8 +1126,12 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
>  
>  	if (tcon->posix_extensions)
>  		smb311_posix_info_to_fattr(fattr, data, sb);
> -	else
> +	else {
>  		cifs_open_info_to_fattr(fattr, data, sb);
> +		inode = cifs_iget(sb, fattr);
> +		if (inode && fattr->cf_flags & CIFS_FATTR_DELETE_PENDING)

You shouldn't ignore if cifs_iget() failed.  Return -ENOMEM instead.

Besides, calling cifs_iget() here looks wrong as @fattr is not fully
set, yet.  Why can't you use @inode from cifs_get_fattr() or do above
check right after cifs_get_fattr() in cifs_get_inode_info{,_unix}()?
Meetakshi Setiya March 13, 2024, 4:17 a.m. UTC | #5
Hi

PFA the follow up patch after the above revision.

Thanks
Meetakshi

On Wed, Mar 6, 2024 at 8:01 PM Paulo Alcantara <pc@manguebit.com> wrote:
>
> meetakshisetiyaoss@gmail.com writes:
>
> > diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> > index b75282c204da..46951f403d31 100644
> > --- a/fs/smb/client/file.c
> > +++ b/fs/smb/client/file.c
> > @@ -483,6 +483,7 @@ struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
> >       cfile->uid = current_fsuid();
> >       cfile->dentry = dget(dentry);
> >       cfile->f_flags = file->f_flags;
> > +     cfile->status_file_deleted = false;
>
> This is unnecessary as @cfile is kzalloc()'d.
>
> >       cfile->invalidHandle = false;
> >       cfile->deferred_close_scheduled = false;
> >       cfile->tlink = cifs_get_tlink(tlink);
> > @@ -1085,7 +1086,7 @@ int cifs_close(struct inode *inode, struct file *file)
> >               if ((cifs_sb->ctx->closetimeo && cinode->oplock == CIFS_CACHE_RHW_FLG)
> >                   && cinode->lease_granted &&
> >                   !test_bit(CIFS_INO_CLOSE_ON_LOCK, &cinode->flags) &&
> > -                 dclose) {
> > +                 dclose && !(cfile->status_file_deleted)) {
> >                       if (test_and_clear_bit(CIFS_INO_MODIFIED_ATTR, &cinode->flags)) {
> >                               inode_set_mtime_to_ts(inode,
> >                                                     inode_set_ctime_current(inode));
> > diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> > index 3073eac989ea..3242e3b74386 100644
> > --- a/fs/smb/client/inode.c
> > +++ b/fs/smb/client/inode.c
> > @@ -893,6 +893,9 @@ cifs_get_file_info(struct file *filp)
> >       struct cifsFileInfo *cfile = filp->private_data;
> >       struct cifs_tcon *tcon = tlink_tcon(cfile->tlink);
> >       struct TCP_Server_Info *server = tcon->ses->server;
> > +     struct dentry *dentry = filp->f_path.dentry;
> > +     void *page = alloc_dentry_path();
> > +     const unsigned char *path;
> >
> >       if (!server->ops->query_file_info)
> >               return -ENOSYS;
>
> You're leaking @page if above condition is true.
>
> > @@ -907,7 +910,14 @@ cifs_get_file_info(struct file *filp)
> >                       data.symlink = true;
> >                       data.reparse.tag = IO_REPARSE_TAG_SYMLINK;
> >               }
> > +             path = build_path_from_dentry(dentry, page);
> > +             if (IS_ERR(path)) {
> > +                     free_dentry_path(page);
> > +                     return PTR_ERR(path);
>
> Now you're leaking @data and @fid if above condition is true.
>
> > +             }
> >               cifs_open_info_to_fattr(&fattr, &data, inode->i_sb);
> > +             if (fattr.cf_flags & CIFS_FATTR_DELETE_PENDING)
> > +                     cifs_mark_open_handles_for_deleted_file(inode, path);
> >               break;
> >       case -EREMOTE:
> >               cifs_create_junction_fattr(&fattr, inode->i_sb);
> > @@ -937,6 +947,7 @@ cifs_get_file_info(struct file *filp)
> >       rc = cifs_fattr_to_inode(inode, &fattr);
> >  cgfi_exit:
> >       cifs_free_open_info(&data);
> > +     free_dentry_path(page);
> >       free_xid(xid);
> >       return rc;
> >  }
> > @@ -1075,6 +1086,7 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
> >       struct kvec rsp_iov, *iov = NULL;
> >       int rsp_buftype = CIFS_NO_BUFFER;
> >       u32 tag = data->reparse.tag;
> > +     struct inode *inode = NULL;
> >       int rc = 0;
> >
> >       if (!tag && server->ops->query_reparse_point) {
> > @@ -1114,8 +1126,12 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
> >
> >       if (tcon->posix_extensions)
> >               smb311_posix_info_to_fattr(fattr, data, sb);
> > -     else
> > +     else {
> >               cifs_open_info_to_fattr(fattr, data, sb);
> > +             inode = cifs_iget(sb, fattr);
> > +             if (inode && fattr->cf_flags & CIFS_FATTR_DELETE_PENDING)
>
> You shouldn't ignore if cifs_iget() failed.  Return -ENOMEM instead.
>
> Besides, calling cifs_iget() here looks wrong as @fattr is not fully
> set, yet.  Why can't you use @inode from cifs_get_fattr() or do above
> check right after cifs_get_fattr() in cifs_get_inode_info{,_unix}()?
diff mbox series

Patch

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 50f7e47c2229..a88c8328b29c 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1417,6 +1417,7 @@  struct cifsFileInfo {
 	bool invalidHandle:1;	/* file closed via session abend */
 	bool swapfile:1;
 	bool oplock_break_cancelled:1;
+	bool status_file_deleted:1; /* file has been deleted */
 	unsigned int oplock_epoch; /* epoch from the lease break */
 	__u32 oplock_level; /* oplock/lease level from the lease break */
 	int count;
diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index ef98c840791f..1f46e0db6e92 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -296,6 +296,10 @@  extern void cifs_close_all_deferred_files(struct cifs_tcon *cifs_tcon);
 
 extern void cifs_close_deferred_file_under_dentry(struct cifs_tcon *cifs_tcon,
 				const char *path);
+
+extern void cifs_mark_open_handles_for_deleted_file(struct inode *inode,
+				const char *path);
+
 extern struct TCP_Server_Info *
 cifs_get_tcp_session(struct smb3_fs_context *ctx,
 		     struct TCP_Server_Info *primary_server);
diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index b75282c204da..46951f403d31 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -483,6 +483,7 @@  struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
 	cfile->uid = current_fsuid();
 	cfile->dentry = dget(dentry);
 	cfile->f_flags = file->f_flags;
+	cfile->status_file_deleted = false;
 	cfile->invalidHandle = false;
 	cfile->deferred_close_scheduled = false;
 	cfile->tlink = cifs_get_tlink(tlink);
@@ -1085,7 +1086,7 @@  int cifs_close(struct inode *inode, struct file *file)
 		if ((cifs_sb->ctx->closetimeo && cinode->oplock == CIFS_CACHE_RHW_FLG)
 		    && cinode->lease_granted &&
 		    !test_bit(CIFS_INO_CLOSE_ON_LOCK, &cinode->flags) &&
-		    dclose) {
+		    dclose && !(cfile->status_file_deleted)) {
 			if (test_and_clear_bit(CIFS_INO_MODIFIED_ATTR, &cinode->flags)) {
 				inode_set_mtime_to_ts(inode,
 						      inode_set_ctime_current(inode));
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 3073eac989ea..3242e3b74386 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -893,6 +893,9 @@  cifs_get_file_info(struct file *filp)
 	struct cifsFileInfo *cfile = filp->private_data;
 	struct cifs_tcon *tcon = tlink_tcon(cfile->tlink);
 	struct TCP_Server_Info *server = tcon->ses->server;
+	struct dentry *dentry = filp->f_path.dentry;
+	void *page = alloc_dentry_path();
+	const unsigned char *path;
 
 	if (!server->ops->query_file_info)
 		return -ENOSYS;
@@ -907,7 +910,14 @@  cifs_get_file_info(struct file *filp)
 			data.symlink = true;
 			data.reparse.tag = IO_REPARSE_TAG_SYMLINK;
 		}
+		path = build_path_from_dentry(dentry, page);
+		if (IS_ERR(path)) {
+			free_dentry_path(page);
+			return PTR_ERR(path);
+		}
 		cifs_open_info_to_fattr(&fattr, &data, inode->i_sb);
+		if (fattr.cf_flags & CIFS_FATTR_DELETE_PENDING)
+			cifs_mark_open_handles_for_deleted_file(inode, path);
 		break;
 	case -EREMOTE:
 		cifs_create_junction_fattr(&fattr, inode->i_sb);
@@ -937,6 +947,7 @@  cifs_get_file_info(struct file *filp)
 	rc = cifs_fattr_to_inode(inode, &fattr);
 cgfi_exit:
 	cifs_free_open_info(&data);
+	free_dentry_path(page);
 	free_xid(xid);
 	return rc;
 }
@@ -1075,6 +1086,7 @@  static int reparse_info_to_fattr(struct cifs_open_info_data *data,
 	struct kvec rsp_iov, *iov = NULL;
 	int rsp_buftype = CIFS_NO_BUFFER;
 	u32 tag = data->reparse.tag;
+	struct inode *inode = NULL;
 	int rc = 0;
 
 	if (!tag && server->ops->query_reparse_point) {
@@ -1114,8 +1126,12 @@  static int reparse_info_to_fattr(struct cifs_open_info_data *data,
 
 	if (tcon->posix_extensions)
 		smb311_posix_info_to_fattr(fattr, data, sb);
-	else
+	else {
 		cifs_open_info_to_fattr(fattr, data, sb);
+		inode = cifs_iget(sb, fattr);
+		if (inode && fattr->cf_flags & CIFS_FATTR_DELETE_PENDING)
+			cifs_mark_open_handles_for_deleted_file(inode, full_path);
+	}
 out:
 	fattr->cf_cifstag = data->reparse.tag;
 	free_rsp_buf(rsp_buftype, rsp_iov.iov_base);
@@ -1170,6 +1186,8 @@  static int cifs_get_fattr(struct cifs_open_info_data *data,
 						   full_path, fattr);
 		} else {
 			cifs_open_info_to_fattr(fattr, data, sb);
+			if (fattr->cf_flags & CIFS_FATTR_DELETE_PENDING)
+				cifs_mark_open_handles_for_deleted_file(*inode, full_path);
 		}
 		break;
 	case -EREMOTE:
@@ -1850,16 +1868,20 @@  int cifs_unlink(struct inode *dir, struct dentry *dentry)
 
 psx_del_no_retry:
 	if (!rc) {
-		if (inode)
+		if (inode) {
+			cifs_mark_open_handles_for_deleted_file(inode, full_path);
 			cifs_drop_nlink(inode);
+		}
 	} else if (rc == -ENOENT) {
 		d_drop(dentry);
 	} else if (rc == -EBUSY) {
 		if (server->ops->rename_pending_delete) {
 			rc = server->ops->rename_pending_delete(full_path,
 								dentry, xid);
-			if (rc == 0)
+			if (rc == 0) {
+				cifs_mark_open_handles_for_deleted_file(inode, full_path);
 				cifs_drop_nlink(inode);
+			}
 		}
 	} else if ((rc == -EACCES) && (dosattr == 0) && inode) {
 		attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
index 0748d7b757b9..9428a0db7718 100644
--- a/fs/smb/client/misc.c
+++ b/fs/smb/client/misc.c
@@ -853,6 +853,40 @@  cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon, const char *path)
 	free_dentry_path(page);
 }
 
+/*
+ * If a dentry has been deleted, all corresponding open handles should know that
+ * so that we do not defer close them.
+ */
+void cifs_mark_open_handles_for_deleted_file(struct inode *inode,
+					     const char *path)
+{
+	struct cifsFileInfo *cfile;
+	void *page;
+	const char *full_path;
+	struct cifsInodeInfo *cinode = CIFS_I(inode);
+
+	page = alloc_dentry_path();
+	spin_lock(&cinode->open_file_lock);
+
+	/*
+	 * note: we need to construct path from dentry and compare only if the
+	 * inode has any hardlinks. When number of hardlinks is 1, we can just
+	 * mark all open handles since they are going to be from the same file.
+	 */
+	if (inode->i_nlink > 1) {
+		list_for_each_entry(cfile, &cinode->openFileList, flist) {
+			full_path = build_path_from_dentry(cfile->dentry, page);
+			if (!IS_ERR(full_path) && strcmp(full_path, path) == 0)
+				cfile->status_file_deleted = true;
+		}
+	} else {
+		list_for_each_entry(cfile, &cinode->openFileList, flist)
+			cfile->status_file_deleted = true;
+	}
+	spin_unlock(&cinode->open_file_lock);
+	free_dentry_path(page);
+}
+
 /* parses DFS referral V3 structure
  * caller is responsible for freeing target_nodes
  * returns:
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 69f3442c5b96..429d83d31280 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -561,8 +561,15 @@  static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 		case SMB2_OP_DELETE:
 			if (rc)
 				trace_smb3_delete_err(xid,  ses->Suid, tcon->tid, rc);
-			else
+			else {
+				/*
+				 * If dentry (hence, inode) is NULL, lease break is going to
+				 * take care of degrading leases on handles for deleted files.
+				 */
+				if (inode)
+					cifs_mark_open_handles_for_deleted_file(inode, full_path);
 				trace_smb3_delete_done(xid, ses->Suid, tcon->tid);
+			}
 			break;
 		case SMB2_OP_MKDIR:
 			if (rc)