Re: [PATCH] fix smb client defer close causes file corruption

[Enzo Matsumiya <ematsumiya@suse.de>]

On 06/23, Steve French wrote:
>Do you see different behavior of this to different servers (Samba,
>Windows, ksmbd, Azure xSMB etc)?
>
>Disabling deferred close carries metadata integrity issues because it
>will lead users to set actimeo (or acregmax) to higher values, and
>disabling deferred close could significantly hurt performance for some
>common workload patterns (open/write/close/open/read/close e.g.)
>
>Other narrower client fixes for this presumably could be done, e.g.
>forcing close on the deferred close when rename a hardlink.

That looks like a safe measure to have IMO.


Cheers,

Enzo

>On Mon, Jun 22, 2026 at 4:40 AM Chunjie Zhu <chunjie.zhu@citrix.com> wrote:
>>
>> Test environment
>>
>>  4 hosts as smb client, 1 host as smb server
>>  smb client hosts, kernel 6.6.138
>>  mount options,
>>   //10.70.48.15/xxx /run/xxx cifs rw,relatime,vers=3.0,
>>   cache=loose,username=xxx,domain=xxx,uid=0,noforceuid,
>>   gid=0,noforcegid,addr=10.70.48.15,file_mode=0755,
>>   dir_mode=0755,soft,nounix,serverino,mapposix,reparse=nfs,
>>   rsize=1048576,wsize=1048576,bsize=1048576,echo_interval=60,
>>   actimeo=0,closetimeo=1
>>
>> Work around
>>
>>  mount with cache=none or closetimeo=0
>>
>> The Race Condition Flow
>>
>>  Step 1: Host-01 closes file
>>
>>   Host-01:
>>    file close (eeefe8d0.vhd)
>>    -> CIFS defers SMB2 CLOSE
>>    -> Handle H1 stored in deferred_closes list
>>    -> Lease L1 (RWH or RH) still active on server
>>    -> Entry: { path=“eeefe8d0.vhd”, handle=H1, inode=I1 }
>>
>>  Step 2: Host-02 does hardlink and rename
>>
>>   Host-02:
>>    hardlink(eeefe8d0.vhd, 0f11b74e.vhd)
>>    -> SMB2: Creates new name for same inode
>>    -> Server: inode I1 now has 2 names (link count = 2)
>>    -> Host-01 lease L1: NO BREAK (same inode, just added name)
>>
>>    crate(eeefe8d0.vhd.new)
>>    -> Entry { path="eeefe8d0.vhd.new", handle=H2, inode=I2 }
>>
>>    rename(eeefe8d0.vhd.new, eeefe8d0.vhd)
>>    -> SMB2: Replaces “eeefe8d0.vhd” name → points to new inode I2
>>    -> Server: old inode I1 now only accessible as “0f11b74e.vhd”
>>    -> Server SHOULD send: Lease Break notification to H1 ← KEY!
>>
>>  Step 3: Lease break delivery is not reliable
>>
>>   strict locking off, level2 oplock
>>
>>    Host-01:
>>    -> Lease break not received or processed
>>    -> H1 is in deferred_closes list (not "active")
>>
>>    Result: Stale entry remains:
>>       { path=“eeefe8d0.vhd”, handle=H1, inode=I1_OLD }
>>
>>    Host-02:
>>    -> Open 0f11b74e.vhd in readonly
>>
>>    Result:
>>       { path="0f11b74e.vhd", inode=I1_NEW }
>>
>>  Step 4: Host-01 reopens file
>>
>>   Host-01:
>>    file open (eeefe8d0.vhd)
>>    -> Kernel checks deferred_closes for “eeefe8d0.vhd”
>>    -> Found H1! (matched by pathname string)
>>    -> REUSES H1 without checking
>>    -> close or reconnect, flush buffered writes
>>       slient corruption?
>>
>> Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
>> ---
>>  fs/smb/client/fs_context.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
>> index 0812af001417..4ed33de0a00d 100644
>> --- a/fs/smb/client/fs_context.c
>> +++ b/fs/smb/client/fs_context.c
>> @@ -1300,11 +1300,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
>>                 ctx->acdirmax = ctx->acregmax = HZ * result.uint_32;
>>                 break;
>>         case Opt_closetimeo:
>> -               if (result.uint_32 > SMB3_MAX_DCLOSETIMEO / HZ) {
>> -                       cifs_errorf(fc, "closetimeo too large\n");
>> +               if (result.uint_32 != 0) {
>> +                       cifs_errorf(fc, "closetimeo must be 0, deferred close is disabled\n");
>>                         goto cifs_parse_mount_err;
>>                 }
>> -               ctx->closetimeo = HZ * result.uint_32;
>> +               ctx->closetimeo = 0;
>>                 break;
>>         case Opt_echo_interval:
>>                 if (result.uint_32 < SMB_ECHO_INTERVAL_MIN ||
>> @@ -1795,7 +1795,7 @@ int smb3_init_fs_context(struct fs_context *fc)
>>
>>         ctx->acregmax = CIFS_DEF_ACTIMEO;
>>         ctx->acdirmax = CIFS_DEF_ACTIMEO;
>> -       ctx->closetimeo = SMB3_DEF_DCLOSETIMEO;
>> +       ctx->closetimeo = 0;
>>         ctx->max_cached_dirs = MAX_CACHED_FIDS;
>>         /* Most clients set timeout to 0, allows server to use its default */
>>         ctx->handle_timeout = 0; /* See MS-SMB2 spec section 2.2.14.2.12 */
>> --
>> 2.52.0
>>
>>
>
>
>-- 
>Thanks,
>
>Steve
>