[PATCH v2 0/3] cifs: three size-management bug fixes

[PATCH v2 0/3] cifs: three size-management bug fixes

[Frank Sorenson]

These patches fix three independent bugs in cifs file size and handle
management.

Patch 1 fixes a use-after-free in cifs_file_set_size(): when the
handle-based set_file_size() call fails and falls through to the
path-based fallback, tcon is reused from the cifsFileInfo that was
already released by cifsFileInfo_put().  If that put drops the last
reference on a tlink that has been removed from the tlink tree, the
subsequent set_path_size() call is a use-after-free.

Patch 2 fixes a premature i_size update in cifs_do_truncate(): when
no cached writable handle is available, the server truncation happens
implicitly via the O_TRUNC flag in the following cifs_open() request,
but the current code sets i_size to 0 locally beforehand.  If the
subsequent open fails, other processes sharing the inode observe a
spuriously zero-sized file.

Patch 3 fixes a loff_t underflow in cifs_remap_file_range() when
len == 0 and off >= i_size.  The computed length is negative, which
corrupts downstream arithmetic and sends a huge ByteCount in the
FSCTL_DUPLICATE_EXTENTS_TO_FILE request.

Patches 1 and 2 fix regressions introduced by commit 110fee6b9bb5
("smb: client: fix missing timestamp updates with O_TRUNC").

v2: rebased

Frank Sorenson (3):
  cifs: clear tcon after cifsFileInfo_put() in cifs_file_set_size()
  cifs: don't update i_size in cifs_do_truncate() without a cached
    handle
  cifs: fix loff_t underflow in cifs_remap_file_range() when len == 0

 fs/smb/client/cifsfs.c |  7 ++++++-
 fs/smb/client/file.c   | 17 +++++++++++++----
 fs/smb/client/inode.c  |  1 +
 3 files changed, 20 insertions(+), 5 deletions(-)

-- 
2.55.0

[PATCH v2 1/3] cifs: clear tcon after cifsFileInfo_put() in cifs_file_set_size()

[Frank Sorenson]

When the else branch of cifs_file_set_size() finds a writable file handle
via find_writable_file(), it borrows tcon and server from the handle's
tlink, attempts the handle-based set_file_size() RPC, and then releases
the handle with cifsFileInfo_put().

If set_file_size() fails, execution falls through to the path-based
fallback, which reuses the borrowed tcon and server under the
"if (tcon == NULL)" guard.  Since tcon is not NULL at that point, the
guard is skipped.  If cifsFileInfo_put() dropped the last reference on a
tlink that was already removed from the tlink tree (TCON_LINK_IN_TREE
cleared, as happens during reconnection or session teardown),
cifs_put_tlink() will have freed tcon; the subsequent set_path_size()
call is then a use-after-free.

Setting tcon = NULL after cifsFileInfo_put() causes the existing guard
to take the cifs_sb_tlink() path, which acquires a fresh reference for
the path-based operation or fails cleanly if the session is gone.

Fixes: 110fee6b9bb5 ("smb: client: fix missing timestamp updates with O_TRUNC")
Cc: stable@vger.kernel.org
Cc: Paulo Alcantara <pc@manguebit.com>
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/inode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 0afff761aab9..5a4916259a04 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -3100,6 +3100,7 @@ int cifs_file_set_size(const unsigned int xid, struct dentry *dentry,
 							size, false);
 			cifs_dbg(FYI, "%s: set_file_size: rc = %d\n", __func__, rc);
 			cifsFileInfo_put(open_file);
+			tcon = NULL;
 		}
 	}
 
-- 
2.55.0

[PATCH v2 2/3] cifs: don't update i_size in cifs_do_truncate() without a cached handle

[Frank Sorenson]

When cifs_do_truncate() is called from cifs_open() for an O_TRUNC open
and find_writable_file() returns NULL (no cached writable handle),
cifs_file_flush() returns 0 without sending a server set_file_size
request.  The outer "if (!rc)" block then falls through unconditionally
to cifs_setsize(), setting the local inode size to 0 even though the
server has not yet been told to truncate the file.

If the subsequent network open in cifs_open() fails (e.g.
STATUS_ACCESS_DENIED from cifs_nt_open()), the caller receives an error
but the local inode remains at size 0 while the server file is still at
its original size.

When cfile is NULL, the O_TRUNC flag in the subsequent cifs_open()
request tells the server to truncate; i_size is updated from the open
response.  Move the size update inside the "if (cfile)" arm so that it
only runs when the server has been explicitly told to truncate, and call
cifs_invalidate_cache() in the no-handle case to flush stale pages
before the open is sent.

Fixes: 110fee6b9bb5 ("smb: client: fix missing timestamp updates with O_TRUNC")
Cc: stable@vger.kernel.org
Cc: Paulo Alcantara <pc@manguebit.com>
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/file.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index ac89c1ba56b1..92af1782bd73 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -1012,10 +1012,19 @@ static int cifs_do_truncate(const unsigned int xid, struct dentry *dentry)
 			server = tcon->ses->server;
 			rc = server->ops->set_file_size(xid, tcon,
 							cfile, 0, false);
-		}
-		if (!rc) {
-			netfs_resize_file(&cinode->netfs, 0, true);
-			cifs_setsize(inode, 0);
+			if (!rc) {
+				netfs_resize_file(&cinode->netfs, 0, true);
+				cifs_setsize(inode, 0);
+			}
+		} else {
+			/*
+			 * No cached handle; the server truncates as part of the
+			 * O_TRUNC open request that follows.  Invalidate stale
+			 * pages now so they are not written back after the
+			 * truncation; i_size is updated from the server's open
+			 * response.
+			 */
+			cifs_invalidate_cache(inode, 0);
 		}
 	}
 	if (cfile)
-- 
2.55.0

[PATCH v2 3/3] cifs: fix loff_t underflow in cifs_remap_file_range() when len == 0

[Frank Sorenson]

When cifs_remap_file_range() is called with len == 0 (the clone-to-EOF
semantic), it computes the effective length as:

    len = src_inode->i_size - off;

If off >= src_inode->i_size, this produces a negative loff_t that
corrupts all downstream arithmetic: filemap_write_and_wait_range() is
called with a wrapped range, and the ByteCount in the
FSCTL_DUPLICATE_EXTENTS_TO_FILE buffer receives a huge value, potentially
causing the server to attempt a multi-terabyte clone.

There is already a check for off >= src_inode->i_size at the point
where -EOPNOTSUPP is remapped to -EINVAL, but that is reached only
after the ioctl has already been sent with the corrupted length.

lock_two_nondirectories() is held at this point (i_rwsem write on both
inodes), so i_size_read() is stable.  Add an early bounds check inside
the len == 0 branch and return -EINVAL via the unlock path if off is at
or beyond EOF.

Fixes: 04b38d601239 ("vfs: pull btrfs clone API to vfs layer")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/cifsfs.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index a1dacc7d8f74..c8e2347aba5d 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -1413,8 +1413,13 @@ static loff_t cifs_remap_file_range(struct file *src_file, loff_t off,
 	 */
 	lock_two_nondirectories(target_inode, src_inode);
 
-	if (len == 0)
+	if (len == 0) {
+		if (off >= i_size_read(src_inode)) {
+			rc = -EINVAL;
+			goto unlock;
+		}
 		len = src_inode->i_size - off;
+	}
 
 	cifs_dbg(FYI, "clone range\n");
 
-- 
2.55.0

Re: [PATCH v2 0/3] cifs: three size-management bug fixes

[Frank Sorenson]

On 8/6/26 9:41 PM, Frank Sorenson wrote:
> These patches fix three independent bugs in cifs file size and handle
> management.

Sashiko brought up some serious concerns which I'll have to address.

Sorry for the noise.


Frank


> Patch 1 fixes a use-after-free in cifs_file_set_size(): when the
> handle-based set_file_size() call fails and falls through to the
> path-based fallback, tcon is reused from the cifsFileInfo that was
> already released by cifsFileInfo_put().  If that put drops the last
> reference on a tlink that has been removed from the tlink tree, the
> subsequent set_path_size() call is a use-after-free.
>
> Patch 2 fixes a premature i_size update in cifs_do_truncate(): when
> no cached writable handle is available, the server truncation happens
> implicitly via the O_TRUNC flag in the following cifs_open() request,
> but the current code sets i_size to 0 locally beforehand.  If the
> subsequent open fails, other processes sharing the inode observe a
> spuriously zero-sized file.
>
> Patch 3 fixes a loff_t underflow in cifs_remap_file_range() when
> len == 0 and off >= i_size.  The computed length is negative, which
> corrupts downstream arithmetic and sends a huge ByteCount in the
> FSCTL_DUPLICATE_EXTENTS_TO_FILE request.
>
> Patches 1 and 2 fix regressions introduced by commit 110fee6b9bb5
> ("smb: client: fix missing timestamp updates with O_TRUNC").
>
> v2: rebased
>
> Frank Sorenson (3):
>    cifs: clear tcon after cifsFileInfo_put() in cifs_file_set_size()
>    cifs: don't update i_size in cifs_do_truncate() without a cached
>      handle
>    cifs: fix loff_t underflow in cifs_remap_file_range() when len == 0
>
>   fs/smb/client/cifsfs.c |  7 ++++++-
>   fs/smb/client/file.c   | 17 +++++++++++++----
>   fs/smb/client/inode.c  |  1 +
>   3 files changed, 20 insertions(+), 5 deletions(-)
>