Re: [PATCH v3 0/4] cifs: follow-on fixes after fscache_resize_cookie() consolidation

Frank Sorenson <[email protected]> Fri, 31 Jul 2026 15:29:08 -0500
Newsgroups org.kernel.vger.linux-cifs
Message-ID <[email protected]>
Reading through the Sashiko findings for this 
https://sashiko.dev/#/patchset/20260731171231.686427-1-sorenson%40redhat.com

These are all pre-existing, and not introduced by this patchset. I have 
fixes for several of them prepared:

* cifs_setsize() calls truncate_pagecache() without i_rwsem 
from cifs_do_truncate()

   Real, pre-existing. cifs_do_truncate() has always been called from 
cifs_open() without the lock.  This would likely require significant 
work to fix.


* Local inode truncated to 0 before network open; no rollback on network 
failure

   Real, but pre-existing and unrelated; I have a patch that fixes 
exactly this.


* len underflow in cifs_remap_file_range() when off > i_size

   Real, but pre-existing, unrelated to any of these hunks.


* smb2_duplicate_extents() leaves file extended if FSCTL fails

   Real, but pre-existing; I have a patch that fixes exactly this


* pagecache_isize_extended() missing in cifs_setsize()

   Real, but pre-existing; I have a patch that fixes exactly this


* Use-after-free in cifs_file_set_size() after cifsFileInfo_put()
   This is real, but it's pre-existing and unrelated to this patchset.  
That said, I have a patch for exactly this.

Frank


On 7/31/26 12:12 PM, Frank Sorenson wrote:
> After commit fa724e235cfd ("cifs: add fscache_resize_cookie() to
> cifs_setsize()") consolidated fscache cookie resizing into cifs_setsize(),
> two further fixes are needed and two dead-code blocks became removable.
>
> Patch 1 fixes cifs_do_truncate() (O_TRUNC via cifs_open()).  That path
> runs without i_rwsem, so it cannot use the locked resize path.
> cifs_invalidate_cache() is the correct alternative: it works without the
> lock and ensures stale cached data is not served once the cookie is later
> activated.
>
> Patch 2 addresses a race identified during review: cifs_setsize() calls
> fscache_resize_cookie() without i_rwsem, but another concurrent open may
> already have the cookie active (IS_CACHING set), making fscache_resize_cookie()
> a real operation requiring the lock.  The fix strips fscache_resize_cookie()
> from cifs_setsize() and introduces cifs_resize_file_locked(), which
> temporarily activates the cookie with fscache_use_cookie(), performs the
> resize under i_rwsem, then deactivates it.  Callers that already hold
> i_rwsem (cifs_file_set_size, smb2_duplicate_extents, smb3_simple_falloc)
> switch to the wrapper; cifs_do_truncate() continues to use cifs_setsize()
> followed by cifs_invalidate_cache() as established in patch 1.
>
> Patches 3 and 4 remove dead code.  Patch 3 removes a caller-side
> truncate_setsize() + fscache_resize_cookie() block from
> cifs_remap_file_range() that became redundant once smb2_duplicate_extents()
> started performing the full size update via cifs_setsize() under the
> i_rwsem held by lock_two_nondirectories().  Patch 4 removes equivalent
> dead blocks from cifs_setattr_unix() and cifs_setattr_nounix(): since
> cifs_file_set_size() calls cifs_setsize() on success, i_size always equals
> attrs->ia_size on the success path, making the subsequent size-inequality
> blocks unreachable.
>
> v3: Fixed broken patch
> v2:
>    - Added patch 2 (cifs_resize_file_locked): based on review feedback
>      (Huiwen He) that the fscache cookie is not guaranteed to be quiescent
>      in cifs_do_truncate() - another concurrent open may already have it
>      active.  The fix strips fscache_resize_cookie() from cifs_setsize() and
>      adds a locked wrapper for callers that hold i_rwsem.
>    - Patch 1 commit message updated: the original rationale (cookie is
>      always quiescent in cifs_do_truncate) was incorrect; the correct reason
>      is that i_rwsem cannot be held in that path.
>    - Patches 2-3 from v1 renumbered to 3-4 to place fixes before cleanups.
>    - Patches 3-4: add Reviewed-by from Huiwen He; otherwise unchanged from v1.
>    - All patches: add Reviewed-by from Paulo Alcantara.
>
> Frank Sorenson (4):
>    cifs: use cifs_invalidate_cache() in cifs_do_truncate() for O_TRUNC
>    cifs: add cifs_resize_file_locked() to guard fscache_resize_cookie()
>      under i_rwsem
>    cifs: remove redundant size-update block in cifs_remap_file_range()
>    cifs: remove dead size-update blocks in cifs_setattr_unix/nounix
>
>   fs/smb/client/cifsfs.c  |  6 +-----
>   fs/smb/client/cifsfs.h  |  1 +
>   fs/smb/client/file.c    |  1 +
>   fs/smb/client/inode.c   | 41 ++++++++++++++++++++++-------------------
>   fs/smb/client/smb2ops.c |  9 +++------
>   5 files changed, 28 insertions(+), 30 deletions(-)
>