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(-)
>