linux-kernel - Re: [PATCH -next] smb: client: remove redundant lstrp update in negotiate protocol

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAH2r5muBsUA3V+JKHuNj-TeOqW5Bhg6cS+2A+zx17owHCYJw4g@mail.gmail.com>
Date: Sun, 10 Aug 2025 22:49:46 -0500
From: Steve French <smfrench@...il.com>
To: Wang Zhaolong <wangzhaolong@...weicloud.com>
Cc: linux-cifs@...r.kernel.org, samba-technical@...ts.samba.org, 
	linux-kernel@...r.kernel.org, yi.zhang@...wei.com, yangerkun@...wei.com, 
	chengzhihao1@...wei.com
Subject: Re: [PATCH -next] smb: client: remove redundant lstrp update in
 negotiate protocol

tentatively merged into cifs-2.6.git for-next pending more review and testing

On Fri, Aug 1, 2025 at 4:07 AM Wang Zhaolong
<wangzhaolong@...weicloud.com> wrote:
>
> Commit 34331d7beed7 ("smb: client: fix first command failure during
> re-negotiation") addressed a race condition by updating lstrp before
> entering negotiate state. However, this approach may have some unintended
> side effects.
>
> The lstrp field is documented as "when we got last response from this
> server", and updating it before actually receiving a server response
> could potentially affect other mechanisms that rely on this timestamp.
> For example, the SMB echo detection logic also uses lstrp as a reference
> point. In scenarios with frequent user operations during reconnect states,
> the repeated calls to cifs_negotiate_protocol() might continuously
> update lstrp, which could interfere with the echo detection timing.
>
> Additionally, commit 266b5d02e14f ("smb: client: fix race condition in
> negotiate timeout by using more precise timing") introduced a dedicated
> neg_start field specifically for tracking negotiate start time. This
> provides a more precise solution for the original race condition while
> preserving the intended semantics of lstrp.
>
> Since the race condition is now properly handled by the neg_start
> mechanism, the lstrp update in cifs_negotiate_protocol() is no longer
> necessary and can be safely removed.
>
> Fixes: 266b5d02e14f ("smb: client: fix race condition in negotiate timeout by using more precise timing")
> Cc: stable@...r.kernel.org
> Signed-off-by: Wang Zhaolong <wangzhaolong@...weicloud.com>
> ---
>  fs/smb/client/connect.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> index 205f547ca49e..a2c49683be66 100644
> --- a/fs/smb/client/connect.c
> +++ b/fs/smb/client/connect.c
> @@ -4205,11 +4205,10 @@ cifs_negotiate_protocol(const unsigned int xid, struct cifs_ses *ses,
>             server->tcpStatus == CifsGood) {
>                 spin_unlock(&server->srv_lock);
>                 return 0;
>         }
>
> -       server->lstrp = jiffies;
>         server->tcpStatus = CifsInNegotiate;
>         server->neg_start = jiffies;
>         spin_unlock(&server->srv_lock);
>
>         rc = server->ops->negotiate(xid, ses, server);
> --
> 2.34.3
>
>


-- 
Thanks,

Steve