[PATCH 4/5] ksmbd: defer CHANGE_NOTIFY completion instead of STATUS_NOT_IMPLEMENTED
"Gaël Blivet-Bailly" <[email protected]>
| Newsgroups | org.kernel.vger.linux-cifs |
|---|---|
| Message-ID | <[email protected]> |
From: Gael Blivet <[email protected]> smb2_notify() currently returns STATUS_NOT_IMPLEMENTED synchronously for every CHANGE_NOTIFY request. Genuine SMB2 servers never complete a CHANGE_NOTIFY spontaneously -- it's satisfied only by a real directory change or with STATUS_NOTIFY_CLEANUP when the watched handle is closed. macOS smbfs.kext depends on this deferred-completion contract: receiving STATUS_NOT_IMPLEMENTED instead makes it hard-freeze on unmount, since it never sees the cleanup it's waiting for. Add a notify_pendings list on struct ksmbd_file (protected by the existing f_lock) and a notify_entry list_head on struct ksmbd_work to link onto it. smb2_notify() now replies STATUS_PENDING immediately and queues a deferred STATUS_NOTIFY_CLEANUP response on the watched handle; __ksmbd_close_fd() drains and sends any pending notifications when the handle is actually closed. The drain splices the list out under fp->f_lock first, then processes the detached copy without the lock -- smb2_notify() on another connection can be adding to the same list at the same time a close happens on this one, and ksmbd_conn_write() can sleep (it takes the connection's write mutex), so it must not be called while the spinlock is held. Also handle the FileId=FFFF...FFFF share-root sentinel that macOS backupd sends to watch for changes without holding an open handle -- without an immediate STATUS_PENDING/STATUS_NOTIFY_CLEANUP reply here, backupd aborts Time Machine setup with STATUS_FILE_CLOSED. Signed-off-by: Gael Blivet <[email protected]> --- fs/smb/server/ksmbd_work.c | 1 + fs/smb/server/ksmbd_work.h | 2 + fs/smb/server/smb2pdu.c | 108 +++++++++++++++++++++++++++++++++++-- fs/smb/server/vfs_cache.c | 30 +++++++++++ fs/smb/server/vfs_cache.h | 6 +++ 5 files changed, 144 insertions(+), 3 deletions(-) diff --git a/fs/smb/server/ksmbd_work.c b/fs/smb/server/ksmbd_work.c index e2c2f4526..97502273a 100644 --- a/fs/smb/server/ksmbd_work.c +++ b/fs/smb/server/ksmbd_work.c @@ -56,6 +56,7 @@ struct ksmbd_work *ksmbd_alloc_work_struct(void) INIT_LIST_HEAD(&work->request_entry); INIT_LIST_HEAD(&work->async_request_entry); INIT_LIST_HEAD(&work->fp_entry); + INIT_LIST_HEAD(&work->notify_entry); INIT_LIST_HEAD(&work->aux_read_list); work->iov_alloc_cnt = ARRAY_SIZE(work->iov_inline); work->iov = work->iov_inline; diff --git a/fs/smb/server/ksmbd_work.h b/fs/smb/server/ksmbd_work.h index 88104f0cf..50c4aa779 100644 --- a/fs/smb/server/ksmbd_work.h +++ b/fs/smb/server/ksmbd_work.h @@ -104,6 +104,8 @@ struct ksmbd_work { /* List head at conn->async_requests */ struct list_head async_request_entry; struct list_head fp_entry; + /* List head at ksmbd_file->notify_pendings */ + struct list_head notify_entry; }; /** diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c index b1be4df04..63beb2bcd 100644 --- a/fs/smb/server/smb2pdu.c +++ b/fs/smb/server/smb2pdu.c @@ -9966,6 +9966,9 @@ int smb2_notify(struct ksmbd_work *work) { struct smb2_change_notify_req *req; struct smb2_change_notify_rsp *rsp; + struct ksmbd_work *in_work; + struct smb2_hdr *in_hdr; + struct ksmbd_file *fp; ksmbd_debug(SMB, "Received smb2 notify\n"); @@ -9980,9 +9983,108 @@ int smb2_notify(struct ksmbd_work *work) return -EIO; } - smb2_set_err_rsp(work); - rsp->hdr.Status = STATUS_NOT_IMPLEMENTED; - return -EOPNOTSUPP; + /* + * macOS backupd sends CHANGE_NOTIFY with FileId=FFFF...FFFF (share-root + * sentinel) to watch for changes on the share root without holding an + * open handle. Respond STATUS_PENDING + STATUS_NOTIFY_CLEANUP immediately; + * without this, backupd aborts Time Machine setup on STATUS_FILE_CLOSED. + */ + if (req->VolatileFileId == SMB2_NO_FID && + req->PersistentFileId == SMB2_NO_FID) { + in_work = ksmbd_alloc_work_struct(); + if (!in_work || allocate_interim_rsp_buf(in_work)) { + if (in_work) + ksmbd_free_work_struct(in_work); + rsp->hdr.Status = STATUS_INSUFFICIENT_RESOURCES; + smb2_set_err_rsp(work); + return 0; + } + if (setup_async_work(work, NULL, NULL)) { + ksmbd_free_work_struct(in_work); + rsp->hdr.Status = STATUS_INSUFFICIENT_RESOURCES; + smb2_set_err_rsp(work); + return 0; + } + smb2_send_interim_resp(work, STATUS_PENDING); + in_work->conn = work->conn; + in_hdr = smb_get_msg(in_work->response_buf); + memcpy(in_hdr, ksmbd_resp_buf_next(work), + __SMB2_HEADER_STRUCTURE_SIZE); + in_hdr->Flags |= SMB2_FLAGS_ASYNC_COMMAND; + in_hdr->Id.AsyncId = cpu_to_le64(work->async_id); + smb2_set_err_rsp(in_work); + in_hdr->Status = STATUS_NOTIFY_CLEANUP; + in_work->async_id = work->async_id; + work->async_id = 0; + release_async_work(work); + ksmbd_conn_write(in_work); + ksmbd_free_work_struct(in_work); + work->send_no_response = 1; + return 0; + } + + /* + * KSMBD does not implement a real change-notification backend. + * Genuine SMB2 servers (and macOS smbfs) never complete a + * CHANGE_NOTIFY spontaneously: it is satisfied only by a real + * directory change, or with STATUS_NOTIFY_CLEANUP when the watched + * handle is closed. Completing it early (e.g. on a timer) makes + * Finder treat the cleanup as "directory changed" and re-enumerate + * the directory forever, leaving items unopenable. Returning + * STATUS_NOT_IMPLEMENTED here (like stock ksmbd) makes macOS smbfs + * hard-freeze on unmount, so this must stay deferred. + */ + fp = ksmbd_lookup_fd_slow(work, req->VolatileFileId, req->PersistentFileId); + if (!fp) { + rsp->hdr.Status = STATUS_FILE_CLOSED; + smb2_set_err_rsp(work); + return 0; + } + + in_work = ksmbd_alloc_work_struct(); + if (!in_work || allocate_interim_rsp_buf(in_work)) { + if (in_work) + ksmbd_free_work_struct(in_work); + ksmbd_fd_put(work, fp); + rsp->hdr.Status = STATUS_INSUFFICIENT_RESOURCES; + smb2_set_err_rsp(work); + return 0; + } + + if (setup_async_work(work, NULL, NULL)) { + ksmbd_free_work_struct(in_work); + ksmbd_fd_put(work, fp); + rsp->hdr.Status = STATUS_INSUFFICIENT_RESOURCES; + smb2_set_err_rsp(work); + return 0; + } + + smb2_send_interim_resp(work, STATUS_PENDING); + + in_work->conn = work->conn; + in_hdr = smb_get_msg(in_work->response_buf); + memcpy(in_hdr, ksmbd_resp_buf_next(work), __SMB2_HEADER_STRUCTURE_SIZE); + in_hdr->Flags |= SMB2_FLAGS_ASYNC_COMMAND; + in_hdr->Id.AsyncId = cpu_to_le64(work->async_id); + smb2_set_err_rsp(in_work); + in_hdr->Status = STATUS_NOTIFY_CLEANUP; + + /* + * Transfer ownership of the async id to in_work; it stays reserved + * until in_work is freed after the deferred response is sent on + * close, so it can't be reused for an unrelated async response. + */ + in_work->async_id = work->async_id; + work->async_id = 0; + release_async_work(work); + + spin_lock(&fp->f_lock); + list_add_tail(&in_work->notify_entry, &fp->notify_pendings); + spin_unlock(&fp->f_lock); + + ksmbd_fd_put(work, fp); + work->send_no_response = 1; + return 0; } /** diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c index 2543dd7e8..76ca47ca0 100644 --- a/fs/smb/server/vfs_cache.c +++ b/fs/smb/server/vfs_cache.c @@ -529,6 +529,7 @@ static void __ksmbd_close_fd(struct ksmbd_file_table *ft, struct ksmbd_file *fp) { struct file *filp; struct ksmbd_lock *smb_lock, *tmp_lock; + struct ksmbd_work *cn_work, *cn_tmp; fd_limit_close(); ksmbd_remove_durable_fd(fp); @@ -561,6 +562,34 @@ static void __ksmbd_close_fd(struct ksmbd_file_table *ft, struct ksmbd_file *fp) kfree(smb_lock); } + /* + * Complete any CHANGE_NOTIFY left pending on this handle now that + * it is closed. KSMBD never completes CHANGE_NOTIFY spontaneously + * (no real change-notification backend), only on close -- matching + * genuine SMB2/macOS smbfs semantics and avoiding the Finder + * "directory changed, re-enumerate everything" loop. + * + * smb2_notify() on another connection can be adding to + * notify_pendings under fp->f_lock at the same time this handle is + * closed. Splice the list out under that same lock before touching + * it, instead of walking it unlocked -- ksmbd_conn_write() can sleep + * (it takes conn's write mutex), so it must not be called while + * fp->f_lock is held. + */ + { + LIST_HEAD(cn_dispose); + + spin_lock(&fp->f_lock); + list_splice_init(&fp->notify_pendings, &cn_dispose); + spin_unlock(&fp->f_lock); + + list_for_each_entry_safe(cn_work, cn_tmp, &cn_dispose, notify_entry) { + list_del_init(&cn_work->notify_entry); + ksmbd_conn_write(cn_work); + ksmbd_free_work_struct(cn_work); + } + } + /* * Drop fp's strong reference on conn (taken in ksmbd_open_fd() / * ksmbd_reopen_durable_fd()). Durable fps that reached the @@ -1080,6 +1109,7 @@ struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work *work, struct file *filp) INIT_LIST_HEAD(&fp->blocked_works); INIT_LIST_HEAD(&fp->node); INIT_LIST_HEAD(&fp->lock_list); + INIT_LIST_HEAD(&fp->notify_pendings); spin_lock_init(&fp->f_lock); mutex_init(&fp->readdir_lock); atomic_set(&fp->refcount, 1); diff --git a/fs/smb/server/vfs_cache.h b/fs/smb/server/vfs_cache.h index 111a4e315..2d722c43c 100644 --- a/fs/smb/server/vfs_cache.h +++ b/fs/smb/server/vfs_cache.h @@ -135,6 +135,12 @@ struct ksmbd_file { bool is_posix_ctxt; struct durable_owner owner; + + /* + * Pending CHANGE_NOTIFY completions for this handle, sent with + * STATUS_NOTIFY_CLEANUP when the handle is closed. + */ + struct list_head notify_pendings; }; static inline void set_ctx_actor(struct dir_context *ctx, -- 2.43.0