Re: [PATCH] cifs: fix i_size inconsistency in smb2_duplicate_extents() on FSCTL failure
Frank Sorenson <[email protected]>
| Newsgroups | org.kernel.vger.linux-cifs,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
On 8/20/26 8:02 PM, Namjae Jeon wrote: > On Fri, Aug 21, 2026 at 6:14 AM Frank Sorenson <[email protected]> wrote: >> smb2_duplicate_extents() pre-extends the target file before sending >> FSCTL_DUPLICATE_EXTENTS_TO_FILE. If the FSCTL fails (e.g. ENOSPC, >> byte-range lock conflict, unsupported file combination), the server >> and client i_size are left reflecting the larger size while the data >> in the extended range was never cloned. >> >> Save the original i_size before pre-extension and restore it on FSCTL >> failure. If rollback fails, force revalidation instead. >> >> Fixes: cfc63fc8126a ("smb3: fix cached file size problems in duplicate extents (reflink)") >> Cc: [email protected] >> Signed-off-by: Frank Sorenson <[email protected]> >> --- >> fs/smb/client/smb2ops.c | 21 ++++++++++++++++++++- >> 1 file changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c >> index 7d6738ffcb80..38bd344a9740 100644 >> --- a/fs/smb/client/smb2ops.c >> +++ b/fs/smb/client/smb2ops.c >> @@ -2200,6 +2200,7 @@ smb2_duplicate_extents(const unsigned int xid, >> struct duplicate_extents_to_file dup_ext_buf; >> struct timespec64 ts; >> struct cifs_tcon *tcon = tlink_tcon(trgtfile->tlink); >> + loff_t orig_size; >> u64 asize; >> >> /* server fileays advertise duplicate extent support with this flag */ >> @@ -2218,7 +2219,8 @@ smb2_duplicate_extents(const unsigned int xid, >> trgtfile->fid.volatile_fid, tcon->tid, >> tcon->ses->Suid, src_off, dest_off, len); >> inode = d_inode(trgtfile->dentry); >> - if (inode->i_size < dest_off + len) { >> + orig_size = i_size_read(inode); >> + if (orig_size < dest_off + len) { >> rc = smb2_set_file_size(xid, tcon, trgtfile, dest_off + len, false); >> if (rc) >> goto duplicate_extents_out; >> @@ -2235,6 +2237,23 @@ smb2_duplicate_extents(const unsigned int xid, >> if (ret_data_len > 0) >> cifs_dbg(FYI, "Non-zero response length in duplicate extents\n"); >> >> + if (rc && i_size_read(inode) > orig_size) { >> + int rrc; >> + >> + /* >> + * FSCTL failed after we pre-extended the file. Attempt to >> + * restore the original size so the caller sees a consistent >> + * file rather than a larger file with uncloned content. >> + */ >> + rrc = smb2_set_file_size(xid, tcon, trgtfile, orig_size, false); > orig_size is only the locally cached size, and > lock_two_nondirectories() does not prevent another SMB client from > modifying the file. If the FSCTL fails, this rollback may truncate the > server-side file to a stale, smaller size and delete data written by > that client. Hmm, yes... it's a fundamental problem; we only have the locally cached size. So if the FSCTL fails, we need to force revalidation and invalidate the cache. -- Frank Sorenson [email protected] Principal Software Maintenance Engineer, filesystems Red Hat