smb3: retrying on failed server close

commit 173217bd73365867378b5e75a86f0049e1069ee8 upstream.

In the current implementation, CIFS close sends a close to the
server and does not check for the success of the server close.
This patch adds functionality to check for server close return
status and retries in case of an EBUSY or EAGAIN error.

This can help avoid handle leaks

Cc: stable@vger.kernel.org
Signed-off-by: Ritvik Budhiraja <rbudhiraja@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 15e1215bc4e5ae6ee5b75f224c9f356b3c6652bd..1a9e705d6500284dac37f605a34513c21b6061ab 100644 (file)
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -401,6 +401,7 @@ smb2_close_cached_fid(struct kref *ref)
 {
        struct cached_fid *cfid = container_of(ref, struct cached_fid,
                                               refcount);
+       int rc;
 
        spin_lock(&cfid->cfids->cfid_list_lock);
        if (cfid->on_list) {
@@ -414,9 +415,10 @@ smb2_close_cached_fid(struct kref *ref)
        cfid->dentry = NULL;
 
        if (cfid->is_open) {
-               SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid,
+               rc = SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid,
                           cfid->fid.volatile_fid);
-               atomic_dec(&cfid->tcon->num_remote_opens);
+               if (rc != -EBUSY && rc != -EAGAIN)
+                       atomic_dec(&cfid->tcon->num_remote_opens);
        }
 
        free_cached_dir(cfid);

diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 2131638f26d0b4b89a89b7ea40a9064403872f8d..fcb93a66e47cbabfbdefa0b6ce59df5d563e2dc6 100644 (file)
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -159,6 +159,7 @@ struct workqueue_struct     *decrypt_wq;
 struct workqueue_struct        *fileinfo_put_wq;
 struct workqueue_struct        *cifsoplockd_wq;
 struct workqueue_struct        *deferredclose_wq;
+struct workqueue_struct        *serverclose_wq;
 __u32 cifs_lock_secret;
 
 /*
@@ -1877,6 +1878,13 @@ init_cifs(void)
                goto out_destroy_cifsoplockd_wq;
        }
 
+       serverclose_wq = alloc_workqueue("serverclose",
+                                          WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
+       if (!serverclose_wq) {
+               rc = -ENOMEM;
+               goto out_destroy_serverclose_wq;
+       }
+
        rc = cifs_init_inodecache();
        if (rc)
                goto out_destroy_deferredclose_wq;
@@ -1951,6 +1959,8 @@ out_destroy_decrypt_wq:
        destroy_workqueue(decrypt_wq);
 out_destroy_cifsiod_wq:
        destroy_workqueue(cifsiod_wq);
+out_destroy_serverclose_wq:
+       destroy_workqueue(serverclose_wq);
 out_clean_proc:
        cifs_proc_clean();
        return rc;
@@ -1980,6 +1990,7 @@ exit_cifs(void)
        destroy_workqueue(cifsoplockd_wq);
        destroy_workqueue(decrypt_wq);
        destroy_workqueue(fileinfo_put_wq);
+       destroy_workqueue(serverclose_wq);
        destroy_workqueue(cifsiod_wq);
        cifs_proc_clean();
 }

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 35a12413bbee6f8ad3aab62dfab8091fe1cecf49..b99767c8d37313bac172a01af087013b87c34b36 100644 (file)
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -425,10 +425,10 @@ struct smb_version_operations {
        /* set fid protocol-specific info */
        void (*set_fid)(struct cifsFileInfo *, struct cifs_fid *, __u32);
        /* close a file */
-       void (*close)(const unsigned int, struct cifs_tcon *,
+       int (*close)(const unsigned int, struct cifs_tcon *,
                      struct cifs_fid *);
        /* close a file, returning file attributes and timestamps */
-       void (*close_getattr)(const unsigned int xid, struct cifs_tcon *tcon,
+       int (*close_getattr)(const unsigned int xid, struct cifs_tcon *tcon,
                      struct cifsFileInfo *pfile_info);
        /* send a flush request to the server */
        int (*flush)(const unsigned int, struct cifs_tcon *, struct cifs_fid *);
@@ -1408,6 +1408,7 @@ struct cifsFileInfo {
        bool invalidHandle:1;   /* file closed via session abend */
        bool swapfile:1;
        bool oplock_break_cancelled:1;
+       bool offload:1; /* offload final part of _put to a wq */
        unsigned int oplock_epoch; /* epoch from the lease break */
        __u32 oplock_level; /* oplock/lease level from the lease break */
        int count;
@@ -1416,6 +1417,7 @@ struct cifsFileInfo {
        struct cifs_search_info srch_inf;
        struct work_struct oplock_break; /* work for oplock breaks */
        struct work_struct put; /* work for the final part of _put */
+       struct work_struct serverclose; /* work for serverclose */
        struct delayed_work deferred;
        bool deferred_close_scheduled; /* Flag to indicate close is scheduled */
        char *symlink_target;
@@ -2073,6 +2075,7 @@ extern struct workqueue_struct *decrypt_wq;
 extern struct workqueue_struct *fileinfo_put_wq;
 extern struct workqueue_struct *cifsoplockd_wq;
 extern struct workqueue_struct *deferredclose_wq;
+extern struct workqueue_struct *serverclose_wq;
 extern __u32 cifs_lock_secret;
 
 extern mempool_t *cifs_mid_poolp;

diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index 606972a95465b19c8cfbf0ab4942e06a36dc03cf..53a8c633221b946cb1c46a30d30bd6c11e212fda 100644 (file)
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -459,6 +459,7 @@ cifs_down_write(struct rw_semaphore *sem)
 }
 
 static void cifsFileInfo_put_work(struct work_struct *work);
+void serverclose_work(struct work_struct *work);
 
 struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
                                       struct tcon_link *tlink, __u32 oplock,
@@ -505,6 +506,7 @@ struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
        cfile->tlink = cifs_get_tlink(tlink);
        INIT_WORK(&cfile->oplock_break, cifs_oplock_break);
        INIT_WORK(&cfile->put, cifsFileInfo_put_work);
+       INIT_WORK(&cfile->serverclose, serverclose_work);
        INIT_DELAYED_WORK(&cfile->deferred, smb2_deferred_work_close);
        mutex_init(&cfile->fh_mutex);
        spin_lock_init(&cfile->file_info_lock);
@@ -596,6 +598,40 @@ static void cifsFileInfo_put_work(struct work_struct *work)
        cifsFileInfo_put_final(cifs_file);
 }
 
+void serverclose_work(struct work_struct *work)
+{
+       struct cifsFileInfo *cifs_file = container_of(work,
+                       struct cifsFileInfo, serverclose);
+
+       struct cifs_tcon *tcon = tlink_tcon(cifs_file->tlink);
+
+       struct TCP_Server_Info *server = tcon->ses->server;
+       int rc = 0;
+       int retries = 0;
+       int MAX_RETRIES = 4;
+
+       do {
+               if (server->ops->close_getattr)
+                       rc = server->ops->close_getattr(0, tcon, cifs_file);
+               else if (server->ops->close)
+                       rc = server->ops->close(0, tcon, &cifs_file->fid);
+
+               if (rc == -EBUSY || rc == -EAGAIN) {
+                       retries++;
+                       msleep(250);
+               }
+       } while ((rc == -EBUSY || rc == -EAGAIN) && (retries < MAX_RETRIES)
+       );
+
+       if (retries == MAX_RETRIES)
+               pr_warn("Serverclose failed %d times, giving up\n", MAX_RETRIES);
+
+       if (cifs_file->offload)
+               queue_work(fileinfo_put_wq, &cifs_file->put);
+       else
+               cifsFileInfo_put_final(cifs_file);
+}
+
 /**
  * cifsFileInfo_put - release a reference of file priv data
  *
@@ -636,10 +672,13 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file,
        struct cifs_fid fid = {};
        struct cifs_pending_open open;
        bool oplock_break_cancelled;
+       bool serverclose_offloaded = false;
 
        spin_lock(&tcon->open_file_lock);
        spin_lock(&cifsi->open_file_lock);
        spin_lock(&cifs_file->file_info_lock);
+
+       cifs_file->offload = offload;
        if (--cifs_file->count > 0) {
                spin_unlock(&cifs_file->file_info_lock);
                spin_unlock(&cifsi->open_file_lock);
@@ -681,13 +720,20 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file,
        if (!tcon->need_reconnect && !cifs_file->invalidHandle) {
                struct TCP_Server_Info *server = tcon->ses->server;
                unsigned int xid;
+               int rc = 0;
 
                xid = get_xid();
                if (server->ops->close_getattr)
-                       server->ops->close_getattr(xid, tcon, cifs_file);
+                       rc = server->ops->close_getattr(xid, tcon, cifs_file);
                else if (server->ops->close)
-                       server->ops->close(xid, tcon, &cifs_file->fid);
+                       rc = server->ops->close(xid, tcon, &cifs_file->fid);
                _free_xid(xid);
+
+               if (rc == -EBUSY || rc == -EAGAIN) {
+                       // Server close failed, hence offloading it as an async op
+                       queue_work(serverclose_wq, &cifs_file->serverclose);
+                       serverclose_offloaded = true;
+               }
        }
 
        if (oplock_break_cancelled)
@@ -695,10 +741,15 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file,
 
        cifs_del_pending_open(&open);
 
-       if (offload)
-               queue_work(fileinfo_put_wq, &cifs_file->put);
-       else
-               cifsFileInfo_put_final(cifs_file);
+       // if serverclose has been offloaded to wq (on failure), it will
+       // handle offloading put as well. If serverclose not offloaded,
+       // we need to handle offloading put here.
+       if (!serverclose_offloaded) {
+               if (offload)
+                       queue_work(fileinfo_put_wq, &cifs_file->put);
+               else
+                       cifsFileInfo_put_final(cifs_file);
+       }
 }
 
 int cifs_open(struct inode *inode, struct file *file)

diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index 64e25233e85deb2062571c28c89bda791fc2ec0b..1aebcf95c1951a66264d987c0000ca10d62df59a 100644 (file)
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -753,11 +753,11 @@ cifs_set_fid(struct cifsFileInfo *cfile, struct cifs_fid *fid, __u32 oplock)
        cinode->can_cache_brlcks = CIFS_CACHE_WRITE(cinode);
 }
 
-static void
+static int
 cifs_close_file(const unsigned int xid, struct cifs_tcon *tcon,
                struct cifs_fid *fid)
 {
-       CIFSSMBClose(xid, tcon, fid->netfid);
+       return CIFSSMBClose(xid, tcon, fid->netfid);
 }
 
 static int

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 978a9f409857aa45e023d24b23def2850a51bc68..a07b74935583aba6342c715ac72fb44f0eab9bef 100644 (file)
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -1392,14 +1392,14 @@ smb2_set_fid(struct cifsFileInfo *cfile, struct cifs_fid *fid, __u32 oplock)
        memcpy(cfile->fid.create_guid, fid->create_guid, 16);
 }
 
-static void
+static int
 smb2_close_file(const unsigned int xid, struct cifs_tcon *tcon,
                struct cifs_fid *fid)
 {
-       SMB2_close(xid, tcon, fid->persistent_fid, fid->volatile_fid);
+       return SMB2_close(xid, tcon, fid->persistent_fid, fid->volatile_fid);
 }
 
-static void
+static int
 smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon,
                   struct cifsFileInfo *cfile)
 {
@@ -1410,7 +1410,7 @@ smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon,
        rc = __SMB2_close(xid, tcon, cfile->fid.persistent_fid,
                   cfile->fid.volatile_fid, &file_inf);
        if (rc)
-               return;
+               return rc;
 
        inode = d_inode(cfile->dentry);
 
@@ -1439,6 +1439,7 @@ smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon,
 
        /* End of file and Attributes should not have to be updated on close */
        spin_unlock(&inode->i_lock);
+       return rc;
 }
 
 static int

diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 4d7d0bdf7a472b29b1dc70b916bd19957e48e624..94bd4c6d2d682f0fa19daee60868ca35d77cfde6 100644 (file)
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -3549,9 +3549,9 @@ __SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
                        memcpy(&pbuf->network_open_info,
                               &rsp->network_open_info,
                               sizeof(pbuf->network_open_info));
+               atomic_dec(&tcon->num_remote_opens);
        }
 
-       atomic_dec(&tcon->num_remote_opens);
 close_exit:
        SMB2_close_free(&rqst);
        free_rsp_buf(resp_buftype, rsp);