Re: [PATCH v2 0/2] cifs: fix attribute cache corruption from concurrent directory operations

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