| Message ID | 20191121193514.3086-1-pshilov@microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] CIFS: Close open handle after interrupted close | expand |
Merged these three (and one of Aurelien's recent patches for multichannel - need updates to that, also waiting on Paulo's DFS fix) into cifs-2.6.git for-next (and also the buildbot's gitub for-next branch) On Thu, Nov 21, 2019 at 1:35 PM Pavel Shilovsky <piastryyy@gmail.com> wrote: > > If Close command is interrupted before sending a request > to the server the client ends up leaking an open file > handle. This wastes server resources and can potentially > block applications that try to remove the file or any > directory containing this file. > > Fix this by putting the close command into a worker queue, > so another thread retries it later. > > Cc: Stable <stable@vger.kernel.org> > Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> > --- > fs/cifs/smb2misc.c | 59 ++++++++++++++++++++++++++++++++++----------- > fs/cifs/smb2pdu.c | 16 +++++++++++- > fs/cifs/smb2proto.h | 3 +++ > 3 files changed, 63 insertions(+), 15 deletions(-) > > diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > index 527c9efd3de0..80a8f4b2daab 100644 > --- a/fs/cifs/smb2misc.c > +++ b/fs/cifs/smb2misc.c > @@ -725,36 +725,67 @@ smb2_cancelled_close_fid(struct work_struct *work) > kfree(cancelled); > } > > +/* Caller should already has an extra reference to @tcon */ > +static int > +__smb2_handle_cancelled_close(struct cifs_tcon *tcon, __u64 persistent_fid, > + __u64 volatile_fid) > +{ > + struct close_cancelled_open *cancelled; > + > + cancelled = kzalloc(sizeof(*cancelled), GFP_KERNEL); > + if (!cancelled) > + return -ENOMEM; > + > + cancelled->fid.persistent_fid = persistent_fid; > + cancelled->fid.volatile_fid = volatile_fid; > + cancelled->tcon = tcon; > + INIT_WORK(&cancelled->work, smb2_cancelled_close_fid); > + WARN_ON(queue_work(cifsiod_wq, &cancelled->work) == false); > + > + return 0; > +} > + > +int > +smb2_handle_cancelled_close(struct cifs_tcon *tcon, __u64 persistent_fid, > + __u64 volatile_fid) > +{ > + int rc; > + > + cifs_dbg(FYI, "%s: tc_count=%d\n", __func__, tcon->tc_count); > + spin_lock(&cifs_tcp_ses_lock); > + tcon->tc_count++; > + spin_unlock(&cifs_tcp_ses_lock); > + > + rc = __smb2_handle_cancelled_close(tcon, persistent_fid, volatile_fid); > + if (rc) > + cifs_put_tcon(tcon); > + > + return rc; > +} > + > int > smb2_handle_cancelled_mid(char *buffer, struct TCP_Server_Info *server) > { > struct smb2_sync_hdr *sync_hdr = (struct smb2_sync_hdr *)buffer; > struct smb2_create_rsp *rsp = (struct smb2_create_rsp *)buffer; > struct cifs_tcon *tcon; > - struct close_cancelled_open *cancelled; > + int rc; > > if (sync_hdr->Command != SMB2_CREATE || > sync_hdr->Status != STATUS_SUCCESS) > return 0; > > - cancelled = kzalloc(sizeof(*cancelled), GFP_KERNEL); > - if (!cancelled) > - return -ENOMEM; > - > tcon = smb2_find_smb_tcon(server, sync_hdr->SessionId, > sync_hdr->TreeId); > - if (!tcon) { > - kfree(cancelled); > + if (!tcon) > return -ENOENT; > - } > > - cancelled->fid.persistent_fid = rsp->PersistentFileId; > - cancelled->fid.volatile_fid = rsp->VolatileFileId; > - cancelled->tcon = tcon; > - INIT_WORK(&cancelled->work, smb2_cancelled_close_fid); > - queue_work(cifsiod_wq, &cancelled->work); > + rc = __smb2_handle_cancelled_close(tcon, rsp->PersistentFileId, > + rsp->VolatileFileId); > + if (rc) > + cifs_put_tcon(tcon); > > - return 0; > + return rc; > } > > /** > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > index 0aa40129dfb5..2f541e9efba1 100644 > --- a/fs/cifs/smb2pdu.c > +++ b/fs/cifs/smb2pdu.c > @@ -2942,7 +2942,21 @@ int > SMB2_close(const unsigned int xid, struct cifs_tcon *tcon, > u64 persistent_fid, u64 volatile_fid) > { > - return SMB2_close_flags(xid, tcon, persistent_fid, volatile_fid, 0); > + int rc; > + int tmp_rc; > + > + rc = SMB2_close_flags(xid, tcon, persistent_fid, volatile_fid, 0); > + > + /* retry close in a worker thread if this one is interrupted */ > + if (rc == -EINTR) { > + tmp_rc = smb2_handle_cancelled_close(tcon, persistent_fid, > + volatile_fid); > + if (tmp_rc) > + cifs_dbg(VFS, "handle cancelled close fid 0x%llx returned error %d\n", > + persistent_fid, tmp_rc); > + } > + > + return rc; > } > > int > diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h > index 07ca72486cfa..e239f98093a9 100644 > --- a/fs/cifs/smb2proto.h > +++ b/fs/cifs/smb2proto.h > @@ -203,6 +203,9 @@ extern int SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon, > extern int SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon, > const u64 persistent_fid, const u64 volatile_fid, > const __u8 oplock_level); > +extern int smb2_handle_cancelled_close(struct cifs_tcon *tcon, > + __u64 persistent_fid, > + __u64 volatile_fid); > extern int smb2_handle_cancelled_mid(char *buffer, > struct TCP_Server_Info *server); > void smb2_cancelled_close_fid(struct work_struct *work); > -- > 2.17.1 >
diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index 527c9efd3de0..80a8f4b2daab 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -725,36 +725,67 @@ smb2_cancelled_close_fid(struct work_struct *work) kfree(cancelled); } +/* Caller should already has an extra reference to @tcon */ +static int +__smb2_handle_cancelled_close(struct cifs_tcon *tcon, __u64 persistent_fid, + __u64 volatile_fid) +{ + struct close_cancelled_open *cancelled; + + cancelled = kzalloc(sizeof(*cancelled), GFP_KERNEL); + if (!cancelled) + return -ENOMEM; + + cancelled->fid.persistent_fid = persistent_fid; + cancelled->fid.volatile_fid = volatile_fid; + cancelled->tcon = tcon; + INIT_WORK(&cancelled->work, smb2_cancelled_close_fid); + WARN_ON(queue_work(cifsiod_wq, &cancelled->work) == false); + + return 0; +} + +int +smb2_handle_cancelled_close(struct cifs_tcon *tcon, __u64 persistent_fid, + __u64 volatile_fid) +{ + int rc; + + cifs_dbg(FYI, "%s: tc_count=%d\n", __func__, tcon->tc_count); + spin_lock(&cifs_tcp_ses_lock); + tcon->tc_count++; + spin_unlock(&cifs_tcp_ses_lock); + + rc = __smb2_handle_cancelled_close(tcon, persistent_fid, volatile_fid); + if (rc) + cifs_put_tcon(tcon); + + return rc; +} + int smb2_handle_cancelled_mid(char *buffer, struct TCP_Server_Info *server) { struct smb2_sync_hdr *sync_hdr = (struct smb2_sync_hdr *)buffer; struct smb2_create_rsp *rsp = (struct smb2_create_rsp *)buffer; struct cifs_tcon *tcon; - struct close_cancelled_open *cancelled; + int rc; if (sync_hdr->Command != SMB2_CREATE || sync_hdr->Status != STATUS_SUCCESS) return 0; - cancelled = kzalloc(sizeof(*cancelled), GFP_KERNEL); - if (!cancelled) - return -ENOMEM; - tcon = smb2_find_smb_tcon(server, sync_hdr->SessionId, sync_hdr->TreeId); - if (!tcon) { - kfree(cancelled); + if (!tcon) return -ENOENT; - } - cancelled->fid.persistent_fid = rsp->PersistentFileId; - cancelled->fid.volatile_fid = rsp->VolatileFileId; - cancelled->tcon = tcon; - INIT_WORK(&cancelled->work, smb2_cancelled_close_fid); - queue_work(cifsiod_wq, &cancelled->work); + rc = __smb2_handle_cancelled_close(tcon, rsp->PersistentFileId, + rsp->VolatileFileId); + if (rc) + cifs_put_tcon(tcon); - return 0; + return rc; } /** diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 0aa40129dfb5..2f541e9efba1 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -2942,7 +2942,21 @@ int SMB2_close(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, u64 volatile_fid) { - return SMB2_close_flags(xid, tcon, persistent_fid, volatile_fid, 0); + int rc; + int tmp_rc; + + rc = SMB2_close_flags(xid, tcon, persistent_fid, volatile_fid, 0); + + /* retry close in a worker thread if this one is interrupted */ + if (rc == -EINTR) { + tmp_rc = smb2_handle_cancelled_close(tcon, persistent_fid, + volatile_fid); + if (tmp_rc) + cifs_dbg(VFS, "handle cancelled close fid 0x%llx returned error %d\n", + persistent_fid, tmp_rc); + } + + return rc; } int diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h index 07ca72486cfa..e239f98093a9 100644 --- a/fs/cifs/smb2proto.h +++ b/fs/cifs/smb2proto.h @@ -203,6 +203,9 @@ extern int SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon, extern int SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon, const u64 persistent_fid, const u64 volatile_fid, const __u8 oplock_level); +extern int smb2_handle_cancelled_close(struct cifs_tcon *tcon, + __u64 persistent_fid, + __u64 volatile_fid); extern int smb2_handle_cancelled_mid(char *buffer, struct TCP_Server_Info *server); void smb2_cancelled_close_fid(struct work_struct *work);
If Close command is interrupted before sending a request to the server the client ends up leaking an open file handle. This wastes server resources and can potentially block applications that try to remove the file or any directory containing this file. Fix this by putting the close command into a worker queue, so another thread retries it later. Cc: Stable <stable@vger.kernel.org> Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> --- fs/cifs/smb2misc.c | 59 ++++++++++++++++++++++++++++++++++----------- fs/cifs/smb2pdu.c | 16 +++++++++++- fs/cifs/smb2proto.h | 3 +++ 3 files changed, 63 insertions(+), 15 deletions(-)