[PATCH OLK-6.6 1/4] [OLK-6.6] cifs: prevent readdir from changing file size due to stale directory metadata
mainline inclusion from mainline-v7.2-rc5 commit e8a8d54c2d508891c142a928fc7d298c4c8bd0dd category: bugfix bugzilla: NA Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- Windows Server's directory enumeration metadata lags behind the actual file size after a write+close or rename. A concurrent readdir() in the window between close() returning to userspace and stat() being called overwrites the correct cached i_size with the stale server value, causing stat() to return the wrong size. Once _cifsFileInfo_put() removes the last writable handle from openFileList, is_size_safe_to_change() permits readdir to overwrite i_size. smb2_close_getattr() then stamps cifs_i->time = jiffies, making the corrupt cached value appear fresh to the next stat(). The existing check (see Fixes:) only blocked stale size updates while an active RW lease was held, not after the last writable handle closes. Add cifsInodeInfo->time_last_write, written via smp_store_release() at writable close and on setattr/truncate. is_size_safe_to_change() checks is_inode_writable() first (acquiring open_file_lock), then rejects a readdir size update if time_last_write falls within acregmax jiffies. The spinlock release in _cifsFileInfo_put() forms a store-release barrier that pairs with the spin_lock() (load-acquire) in is_inode_writable(), ensuring the subsequent smp_load_acquire() on time_last_write observes any update from a concurrent close(). When a size update is rejected and the server value differs from the cached one, cifs_i->time is cleared to force a fresh QUERY_INFO on the next stat(). readdir is also blocked from changing i_size while writable handles are open or an RW lease is held, even on direct-IO mounts. For deferred close (closetimeo > 0), time_last_write is refreshed at the actual server close in smb2_deferred_work_close() and in the cifs_close_deferred_file*() drain paths invoked by lease/oplock breaks and tcon teardown, anchoring the protection window to the real close time rather than the earlier userspace close. time_last_write == 0 skips the time_before() check to avoid false positives near boot on 32-bit systems where jiffies starts close to INITIAL_JIFFIES. Does not reproduce against Samba or with actimeo=0. Fixes: e4b61f3b1c67 ("cifs: prevent updating file size from server if we have a read/write lease") Signed-off-by: Frank Sorenson <sorenson@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com> Conflicts: fs/smb/client/cifsfs.c fs/smb/client/cifsglob.h fs/smb/client/file.c fs/smb/client/inode.c fs/smb/client/misc.c [Conflicts due to context] Signed-off-by: Lu Chentao <luchentao1@huawei.com> --- fs/smb/client/cifsfs.c | 1 + fs/smb/client/cifsglob.h | 1 + fs/smb/client/file.c | 70 ++++++++++++++++++++++++++++++++++++---- fs/smb/client/inode.c | 7 +++- fs/smb/client/misc.c | 27 ++++++++++++++-- 5 files changed, 96 insertions(+), 10 deletions(-) diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c index a2887b978dc8..35863213b46c 100644 --- a/fs/smb/client/cifsfs.c +++ b/fs/smb/client/cifsfs.c @@ -399,10 +399,11 @@ cifs_alloc_inode(struct super_block *sb) cifs_inode = alloc_inode_sb(sb, cifs_inode_cachep, GFP_KERNEL); if (!cifs_inode) return NULL; cifs_inode->cifsAttrs = 0x20; /* default */ cifs_inode->time = 0; + cifs_inode->time_last_write = 0; /* * Until the file is open and we have gotten oplock info back from the * server, can not assume caching of file data or metadata. */ cifs_set_oplock_level(cifs_inode, 0); diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index fdafc6f1eb4f..cd1a60f63cbc 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -1599,10 +1599,11 @@ struct cifsInodeInfo { #define CIFS_INO_CLOSE_ON_LOCK (7) /* Not to defer the close when lock is set */ unsigned long flags; spinlock_t writers_lock; unsigned int writers; /* Number of writers on this inode */ unsigned long time; /* jiffies of last update of inode */ + unsigned long time_last_write; /* jiffies of last writable close or truncate */ u64 server_eof; /* current file size on server -- protected by i_lock */ u64 uniqueid; /* server inode number */ u64 createtime; /* creation time on server */ __u8 lease_key[SMB2_LEASE_KEY_SIZE]; /* lease key for this inode */ struct list_head deferred_closes; /* list of deferred closes */ diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index 5eeadf94c279..b3f4c0584e02 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -1217,15 +1217,25 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush) void smb2_deferred_work_close(struct work_struct *work) { struct cifsFileInfo *cfile = container_of(work, struct cifsFileInfo, deferred.work); + struct cifsInodeInfo *cinode = CIFS_I(d_inode(cfile->dentry)); - spin_lock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock); + spin_lock(&cinode->deferred_lock); cifs_del_deferred_close(cfile); cfile->deferred_close_scheduled = false; - spin_unlock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock); + spin_unlock(&cinode->deferred_lock); + /* + * Refresh time_last_write immediately before the actual server close + * so the protection window is anchored to the real close time, not + * the earlier userspace close time stored by cifs_close(). + */ + if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) { + /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ + smp_store_release(&cinode->time_last_write, jiffies); + } _cifsFileInfo_put(cfile, true, false); } static bool smb2_can_defer_close(struct inode *inode, struct cifs_deferred_close *dclose) @@ -1250,10 +1260,14 @@ int cifs_close(struct inode *inode, struct file *file) cifs_fscache_unuse_inode_cookie(inode, file->f_mode & FMODE_WRITE); if (file->private_data != NULL) { cfile = file->private_data; file->private_data = NULL; + if (file->f_mode & FMODE_WRITE) { + /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ + smp_store_release(&cinode->time_last_write, jiffies); + } dclose = kmalloc(sizeof(struct cifs_deferred_close), GFP_KERNEL); if ((cfile->status_file_deleted == false) && (smb2_can_defer_close(inode, dclose))) { if (test_and_clear_bit(CIFS_INO_MODIFIED_ATTR, &cinode->flags)) { inode_set_mtime_to_ts(inode, @@ -5004,31 +5018,75 @@ static int is_inode_writable(struct cifsInodeInfo *cifs_inode) but this is tricky to do without racing with writebehind page caching in the current Linux kernel design */ bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file, bool from_readdir) { + struct cifs_sb_info *cifs_sb; + unsigned long tlw; + if (!cifsInode) return true; + cifs_sb = CIFS_SB(cifsInode->netfs.inode.i_sb); + if (is_inode_writable(cifsInode) || ((cifsInode->oplock & CIFS_CACHE_RW_FLG) != 0 && from_readdir)) { /* This inode is open for write at least once */ - struct cifs_sb_info *cifs_sb; - cifs_sb = CIFS_SB(cifsInode->netfs.inode.i_sb); + /* + * Readdir data is unreliable when we have writable handles or + * an exclusive lease -- never allow it to change i_size, even + * on direct-IO mounts where the server's directory metadata + * can still lag behind the actual file state. + */ + if (from_readdir) + return false; + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DIRECT_IO) { /* since no page cache to corrupt on directio we can change size safely */ return true; } if (i_size_read(&cifsInode->netfs.inode) < end_of_file) return true; return false; - } else - return true; + } + + /* + * No writable handles open. Check whether we are within the attribute + * cache validity window of a recent local modification. + * + * For the close() path: cifs_close() calls smp_store_release() on + * time_last_write before _cifsFileInfo_put() removes the handle under + * open_file_lock. That spin_unlock() is a store-release that pairs + * with the spin_lock() (load-acquire) in is_inode_writable() above, + * so if is_inode_writable() returned false the smp_load_acquire() + * below is guaranteed to observe any time_last_write update from a + * concurrent close(). + * + * For the setattr/truncate paths: those callers use smp_store_release() + * directly; the smp_load_acquire() below pairs with that store. There + * is no shared lock between setattr and readdir, so this relies on + * acquire-release semantics alone. The store propagation latency on + * weakly-ordered architectures (nanoseconds) is negligible relative to + * the acregmax window (seconds) and the readdir RPC round-trip + * (milliseconds), making this a sound design choice in practice. + * + * time_last_write == 0 means the inode has never been written locally; + * skip the window check to avoid false positives near boot time when + * jiffies is still close to INITIAL_JIFFIES on 32-bit systems. + */ + if (from_readdir) { + /* Pairs with smp_store_release() at close and truncate sites. */ + tlw = smp_load_acquire(&cifsInode->time_last_write); + if (tlw && time_before(jiffies, tlw + cifs_sb->ctx->acregmax)) + return false; + } + + return true; } static int cifs_write_begin(struct file *file, struct address_space *mapping, loff_t pos, unsigned len, struct page **pagep, void **fsdata) diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c index f5b50cb65360..94b8ceff3c60 100644 --- a/fs/smb/client/inode.c +++ b/fs/smb/client/inode.c @@ -225,10 +225,12 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr, * i_blocks is not related to (i_size / i_blksize), * but instead 512 byte (2**9) size is required for * calculating num blocks. */ inode->i_blocks = (512 - 1 + fattr->cf_bytes) >> 9; + } else if (from_readdir && i_size_read(inode) != fattr->cf_eof) { + cifs_i->time = 0; } if (S_ISLNK(fattr->cf_mode) && fattr->cf_symlink_target) { kfree(cifs_i->symlink_target); cifs_i->symlink_target = fattr->cf_symlink_target; @@ -324,11 +326,10 @@ cifs_unix_basic_to_fattr(struct cifs_fattr *fattr, FILE_UNIX_BASIC_INFO *info, kuid_t uid = make_kuid(&init_user_ns, id); if (uid_valid(uid)) fattr->cf_uid = uid; } } - fattr->cf_gid = cifs_sb->ctx->linux_gid; if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_OVERR_GID)) { u64 id = le64_to_cpu(info->Gid); if (id < ((gid_t)-1)) { kgid_t gid = make_kgid(&init_user_ns, id); @@ -3129,10 +3130,12 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs) if (rc) goto out; if ((attrs->ia_valid & ATTR_SIZE) && attrs->ia_size != i_size_read(inode)) { + /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ + smp_store_release(&cifsInode->time_last_write, jiffies); truncate_setsize(inode, attrs->ia_size); fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size); } setattr_copy(&nop_mnt_idmap, inode, attrs); @@ -3330,10 +3333,12 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs) if (rc) goto cifs_setattr_exit; if ((attrs->ia_valid & ATTR_SIZE) && attrs->ia_size != i_size_read(inode)) { + /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ + smp_store_release(&cifsInode->time_last_write, jiffies); truncate_setsize(inode, attrs->ia_size); fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size); } setattr_copy(&nop_mnt_idmap, inode, attrs); diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c index 8bfb04e36ad7..cee96842d653 100644 --- a/fs/smb/client/misc.c +++ b/fs/smb/client/misc.c @@ -798,11 +798,18 @@ cifs_close_deferred_file(struct cifsInodeInfo *cifs_inode) } } spin_unlock(&cifs_inode->open_file_lock); list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) { - _cifsFileInfo_put(tmp_list->cfile, false, false); + struct cifsFileInfo *cfile = tmp_list->cfile; + + if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) { + /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ + smp_store_release(&CIFS_I(d_inode(cfile->dentry))->time_last_write, + jiffies); + } + _cifsFileInfo_put(cfile, false, false); list_del(&tmp_list->list); kfree(tmp_list); } } @@ -831,11 +838,18 @@ cifs_close_all_deferred_files(struct cifs_tcon *tcon) } } spin_unlock(&tcon->open_file_lock); list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) { - _cifsFileInfo_put(tmp_list->cfile, true, false); + struct cifsFileInfo *cfile = tmp_list->cfile; + + if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) { + /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ + smp_store_release(&CIFS_I(d_inode(cfile->dentry))->time_last_write, + jiffies); + } + _cifsFileInfo_put(cfile, true, false); list_del(&tmp_list->list); kfree(tmp_list); } } @@ -907,11 +921,18 @@ cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon, const char *path) } } spin_unlock(&tcon->open_file_lock); list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) { - _cifsFileInfo_put(tmp_list->cfile, true, false); + struct cifsFileInfo *cfile = tmp_list->cfile; + + if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) { + /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ + smp_store_release(&CIFS_I(d_inode(cfile->dentry))->time_last_write, + jiffies); + } + _cifsFileInfo_put(cfile, true, false); list_del(&tmp_list->list); kfree(tmp_list); } free_dentry_path(page); } -- 2.52.0
mainline inclusion from mainline-v7.2-rc5 commit c2f2e83e3bbc5483730fd4ee903182761f1ae50f category: bugfix bugzilla: NA CVE: CVE-2026-68312 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- In cifs_close_deferred_file(), cifs_close_all_deferred_files(), and cifs_close_deferred_file_under_dentry(), when a pending deferred close is cancelled via cancel_delayed_work(), the subsequent kmalloc_obj() to add the file to the local processing list may fail under memory pressure. The loop breaks immediately, but the cancelled work is no longer pending (it would have called _cifsFileInfo_put()), and the cfile is never added to file_head for processing. The cifsFileInfo reference and the open server handle both leak. Fix by saving the cfile that failed allocation in a local variable, breaking as before, and calling _cifsFileInfo_put() on it after releasing the lock. Any files later in the iteration are unaffected since their deferred work is still pending and will fire normally. Fixes: e3fc065682eb ("cifs: Deferred close performance improvements") Signed-off-by: Frank Sorenson <sorenson@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com> Conflicts: fs/smb/client/misc.c [Conflicts due to context] Signed-off-by: Lu Chentao <luchentao1@huawei.com> --- fs/smb/client/misc.c | 45 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c index cee96842d653..39ba5e20887e 100644 --- a/fs/smb/client/misc.c +++ b/fs/smb/client/misc.c @@ -771,11 +771,11 @@ cifs_del_deferred_close(struct cifsFileInfo *cfile) } void cifs_close_deferred_file(struct cifsInodeInfo *cifs_inode) { - struct cifsFileInfo *cfile = NULL; + struct cifsFileInfo *cfile = NULL, *failed_cfile = NULL; struct file_list *tmp_list, *tmp_next_list; struct list_head file_head; if (cifs_inode == NULL) return; @@ -788,19 +788,30 @@ cifs_close_deferred_file(struct cifsInodeInfo *cifs_inode) spin_lock(&cifs_inode->deferred_lock); cifs_del_deferred_close(cfile); spin_unlock(&cifs_inode->deferred_lock); tmp_list = kmalloc(sizeof(struct file_list), GFP_ATOMIC); - if (tmp_list == NULL) + if (tmp_list == NULL) { + failed_cfile = cfile; break; + } tmp_list->cfile = cfile; list_add_tail(&tmp_list->list, &file_head); } } } spin_unlock(&cifs_inode->open_file_lock); + if (failed_cfile) { + if (OPEN_FMODE(failed_cfile->f_flags) & FMODE_WRITE) { + /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ + smp_store_release(&CIFS_I(d_inode(failed_cfile->dentry))->time_last_write, + jiffies); + } + _cifsFileInfo_put(failed_cfile, false, false); + } + list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) { struct cifsFileInfo *cfile = tmp_list->cfile; if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) { /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ @@ -814,11 +825,11 @@ cifs_close_deferred_file(struct cifsInodeInfo *cifs_inode) } void cifs_close_all_deferred_files(struct cifs_tcon *tcon) { - struct cifsFileInfo *cfile; + struct cifsFileInfo *cfile, *failed_cfile = NULL; struct file_list *tmp_list, *tmp_next_list; struct list_head file_head; INIT_LIST_HEAD(&file_head); spin_lock(&tcon->open_file_lock); @@ -828,19 +839,30 @@ cifs_close_all_deferred_files(struct cifs_tcon *tcon) spin_lock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock); cifs_del_deferred_close(cfile); spin_unlock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock); tmp_list = kmalloc(sizeof(struct file_list), GFP_ATOMIC); - if (tmp_list == NULL) + if (tmp_list == NULL) { + failed_cfile = cfile; break; + } tmp_list->cfile = cfile; list_add_tail(&tmp_list->list, &file_head); } } } spin_unlock(&tcon->open_file_lock); + if (failed_cfile) { + if (OPEN_FMODE(failed_cfile->f_flags) & FMODE_WRITE) { + /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ + smp_store_release(&CIFS_I(d_inode(failed_cfile->dentry))->time_last_write, + jiffies); + } + _cifsFileInfo_put(failed_cfile, true, false); + } + list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) { struct cifsFileInfo *cfile = tmp_list->cfile; if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) { /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ @@ -891,11 +913,11 @@ void cifs_close_all_deferred_files_sb(struct cifs_sb_info *cifs_sb) } void cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon, const char *path) { - struct cifsFileInfo *cfile; + struct cifsFileInfo *cfile, *failed_cfile = NULL; struct file_list *tmp_list, *tmp_next_list; struct list_head file_head; void *page; const char *full_path; @@ -910,20 +932,31 @@ cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon, const char *path) spin_lock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock); cifs_del_deferred_close(cfile); spin_unlock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock); tmp_list = kmalloc(sizeof(struct file_list), GFP_ATOMIC); - if (tmp_list == NULL) + if (tmp_list == NULL) { + failed_cfile = cfile; break; + } tmp_list->cfile = cfile; list_add_tail(&tmp_list->list, &file_head); } } } } spin_unlock(&tcon->open_file_lock); + if (failed_cfile) { + if (OPEN_FMODE(failed_cfile->f_flags) & FMODE_WRITE) { + /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ + smp_store_release(&CIFS_I(d_inode(failed_cfile->dentry))->time_last_write, + jiffies); + } + _cifsFileInfo_put(failed_cfile, true, false); + } + list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) { struct cifsFileInfo *cfile = tmp_list->cfile; if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) { /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ -- 2.52.0
mainline inclusion from mainline-v7.2-rc5 commit 0e3ea5445c228048f937ad5a944c27859a78f971 category: bugfix bugzilla: NA Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- The time_last_write stamp was scattered across cifs_close(), smb2_deferred_work_close(), and the three drain functions in misc.c. This missed the case where background I/O holds the final reference after userspace close() returns, and required explicit maintenance at each close-path site. Move the smp_store_release() into _cifsFileInfo_put(), immediately before releasing open_file_lock. This single location covers all close paths unconditionally: normal close, background I/O dropping the final reference, deferred close via timer or external drain. The spinlock's store-release/load-acquire pairing with is_inode_writable() already provides the ordering guarantee documented in is_size_safe_to_change(). Remove the now-redundant stamps from cifs_close(), smb2_deferred_work_close(), and all six stamp sites in the misc.c deferred-close drain functions. Fixes: e8a8d54c2d50 ("cifs: prevent readdir from changing file size due to stale directory metadata") Signed-off-by: Frank Sorenson <sorenson@redhat.com> Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Signed-off-by: Steve French <stfrench@microsoft.com> Conflicts: fs/smb/client/file.c fs/smb/client/misc.c [Conflicts due to context] Signed-off-by: Lu Chentao <luchentao1@huawei.com> --- fs/smb/client/file.c | 37 ++++++++++++++------------------ fs/smb/client/misc.c | 51 ++++++-------------------------------------- 2 files changed, 22 insertions(+), 66 deletions(-) diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index b3f4c0584e02..0f527180fd74 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -780,10 +780,18 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file, if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO) set_bit(CIFS_INO_INVALID_MAPPING, &cifsi->flags); cifs_set_oplock_level(cifsi, 0); } + if (OPEN_FMODE(cifs_file->f_flags) & FMODE_WRITE) { + /* Stamp while open_file_lock is held; covers all close paths + * including background I/O. Pairs with smp_load_acquire() in + * is_size_safe_to_change(). + */ + smp_store_release(&cifsi->time_last_write, jiffies); + } + spin_unlock(&cifsi->open_file_lock); spin_unlock(&tcon->open_file_lock); oplock_break_cancelled = wait_oplock_handler ? cancel_work_sync(&cifs_file->oplock_break) : false; @@ -1223,19 +1231,10 @@ void smb2_deferred_work_close(struct work_struct *work) spin_lock(&cinode->deferred_lock); cifs_del_deferred_close(cfile); cfile->deferred_close_scheduled = false; spin_unlock(&cinode->deferred_lock); - /* - * Refresh time_last_write immediately before the actual server close - * so the protection window is anchored to the real close time, not - * the earlier userspace close time stored by cifs_close(). - */ - if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) { - /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ - smp_store_release(&cinode->time_last_write, jiffies); - } _cifsFileInfo_put(cfile, true, false); } static bool smb2_can_defer_close(struct inode *inode, struct cifs_deferred_close *dclose) @@ -1260,14 +1259,10 @@ int cifs_close(struct inode *inode, struct file *file) cifs_fscache_unuse_inode_cookie(inode, file->f_mode & FMODE_WRITE); if (file->private_data != NULL) { cfile = file->private_data; file->private_data = NULL; - if (file->f_mode & FMODE_WRITE) { - /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ - smp_store_release(&cinode->time_last_write, jiffies); - } dclose = kmalloc(sizeof(struct cifs_deferred_close), GFP_KERNEL); if ((cfile->status_file_deleted == false) && (smb2_can_defer_close(inode, dclose))) { if (test_and_clear_bit(CIFS_INO_MODIFIED_ATTR, &cinode->flags)) { inode_set_mtime_to_ts(inode, @@ -5055,17 +5050,17 @@ bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file, /* * No writable handles open. Check whether we are within the attribute * cache validity window of a recent local modification. * - * For the close() path: cifs_close() calls smp_store_release() on - * time_last_write before _cifsFileInfo_put() removes the handle under - * open_file_lock. That spin_unlock() is a store-release that pairs - * with the spin_lock() (load-acquire) in is_inode_writable() above, - * so if is_inode_writable() returned false the smp_load_acquire() - * below is guaranteed to observe any time_last_write update from a - * concurrent close(). + * For the close() path: _cifsFileInfo_put() stamps time_last_write + * (via smp_store_release()) before releasing open_file_lock. That + * spin_unlock() is a store-release that pairs with the spin_lock() + * (load-acquire) in is_inode_writable() above, so if + * is_inode_writable() returned false the smp_load_acquire() below is + * guaranteed to observe any time_last_write update from a concurrent + * close(), covering all close paths including background I/O. * * For the setattr/truncate paths: those callers use smp_store_release() * directly; the smp_load_acquire() below pairs with that store. There * is no shared lock between setattr and readdir, so this relies on * acquire-release semantics alone. The store propagation latency on @@ -5076,11 +5071,11 @@ bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file, * time_last_write == 0 means the inode has never been written locally; * skip the window check to avoid false positives near boot time when * jiffies is still close to INITIAL_JIFFIES on 32-bit systems. */ if (from_readdir) { - /* Pairs with smp_store_release() at close and truncate sites. */ + /* Pairs with smp_store_release() in _cifsFileInfo_put() and setattr. */ tlw = smp_load_acquire(&cifsInode->time_last_write); if (tlw && time_before(jiffies, tlw + cifs_sb->ctx->acregmax)) return false; } diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c index 39ba5e20887e..f9faaebd82da 100644 --- a/fs/smb/client/misc.c +++ b/fs/smb/client/misc.c @@ -799,28 +799,15 @@ cifs_close_deferred_file(struct cifsInodeInfo *cifs_inode) } } } spin_unlock(&cifs_inode->open_file_lock); - if (failed_cfile) { - if (OPEN_FMODE(failed_cfile->f_flags) & FMODE_WRITE) { - /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ - smp_store_release(&CIFS_I(d_inode(failed_cfile->dentry))->time_last_write, - jiffies); - } + if (failed_cfile) _cifsFileInfo_put(failed_cfile, false, false); - } list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) { - struct cifsFileInfo *cfile = tmp_list->cfile; - - if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) { - /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ - smp_store_release(&CIFS_I(d_inode(cfile->dentry))->time_last_write, - jiffies); - } - _cifsFileInfo_put(cfile, false, false); + _cifsFileInfo_put(tmp_list->cfile, false, false); list_del(&tmp_list->list); kfree(tmp_list); } } @@ -850,28 +837,15 @@ cifs_close_all_deferred_files(struct cifs_tcon *tcon) } } } spin_unlock(&tcon->open_file_lock); - if (failed_cfile) { - if (OPEN_FMODE(failed_cfile->f_flags) & FMODE_WRITE) { - /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ - smp_store_release(&CIFS_I(d_inode(failed_cfile->dentry))->time_last_write, - jiffies); - } + if (failed_cfile) _cifsFileInfo_put(failed_cfile, true, false); - } list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) { - struct cifsFileInfo *cfile = tmp_list->cfile; - - if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) { - /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ - smp_store_release(&CIFS_I(d_inode(cfile->dentry))->time_last_write, - jiffies); - } - _cifsFileInfo_put(cfile, true, false); + _cifsFileInfo_put(tmp_list->cfile, true, false); list_del(&tmp_list->list); kfree(tmp_list); } } @@ -944,28 +918,15 @@ cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon, const char *path) } } } spin_unlock(&tcon->open_file_lock); - if (failed_cfile) { - if (OPEN_FMODE(failed_cfile->f_flags) & FMODE_WRITE) { - /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ - smp_store_release(&CIFS_I(d_inode(failed_cfile->dentry))->time_last_write, - jiffies); - } + if (failed_cfile) _cifsFileInfo_put(failed_cfile, true, false); - } list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) { - struct cifsFileInfo *cfile = tmp_list->cfile; - - if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) { - /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ - smp_store_release(&CIFS_I(d_inode(cfile->dentry))->time_last_write, - jiffies); - } - _cifsFileInfo_put(cfile, true, false); + _cifsFileInfo_put(tmp_list->cfile, true, false); list_del(&tmp_list->list); kfree(tmp_list); } free_dentry_path(page); } -- 2.52.0
mainline inclusion from mainline-v7.2-rc5 commit ecababf08905958ba8c125979c4e39fc2f1a8a05 category: bugfix bugzilla: NA Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- cifs_file_set_size() calls cifs_setsize() on success, which calls i_size_write(), updating i_size to the new value. The subsequent check attrs->ia_size != i_size_read() in both cifs_setattr_unix() and cifs_setattr_nounix() therefore always evaluates false after a successful cifs_file_set_size(), making the smp_store_release() of time_last_write dead code. The truncate path was unprotected against stale readdir size updates. Move the stamp to before the cifs_file_set_size() RPC call, guarded by attrs->ia_size != i_size_read() to exclude no-op same-size ftruncate(2) calls from stamping time_last_write unnecessarily. On the error path the stamp remains rather than being restored: restoring a stale snapshot (prev_tlw) could silently erase a concurrent _cifsFileInfo_put() close stamp if that close arrived between the READ_ONCE and the smp_store_release. readdir is suppressed until the stamp expires, which extends beyond one acregmax if the caller retries failed truncations. stat() is unaffected: the cifs_revalidate_dentry_attr() path calls cifs_fattr_to_inode() with from_readdir=false, which bypasses the time_last_write check in is_size_safe_to_change() entirely and always writes the authoritative QUERY_INFO result to i_size. Remove the now-unreachable stamp from the dead block in both functions. Fixes: e8a8d54c2d50 ("cifs: prevent readdir from changing file size due to stale directory metadata") Signed-off-by: Frank Sorenson <sorenson@redhat.com> Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Signed-off-by: Steve French <stfrench@microsoft.com> Conflicts: fs/smb/client/inode.c [Conflicts due to context; adapted cifs_file_set_size() -> cifs_set_file_size()] Signed-off-by: Lu Chentao <luchentao1@huawei.com> --- fs/smb/client/inode.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c index 94b8ceff3c60..a89a249da054 100644 --- a/fs/smb/client/inode.c +++ b/fs/smb/client/inode.c @@ -3058,10 +3058,21 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs) mapping_set_error(inode->i_mapping, rc); rc = 0; if (attrs->ia_valid & ATTR_SIZE) { + if (attrs->ia_size != i_size_read(inode)) { + /* Stamp before RPC. On failure the stamp remains: restoring a + * stale snapshot could silently erase a concurrent + * _cifsFileInfo_put() close stamp. readdir is suppressed + * until the stamp expires; stat() bypasses this via the + * from_readdir=false path in is_size_safe_to_change() and + * always returns an authoritative QUERY_INFO result. + * Pairs with smp_load_acquire() in is_size_safe_to_change(). + */ + smp_store_release(&cifsInode->time_last_write, jiffies); + } rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry); if (rc != 0) goto out; } @@ -3130,12 +3141,10 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs) if (rc) goto out; if ((attrs->ia_valid & ATTR_SIZE) && attrs->ia_size != i_size_read(inode)) { - /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ - smp_store_release(&cifsInode->time_last_write, jiffies); truncate_setsize(inode, attrs->ia_size); fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size); } setattr_copy(&nop_mnt_idmap, inode, attrs); @@ -3227,10 +3236,21 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs) else rc = 0; } if (attrs->ia_valid & ATTR_SIZE) { + if (attrs->ia_size != i_size_read(inode)) { + /* Stamp before RPC. On failure the stamp remains: restoring a + * stale snapshot could silently erase a concurrent + * _cifsFileInfo_put() close stamp. readdir is suppressed + * until the stamp expires; stat() bypasses this via the + * from_readdir=false path in is_size_safe_to_change() and + * always returns an authoritative QUERY_INFO result. + * Pairs with smp_load_acquire() in is_size_safe_to_change(). + */ + smp_store_release(&cifsInode->time_last_write, jiffies); + } rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry); if (rc != 0) goto cifs_setattr_exit; } @@ -3333,12 +3353,10 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs) if (rc) goto cifs_setattr_exit; if ((attrs->ia_valid & ATTR_SIZE) && attrs->ia_size != i_size_read(inode)) { - /* Pairs with smp_load_acquire() in is_size_safe_to_change(). */ - smp_store_release(&cifsInode->time_last_write, jiffies); truncate_setsize(inode, attrs->ia_size); fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size); } setattr_copy(&nop_mnt_idmap, inode, attrs); -- 2.52.0
反馈: 您发送到kernel@openeuler.org的补丁/补丁集，转换为PR失败！ 邮件列表地址：https://mailweb.openeuler.org/archives/list/kernel@openeuler.org/message/Y6D... 失败原因：补丁集缺失封面信息 建议解决方法：请提供补丁集并重新发送您的补丁集到邮件列表 FeedBack: The patch(es) which you have sent to kernel@openeuler.org has been converted to PR failed! Mailing list address: https://mailweb.openeuler.org/archives/list/kernel@openeuler.org/message/Y6D... Failed Reason: the cover of the patches is missing Suggest Solution: please checkout and apply the patches' cover and send all again
participants (2)
-
Lu Chentao -
patchwork bot