* [PATCH v2 1/2] cifs: serialize readdir with directory cache invalidation from lease breaks
2026-07-20 16:35 [PATCH v2 0/2] cifs: fix attribute cache corruption from concurrent directory operations Frank Sorenson
@ 2026-07-20 16:35 ` Frank Sorenson
2026-07-20 16:35 ` [PATCH v2 2/2] cifs: prevent readdir from changing file size due to stale directory metadata Frank Sorenson
2026-07-20 17:19 ` [PATCH v2 0/2] cifs: fix attribute cache corruption from concurrent directory operations Frank Sorenson
2 siblings, 0 replies; 4+ messages in thread
From: Frank Sorenson @ 2026-07-20 16:35 UTC (permalink / raw)
To: linux-cifs, pc, stfrench
When SMB2 directory lease breaks occur concurrently with readdir
(getdents) operations, the lease break handler can invalidate the
directory cache while readdir is still traversing it, corrupting the
dcache and causing subsequent stat() calls to return wrong file sizes
or EIO errors.
The race:
1. rename() completes successfully, returns to userspace
2. Userspace calls getdents64 on the directory
3. cifs_readdir() begins traversing directory cache entries
4. Server sends a lease break notification (directory was modified)
5. cifs_oplock_break() -> cifs_revalidate_mapping() -> cifs_zap_mapping()
acquires CIFS_INO_LOCK and invalidates the page cache
6. RACE: cache invalidation runs concurrently with readdir traversal
7. Subsequent stat() calls return wrong file sizes from the corrupted cache
This bug has existed since directory-level lease support was added.
The fix uses the existing CIFS_INO_LOCK bit to serialize cifs_readdir()
with cifs_revalidate_mapping(), which the lease break handler calls.
Since cifs_revalidate_mapping() already acquires CIFS_INO_LOCK before
invalidating, having cifs_readdir() hold it makes the two operations
mutually exclusive.
cifs_wait_bit_killable() is made non-static so readdir.c can use it
as the wait function for wait_on_bit_lock_action(). On lock acquisition
failure (signal), the already-allocated dentry path page is freed before
returning.
Reproducer: concurrent renames + readdir with 2 or more threads against
a Windows Server share (directory leases required; does not reproduce
against Samba or with actimeo=0).
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
fs/smb/client/cifsproto.h | 1 +
fs/smb/client/inode.c | 2 +-
fs/smb/client/readdir.c | 12 ++++++++++++
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index 00168839c123..e1f8304d323c 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -183,6 +183,7 @@ void cifs_dir_info_to_fattr(struct cifs_fattr *fattr,
int cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr,
bool from_readdir);
struct inode *cifs_iget(struct super_block *sb, struct cifs_fattr *fattr);
+int cifs_wait_bit_killable(struct wait_bit_key *key, int mode);
int cifs_get_inode_info(struct inode **inode, const char *full_path,
struct cifs_open_info_data *data,
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index deed04dd9b91..e75138f5f6bc 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -2772,7 +2772,7 @@ cifs_dentry_needs_reval(struct dentry *dentry)
* @key: currently unused
* @mode: the task state to sleep in
*/
-static int
+int
cifs_wait_bit_killable(struct wait_bit_key *key, int mode)
{
schedule();
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index ee5996e6d7d8..862ccc1a7e13 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -1064,6 +1064,17 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
void *page = alloc_dentry_path();
struct cached_fid *cfid = NULL;
struct cifs_sb_info *cifs_sb = CIFS_SB(file);
+ struct inode *inode = file_inode(file);
+ struct cifsInodeInfo *cinode = CIFS_I(inode);
+ int lock_rc;
+
+ lock_rc = wait_on_bit_lock_action(&cinode->flags, CIFS_INO_LOCK,
+ cifs_wait_bit_killable,
+ TASK_KILLABLE|TASK_FREEZABLE_UNSAFE);
+ if (lock_rc) {
+ free_dentry_path(page);
+ return lock_rc;
+ }
xid = get_xid();
@@ -1226,5 +1237,6 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
close_cached_dir(cfid);
free_dentry_path(page);
free_xid(xid);
+ clear_and_wake_up_bit(CIFS_INO_LOCK, &cinode->flags);
return rc;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v2 2/2] cifs: prevent readdir from changing file size due to stale directory metadata
2026-07-20 16:35 [PATCH v2 0/2] cifs: fix attribute cache corruption from concurrent directory operations Frank Sorenson
2026-07-20 16:35 ` [PATCH v2 1/2] cifs: serialize readdir with directory cache invalidation from lease breaks Frank Sorenson
@ 2026-07-20 16:35 ` Frank Sorenson
2026-07-20 17:19 ` [PATCH v2 0/2] cifs: fix attribute cache corruption from concurrent directory operations Frank Sorenson
2 siblings, 0 replies; 4+ messages in thread
From: Frank Sorenson @ 2026-07-20 16:35 UTC (permalink / raw)
To: linux-cifs, pc, stfrench
Windows Server's directory enumeration metadata (EndOfFile) lags behind
the actual file size immediately after a write and close. If a concurrent
readdir() runs in the window between close() returning to userspace and
stat() being called, it receives a stale size from the server's directory
listing and overwrites the correct cached i_size. A subsequent stat()
within the actimeo window then returns this incorrect value.
The actual race sequence:
1. Thread A: write N bytes to file, then close()
2. cifs_close() calls cifsFileInfo_put(), removing the handle from
openFileList -- is_inode_writable() now returns false
3. Thread B: readdir() runs, server returns stale EndOfFile=0 in the
directory enumeration response
4. is_size_safe_to_change() returns true (no writable handles, no RW
lease), so cifs_fattr_to_inode() overwrites i_size with 0
5. smb2_close_getattr() completes and stamps cifs_i->time = jiffies,
marking the (now corrupt) cache as valid
6. Thread A: stat() finds the cache valid and returns i_size=0
The existing is_size_safe_to_change() check blocks stale size updates
from readdir while an active RW lease is held, but does not cover the
window after the last writable handle is closed. This is a
cross-syscall-boundary race: kernel locking alone cannot prevent it
because the stale data arrives from the server after close() has
returned.
Fix this by tracking the time of the last writable close or truncate in
a new cifsInodeInfo->time_last_write field. When readdir attempts to
change i_size, is_size_safe_to_change() now first checks whether we are
still within acregmax jiffies of the last local write. If so, the
update is blocked regardless of whether any file handles remain open.
When the size update is blocked and the server-reported size differs from
the locally cached value, cifs_i->time is set to zero. This invalidates
the attribute cache so that the next stat() issues a fresh QUERY_INFO
to the server, which returns the authoritative size from the server's
open-file table rather than the stale directory enumeration metadata.
This is the same mechanism used by actimeo=0, which prevents the bug
entirely by bypassing the attribute cache on every stat().
Additionally, when the file has writable handles open or holds an active
RW lease, readdir is now unconditionally blocked from changing i_size
(previously it could grow i_size from readdir data in those cases).
With writable handles or an exclusive lease, the client is the
authoritative source for the file's size and readdir data is unreliable.
time_last_write is also set in the setattr truncation paths so that
truncate() followed by write()+close()+stat() is protected by the same
mechanism.
Reproducer: concurrent write+close+stat and readdir with 2 or more
threads against a Windows Server share. The bug does not reproduce
against Samba (no directory metadata lag) or with actimeo=0 (attribute
cache bypassed).
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
fs/smb/client/cifsfs.c | 1 +
fs/smb/client/cifsglob.h | 1 +
fs/smb/client/file.c | 24 +++++++++++++++++++++---
fs/smb/client/inode.c | 4 ++++
4 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 4df6ca03a8de..505b9ef1a08a 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -440,6 +440,7 @@ cifs_alloc_inode(struct super_block *sb)
return NULL;
cifs_inode->cifsAttrs = ATTR_ARCHIVE; /* 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.
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 08e94633a9c1..bf8dc8d16fae 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1566,6 +1566,7 @@ struct cifsInodeInfo {
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 local write/close */
u64 uniqueid; /* server inode number */
u64 createtime; /* creation time on server */
__u8 lease_key[SMB2_LEASE_KEY_SIZE]; /* lease key for this inode */
diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index 968740e7c9c3..84ecbca01d69 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -1452,6 +1452,9 @@ int cifs_close(struct inode *inode, struct file *file)
struct cifs_deferred_close *dclose;
struct cifs_tcon *tcon;
+ if (file->f_mode & FMODE_WRITE)
+ cinode->time_last_write = jiffies;
+
cifs_fscache_unuse_inode_cookie(inode, file->f_mode & FMODE_WRITE);
if (file->private_data != NULL) {
@@ -3225,13 +3228,21 @@ static int is_inode_writable(struct cifsInodeInfo *cifs_inode)
bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file,
bool from_readdir)
{
+ struct cifs_sb_info *cifs_sb;
+
if (!cifsInode)
return true;
+ cifs_sb = CIFS_SB(cifsInode);
+
+ if (from_readdir) {
+ if (time_before(jiffies, cifsInode->time_last_write + cifs_sb->ctx->acregmax))
+ return false;
+ }
+
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(cifsInode);
if (cifs_sb_flags(cifs_sb) & CIFS_MOUNT_DIRECT_IO) {
/* since no page cache to corrupt on directio
@@ -3239,12 +3250,18 @@ bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file,
return true;
}
+ /* Readdir data is unreliable when we have writable handles or
+ * an exclusive lease -- never allow it to change i_size. */
+ if (from_readdir)
+ return false;
+
if (i_size_read(&cifsInode->netfs.inode) < end_of_file)
return true;
return false;
- } else
- return true;
+ }
+
+ return true;
}
void cifs_oplock_break(struct work_struct *work)
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index e75138f5f6bc..026382b7d4fe 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -237,6 +237,8 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr,
if (is_size_safe_to_change(cifs_i, fattr->cf_eof, from_readdir)) {
i_size_write(inode, fattr->cf_eof);
inode->i_blocks = CIFS_INO_BLOCKS(fattr->cf_bytes);
+ } 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) {
@@ -3280,6 +3282,7 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
truncate_setsize(inode, attrs->ia_size);
netfs_resize_file(&cifsInode->netfs, attrs->ia_size, true);
fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size);
+ cifsInode->time_last_write = jiffies;
}
setattr_copy(&nop_mnt_idmap, inode, attrs);
@@ -3481,6 +3484,7 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
truncate_setsize(inode, attrs->ia_size);
netfs_resize_file(&cifsInode->netfs, attrs->ia_size, true);
fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size);
+ cifsInode->time_last_write = jiffies;
}
setattr_copy(&nop_mnt_idmap, inode, attrs);
--
2.55.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2 0/2] cifs: fix attribute cache corruption from concurrent directory operations
2026-07-20 16:35 [PATCH v2 0/2] cifs: fix attribute cache corruption from concurrent directory operations Frank Sorenson
2026-07-20 16:35 ` [PATCH v2 1/2] cifs: serialize readdir with directory cache invalidation from lease breaks Frank Sorenson
2026-07-20 16:35 ` [PATCH v2 2/2] cifs: prevent readdir from changing file size due to stale directory metadata Frank Sorenson
@ 2026-07-20 17:19 ` Frank Sorenson
2 siblings, 0 replies; 4+ messages in thread
From: Frank Sorenson @ 2026-07-20 17:19 UTC (permalink / raw)
To: linux-cifs, pc, stfrench
Sashiko doesn't appear very happy with either patch. Looks like I'll
need to rethink the approaches.
Frank
On 7/20/26 11:35 AM, Frank Sorenson wrote:
> This series fixes two distinct but related race conditions in the
> cifs/smb3 client where directory operations (lease breaks and readdir)
> can corrupt the attribute cache of recently modified files, causing
> subsequent stat() calls to return incorrect file sizes or fail with EIO.
>
> Both issues stem from the fundamental problem that directory-level
> metadata -- whether from a lease break notification or a readdir
> enumeration -- is not strictly synchronized with the authoritative state
> of open or recently closed files.
>
> Both bugs require as few as 2 concurrent threads performing directory
> and file operations simultaneously. They only reproduce against Windows
> Server (where directory lease breaks occur and directory enumeration
> metadata lags behind file state); they do not reproduce against Samba.
> The workaround for both is to mount with actimeo=0, which forces every
> stat() to query the server directly rather than trusting the cache.
>
>
> Patch 1: cifs: serialize readdir with directory cache invalidation from lease breaks
> -------------------------------------------------------------------------------------
> When a directory lease break occurs while readdir is actively traversing
> the directory cache, the lease break handler calls cifs_revalidate_mapping()
> -> cifs_zap_mapping() while holding CIFS_INO_LOCK, racing with
> cifs_readdir() which traverses the same cache without that lock. The
> resulting corruption causes subsequent stat() calls to return wrong file
> sizes or EIO errors.
>
> Fix: acquire CIFS_INO_LOCK at the start of cifs_readdir() so that lease
> break cache invalidation and readdir traversal are mutually exclusive.
>
>
> Patch 2: cifs: prevent readdir from changing file size due to stale directory metadata
> ---------------------------------------------------------------------------------------
> After writing to a file and closing it, concurrent readdir() can fetch
> stale directory metadata from the server (EndOfFile=0 for a recently
> written file) and overwrite the correct cached i_size. The race window
> is between cifsFileInfo_put() removing the handle from openFileList
> (after which is_inode_writable() returns false) and stat() being called.
> The existing is_size_safe_to_change() check only blocks this when an
> active RW lease was held -- not after the last writable handle is closed.
>
> Fix: track the time of the last writable close or truncate in a new
> cifsInodeInfo->time_last_write field. If readdir attempts to change
> i_size within acregmax jiffies of that timestamp, the update is
> suppressed. When the suppressed size differs from the cached value,
> cifs_i->time is set to zero, forcing the next stat() to issue a fresh
> QUERY_INFO RPC. QUERY_INFO returns the authoritative size from the
> server's open-file table rather than stale directory enumeration metadata,
> which is the same path taken by actimeo=0.
>
>
> Testing
> -------
> Both bugs reproduce against Windows Server 2022 with SMB 3.1.1 and
> at least 2 concurrent threads. A reproducer program exercising
> concurrent rename+readdir (bug 1) and write+close+stat with concurrent
> readdir (bug 2) was run for 400000 iterations with both patches applied
> without hitting either bug.
>
> A reproducer is available at https://github.com/fsorenson/cifs_cache_race_repro/
>
>
> v2: fix malformed patch 2
>
>
> Frank Sorenson (2):
> cifs: serialize readdir with directory cache invalidation from lease breaks
> cifs: prevent readdir from changing file size due to stale directory metadata
>
> fs/smb/client/cifsfs.c | 1 +
> fs/smb/client/cifsglob.h | 1 +
> fs/smb/client/cifsproto.h | 1 +
> fs/smb/client/file.c | 24 +++++++++++++++++++++---
> fs/smb/client/inode.c | 6 +++++-
> fs/smb/client/readdir.c | 12 ++++++++++++
> 6 files changed, 45 insertions(+), 4 deletions(-)
>
> --
> 2.55.0
^ permalink raw reply [flat|nested] 4+ messages in thread