The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] fix smb client defer close causes file corruption
@ 2026-06-22  9:14 Chunjie Zhu
  2026-06-23 20:39 ` Steve French
  0 siblings, 1 reply; 6+ messages in thread
From: Chunjie Zhu @ 2026-06-22  9:14 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey
  Cc: Chunjie Zhu, linux-cifs, samba-technical, linux-kernel

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] fix smb client defer close causes file corruption
  2026-06-22  9:14 [PATCH] fix smb client defer close causes file corruption Chunjie Zhu
@ 2026-06-23 20:39 ` Steve French
  2026-06-24  3:44   ` Subject: " Chunjie Zhu
  2026-06-24  4:19   ` [PATCH] fix smb client defer close causes file corruption Enzo Matsumiya
  0 siblings, 2 replies; 6+ messages in thread
From: Steve French @ 2026-06-23 20:39 UTC (permalink / raw)
  To: Chunjie Zhu
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, linux-cifs, samba-technical, linux-kernel

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.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Subject: Re: [PATCH] fix smb client defer close causes file corruption
  2026-06-23 20:39 ` Steve French
@ 2026-06-24  3:44   ` Chunjie Zhu
  2026-06-24  4:18     ` Enzo Matsumiya
  2026-06-24  4:19   ` [PATCH] fix smb client defer close causes file corruption Enzo Matsumiya
  1 sibling, 1 reply; 6+ messages in thread
From: Chunjie Zhu @ 2026-06-24  3:44 UTC (permalink / raw)
  To: smfrench
  Cc: chunjie.zhu, linux-cifs, linux-kernel, lsahlber, pc,
	samba-technical, sfrench, sprasad, tom

From my experience, these common workloads tend to occur within a single
process. Therefore, it might be worth optimizing cifs_get_readable_path
by extending its matching logic: instead of matching only the file path,
we could also check the process PID. The old handler would then be reused
only when both the file path and the PID match.

Do you think this approach is viable?

> 
> Other narrower client fixes for this presumably could be done, e.g.
> forcing close on the deferred close when rename a hardlink.
> 
> On Mon, Jun 22, 2026 at 4:40=E2=80=AFAM 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=3D3.0,
> >   cache=3Dloose,username=3Dxxx,domain=3Dxxx,uid=3D0,noforceuid,
> >   gid=3D0,noforcegid,addr=3D10.70.48.15,file_mode=3D0755,
> >   dir_mode=3D0755,soft,nounix,serverino,mapposix,reparse=3Dnfs,
> >   rsize=3D1048576,wsize=3D1048576,bsize=3D1048576,echo_interval=3D60,
> >   actimeo=3D0,closetimeo=3D1
> >
> > Work around
> >
> >  mount with cache=3Dnone or closetimeo=3D0
> >
> > 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=3D=E2=80=9Ceeefe8d0.vhd=E2=80=9D, handle=3DH1, inode=
> =3DI1 }
> >
> >  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 =3D 2)
> >    -> Host-01 lease L1: NO BREAK (same inode, just added name)
> >
> >    crate(eeefe8d0.vhd.new)
> >    -> Entry { path=3D"eeefe8d0.vhd.new", handle=3DH2, inode=3DI2 }
> >
> >    rename(eeefe8d0.vhd.new, eeefe8d0.vhd)
> >    -> SMB2: Replaces =E2=80=9Ceeefe8d0.vhd=E2=80=9D name =E2=86=92 points=
>  to new inode I2
> >    -> Server: old inode I1 now only accessible as =E2=80=9C0f11b74e.vhd=
> =E2=80=9D
> >    -> Server SHOULD send: Lease Break notification to H1 =E2=86=90 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=3D=E2=80=9Ceeefe8d0.vhd=E2=80=9D, handle=3DH1, inode=3DI1_OL=
> D }
> >
> >    Host-02:
> >    -> Open 0f11b74e.vhd in readonly
> >
> >    Result:
> >       { path=3D"0f11b74e.vhd", inode=3DI1_NEW }
> >
> >  Step 4: Host-01 reopens file
> >
> >   Host-01:
> >    file open (eeefe8d0.vhd)
> >    -> Kernel checks deferred_closes for =E2=80=9Ceeefe8d0.vhd=E2=80=9D
> >    -> 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 =3D ctx->acregmax =3D 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 !=3D 0) {
> > +                       cifs_errorf(fc, "closetimeo must be 0, deferred c=
> lose is disabled\n");
> >                         goto cifs_parse_mount_err;
> >                 }
> > -               ctx->closetimeo =3D HZ * result.uint_32;
> > +               ctx->closetimeo =3D 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 =3D CIFS_DEF_ACTIMEO;
> >         ctx->acdirmax =3D CIFS_DEF_ACTIMEO;
> > -       ctx->closetimeo =3D SMB3_DEF_DCLOSETIMEO;
> > +       ctx->closetimeo =3D 0;
> >         ctx->max_cached_dirs =3D MAX_CACHED_FIDS;
> >         /* Most clients set timeout to 0, allows server to use its defaul=
> t */
> >         ctx->handle_timeout =3D 0; /* See MS-SMB2 spec section 2.2.14.2.1=
> 2 */
> > --
> > 2.52.0
> >
> >
> 
> 
> --=20
> Thanks,
> 
> Steve
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Subject: Re: [PATCH] fix smb client defer close causes file corruption
  2026-06-24  3:44   ` Subject: " Chunjie Zhu
@ 2026-06-24  4:18     ` Enzo Matsumiya
  2026-06-24  7:19       ` Subject: Re: [PATCH] fix smb client defer close causes file Chunjie Zhu
  0 siblings, 1 reply; 6+ messages in thread
From: Enzo Matsumiya @ 2026-06-24  4:18 UTC (permalink / raw)
  To: Chunjie Zhu
  Cc: smfrench, linux-cifs, linux-kernel, lsahlber, pc, samba-technical,
	sfrench, sprasad, tom

On 06/24, Chunjie Zhu wrote:
>From my experience, these common workloads tend to occur within a single
>process. Therefore, it might be worth optimizing cifs_get_readable_path
>by extending its matching logic: instead of matching only the file path,
>we could also check the process PID. The old handler would then be reused
>only when both the file path and the PID match.
>
>Do you think this approach is viable?

I don't think this is a good idea; PIDs change and are naturally made to
be reused as well, so storing PID for a 'later' (no matter how later)
check is 100% unreliable.

As for the original bug report, do you happen to have a simple
reproducer?

I created one on top of your instructions and I'm definitely getting a
lease break on rename, which IIUC is the alleged root cause.
This was with closetimeo=30 on a Windows Server 2022 share.

Also, I'm failing to understand how cache= has any impact on this -- I
get the same successful results with none,loose,strict.


Cheers,

Enzo

>> Other narrower client fixes for this presumably could be done, e.g.
>> forcing close on the deferred close when rename a hardlink.
>>
>> On Mon, Jun 22, 2026 at 4:40=E2=80=AFAM 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=3D3.0,
>> >   cache=3Dloose,username=3Dxxx,domain=3Dxxx,uid=3D0,noforceuid,
>> >   gid=3D0,noforcegid,addr=3D10.70.48.15,file_mode=3D0755,
>> >   dir_mode=3D0755,soft,nounix,serverino,mapposix,reparse=3Dnfs,
>> >   rsize=3D1048576,wsize=3D1048576,bsize=3D1048576,echo_interval=3D60,
>> >   actimeo=3D0,closetimeo=3D1
>> >
>> > Work around
>> >
>> >  mount with cache=3Dnone or closetimeo=3D0
>> >
>> > 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=3D=E2=80=9Ceeefe8d0.vhd=E2=80=9D, handle=3DH1, inode=
>> =3DI1 }
>> >
>> >  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 =3D 2)
>> >    -> Host-01 lease L1: NO BREAK (same inode, just added name)
>> >
>> >    crate(eeefe8d0.vhd.new)
>> >    -> Entry { path=3D"eeefe8d0.vhd.new", handle=3DH2, inode=3DI2 }
>> >
>> >    rename(eeefe8d0.vhd.new, eeefe8d0.vhd)
>> >    -> SMB2: Replaces =E2=80=9Ceeefe8d0.vhd=E2=80=9D name =E2=86=92 points=
>>  to new inode I2
>> >    -> Server: old inode I1 now only accessible as =E2=80=9C0f11b74e.vhd=
>> =E2=80=9D
>> >    -> Server SHOULD send: Lease Break notification to H1 =E2=86=90 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=3D=E2=80=9Ceeefe8d0.vhd=E2=80=9D, handle=3DH1, inode=3DI1_OL=
>> D }
>> >
>> >    Host-02:
>> >    -> Open 0f11b74e.vhd in readonly
>> >
>> >    Result:
>> >       { path=3D"0f11b74e.vhd", inode=3DI1_NEW }
>> >
>> >  Step 4: Host-01 reopens file
>> >
>> >   Host-01:
>> >    file open (eeefe8d0.vhd)
>> >    -> Kernel checks deferred_closes for =E2=80=9Ceeefe8d0.vhd=E2=80=9D
>> >    -> 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 =3D ctx->acregmax =3D 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 !=3D 0) {
>> > +                       cifs_errorf(fc, "closetimeo must be 0, deferred c=
>> lose is disabled\n");
>> >                         goto cifs_parse_mount_err;
>> >                 }
>> > -               ctx->closetimeo =3D HZ * result.uint_32;
>> > +               ctx->closetimeo =3D 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 =3D CIFS_DEF_ACTIMEO;
>> >         ctx->acdirmax =3D CIFS_DEF_ACTIMEO;
>> > -       ctx->closetimeo =3D SMB3_DEF_DCLOSETIMEO;
>> > +       ctx->closetimeo =3D 0;
>> >         ctx->max_cached_dirs =3D MAX_CACHED_FIDS;
>> >         /* Most clients set timeout to 0, allows server to use its defaul=
>> t */
>> >         ctx->handle_timeout =3D 0; /* See MS-SMB2 spec section 2.2.14.2.1=
>> 2 */
>> > --
>> > 2.52.0
>> >
>> >
>>
>>
>> --=20
>> Thanks,
>>
>> Steve
>>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fix smb client defer close causes file corruption
  2026-06-23 20:39 ` Steve French
  2026-06-24  3:44   ` Subject: " Chunjie Zhu
@ 2026-06-24  4:19   ` Enzo Matsumiya
  1 sibling, 0 replies; 6+ messages in thread
From: Enzo Matsumiya @ 2026-06-24  4:19 UTC (permalink / raw)
  To: Steve French
  Cc: Chunjie Zhu, Steve French, Paulo Alcantara, Ronnie Sahlberg,
	Shyam Prasad N, Tom Talpey, linux-cifs, samba-technical,
	linux-kernel

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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Subject: Re: [PATCH] fix smb client defer close causes file
  2026-06-24  4:18     ` Enzo Matsumiya
@ 2026-06-24  7:19       ` Chunjie Zhu
  0 siblings, 0 replies; 6+ messages in thread
From: Chunjie Zhu @ 2026-06-24  7:19 UTC (permalink / raw)
  To: ematsumiya
  Cc: chunjie.zhu, linux-cifs, linux-kernel, lsahlber, pc,
	samba-technical, sfrench, smfrench, sprasad, tom

It looks like it's not enough to mark a reused file handle by only file path.
Is there any plan to update it?
 
> As for the original bug report, do you happen to have a simple
> reproducer?
> 
> I created one on top of your instructions and I'm definitely getting a
> lease break on rename, which IIUC is the alleged root cause.
> This was with closetimeo=30 on a Windows Server 2022 share.

4 clients shared mount looks simple. Currently I do not have a simpler
reproduce step or script.

> Also, I'm failing to understand how cache= has any impact on this -- I
> get the same successful results with none,loose,strict.

In real tests with cache=none, we cannot see the symptom, by disabling
cache/buffer, client goes back to the server more directly, and stale cached
data/metadata windows shrink.

> 
> Cheers,
> 
> Enzo
> 
> >> Other narrower client fixes for this presumably could be done, e.g.
> >> forcing close on the deferred close when rename a hardlink.
> >>
> >> On Mon, Jun 22, 2026 at 4:40=E2=80=AFAM 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=3D3.0,
> >> >   cache=3Dloose,username=3Dxxx,domain=3Dxxx,uid=3D0,noforceuid,
> >> >   gid=3D0,noforcegid,addr=3D10.70.48.15,file_mode=3D0755,
> >> >   dir_mode=3D0755,soft,nounix,serverino,mapposix,reparse=3Dnfs,
> >> >   rsize=3D1048576,wsize=3D1048576,bsize=3D1048576,echo_interval=3D60,
> >> >   actimeo=3D0,closetimeo=3D1
> >> >
> >> > Work around
> >> >
> >> >  mount with cache=3Dnone or closetimeo=3D0
> >> >
> >> > 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=3D=E2=80=9Ceeefe8d0.vhd=E2=80=9D, handle=3DH1, inode=
> >> =3DI1 }
> >> >
> >> >  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 =3D 2)
> >> >    -> Host-01 lease L1: NO BREAK (same inode, just added name)
> >> >
> >> >    crate(eeefe8d0.vhd.new)
> >> >    -> Entry { path=3D"eeefe8d0.vhd.new", handle=3DH2, inode=3DI2 }
> >> >
> >> >    rename(eeefe8d0.vhd.new, eeefe8d0.vhd)
> >> >    -> SMB2: Replaces =E2=80=9Ceeefe8d0.vhd=E2=80=9D name =E2=86=92 points=
> >>  to new inode I2
> >> >    -> Server: old inode I1 now only accessible as =E2=80=9C0f11b74e.vhd=
> >> =E2=80=9D
> >> >    -> Server SHOULD send: Lease Break notification to H1 =E2=86=90 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=3D=E2=80=9Ceeefe8d0.vhd=E2=80=9D, handle=3DH1, inode=3DI1_OL=
> >> D }
> >> >
> >> >    Host-02:
> >> >    -> Open 0f11b74e.vhd in readonly
> >> >
> >> >    Result:
> >> >       { path=3D"0f11b74e.vhd", inode=3DI1_NEW }
> >> >
> >> >  Step 4: Host-01 reopens file
> >> >
> >> >   Host-01:
> >> >    file open (eeefe8d0.vhd)
> >> >    -> Kernel checks deferred_closes for =E2=80=9Ceeefe8d0.vhd=E2=80=9D
> >> >    -> 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 =3D ctx->acregmax =3D 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 !=3D 0) {
> >> > +                       cifs_errorf(fc, "closetimeo must be 0, deferred c=
> >> lose is disabled\n");
> >> >                         goto cifs_parse_mount_err;
> >> >                 }
> >> > -               ctx->closetimeo =3D HZ * result.uint_32;
> >> > +               ctx->closetimeo =3D 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 =3D CIFS_DEF_ACTIMEO;
> >> >         ctx->acdirmax =3D CIFS_DEF_ACTIMEO;
> >> > -       ctx->closetimeo =3D SMB3_DEF_DCLOSETIMEO;
> >> > +       ctx->closetimeo =3D 0;
> >> >         ctx->max_cached_dirs =3D MAX_CACHED_FIDS;
> >> >         /* Most clients set timeout to 0, allows server to use its defaul=
> >> t */
> >> >         ctx->handle_timeout =3D 0; /* See MS-SMB2 spec section 2.2.14.2.1=
> >> 2 */
> >> > --
> >> > 2.52.0
> >> >
> >> >
> >>
> >>
> >> --=20
> >> Thanks,
> >>
> >> Steve
> >>
> >
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-24  7:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-22  9:14 [PATCH] fix smb client defer close causes file corruption Chunjie Zhu
2026-06-23 20:39 ` Steve French
2026-06-24  3:44   ` Subject: " Chunjie Zhu
2026-06-24  4:18     ` Enzo Matsumiya
2026-06-24  7:19       ` Subject: Re: [PATCH] fix smb client defer close causes file Chunjie Zhu
2026-06-24  4:19   ` [PATCH] fix smb client defer close causes file corruption Enzo Matsumiya

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox