[PATCH] cifs: fix use-after-free of server info on cifs_ses_add_channel error

[PATCH] cifs: fix use-after-free of server info on cifs_ses_add_channel error

[Henry Martin]

The error path of cifs_ses_add_channel() calls cifs_put_tcp_session()
before cifs_chan_clear_need_reconnect().  The latter reaches
cifs_ses_get_chan_index(), which dereferences server->terminate; if
the put was the final reference, TCP_Server_Info is already freed and
this is a use-after-free.  Same put-then-use pattern as the recently
fixed sibling in commit 717e0a25036b6.

Reorder: clear the reconnect bit and drop chan_count under chan_lock
first, release the server reference last.

This vulnerability was discovered by Tencent CodeBuddy Security.

Cc: stable@vger.kernel.org
Fixes: ee1d21794e55a ("cifs: handle when server stops supporting multichannel")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---
 fs/smb/client/sess.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
index e095f41b5882..650c0f855a29 100644
--- a/fs/smb/client/sess.c
+++ b/fs/smb/client/sess.c
@@ -626,12 +626,16 @@ cifs_ses_add_channel(struct cifs_ses *ses,

 out:
 	if (rc && chan->server) {
-		cifs_put_tcp_session(chan->server, 0);
-
 		spin_lock(&ses->chan_lock);

-		/* we rely on all bits beyond chan_count to be clear */
+		/* Clear the reconnect bitmask BEFORE dropping the server
+		 * reference: cifs_ses_get_chan_index() dereferences
+		 * server->terminate, and cifs_put_tcp_session() may be the
+		 * final put that frees chan->server (use-after-free).
+		 */
 		cifs_chan_clear_need_reconnect(ses, chan->server);
+
+		/* we rely on all bits beyond chan_count to be clear */
 		ses->chan_count--;
 		/*
 		 * chan_count should never reach 0 as at least the primary
@@ -639,6 +643,8 @@ cifs_ses_add_channel(struct cifs_ses *ses,
 		 */
 		WARN_ON(ses->chan_count < 1);
 		spin_unlock(&ses->chan_lock);
+
+		cifs_put_tcp_session(chan->server, 0);
 	}

 	kfree(ctx->UNC);
--
2.43.7

Re: [PATCH] cifs: fix use-after-free of server info on cifs_ses_add_channel error

[Namjae Jeon]

On Fri, Oct 9, 2026 at 5:03 PM Henry Martin <bsdhenrymartin@gmail.com> wrote:
>
> The error path of cifs_ses_add_channel() calls cifs_put_tcp_session()
> before cifs_chan_clear_need_reconnect().  The latter reaches
> cifs_ses_get_chan_index(), which dereferences server->terminate; if
> the put was the final reference, TCP_Server_Info is already freed and
> this is a use-after-free.  Same put-then-use pattern as the recently
> fixed sibling in commit 717e0a25036b6.
>
> Reorder: clear the reconnect bit and drop chan_count under chan_lock
> first, release the server reference last.
>
> This vulnerability was discovered by Tencent CodeBuddy Security.
>
> Cc: stable@vger.kernel.org
> Fixes: ee1d21794e55a ("cifs: handle when server stops supporting multichannel")
> Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
> ---
>  fs/smb/client/sess.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
> index e095f41b5882..650c0f855a29 100644
> --- a/fs/smb/client/sess.c
> +++ b/fs/smb/client/sess.c
> @@ -626,12 +626,16 @@ cifs_ses_add_channel(struct cifs_ses *ses,
>
>  out:
>         if (rc && chan->server) {
> -               cifs_put_tcp_session(chan->server, 0);
> -
>                 spin_lock(&ses->chan_lock);
>
> -               /* we rely on all bits beyond chan_count to be clear */
> +               /* Clear the reconnect bitmask BEFORE dropping the server
> +                * reference: cifs_ses_get_chan_index() dereferences
> +                * server->terminate, and cifs_put_tcp_session() may be the
> +                * final put that frees chan->server (use-after-free).
> +                */
>                 cifs_chan_clear_need_reconnect(ses, chan->server);
> +
> +               /* we rely on all bits beyond chan_count to be clear */
>                 ses->chan_count--;
>                 /*
>                  * chan_count should never reach 0 as at least the primary
> @@ -639,6 +643,8 @@ cifs_ses_add_channel(struct cifs_ses *ses,
>                  */
>                 WARN_ON(ses->chan_count < 1);
>                 spin_unlock(&ses->chan_lock);
> +
> +               cifs_put_tcp_session(chan->server, 0);
As soon as the lock is released, a new channel can be added in the
slot indicated by the decremented chan_count. Then the final put will
act on the new channel. Please save the server pointer under chan_lock
and use that for cifs_put_tcp_session().

Re: [PATCH] cifs: fix use-after-free of server info on cifs_ses_add_channel error

[henry martin]

Hi Jeon,

You're right, thanks for catching that.  chan points into
ses->chans[], so once chan_count is decremented under chan_lock a
concurrent add_channel can reuse the slot and overwrite chan->server
before the unlocked put reads it.

v2 caches the pointer in a local under chan_lock and uses the cached
pointer for both cifs_chan_clear_need_reconnect() and the final
cifs_put_tcp_session(), so the reference we drop is always the one
this error path took.

Regards,
Henry

Namjae Jeon <linkinjeon@kernel.org> 于2026年10月9日周五 21:32写道：
>
> On Fri, Oct 9, 2026 at 5:03 PM Henry Martin <bsdhenrymartin@gmail.com> wrote:
> >
> > The error path of cifs_ses_add_channel() calls cifs_put_tcp_session()
> > before cifs_chan_clear_need_reconnect().  The latter reaches
> > cifs_ses_get_chan_index(), which dereferences server->terminate; if
> > the put was the final reference, TCP_Server_Info is already freed and
> > this is a use-after-free.  Same put-then-use pattern as the recently
> > fixed sibling in commit 717e0a25036b6.
> >
> > Reorder: clear the reconnect bit and drop chan_count under chan_lock
> > first, release the server reference last.
> >
> > This vulnerability was discovered by Tencent CodeBuddy Security.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: ee1d21794e55a ("cifs: handle when server stops supporting multichannel")
> > Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
> > ---
> >  fs/smb/client/sess.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
> > index e095f41b5882..650c0f855a29 100644
> > --- a/fs/smb/client/sess.c
> > +++ b/fs/smb/client/sess.c
> > @@ -626,12 +626,16 @@ cifs_ses_add_channel(struct cifs_ses *ses,
> >
> >  out:
> >         if (rc && chan->server) {
> > -               cifs_put_tcp_session(chan->server, 0);
> > -
> >                 spin_lock(&ses->chan_lock);
> >
> > -               /* we rely on all bits beyond chan_count to be clear */
> > +               /* Clear the reconnect bitmask BEFORE dropping the server
> > +                * reference: cifs_ses_get_chan_index() dereferences
> > +                * server->terminate, and cifs_put_tcp_session() may be the
> > +                * final put that frees chan->server (use-after-free).
> > +                */
> >                 cifs_chan_clear_need_reconnect(ses, chan->server);
> > +
> > +               /* we rely on all bits beyond chan_count to be clear */
> >                 ses->chan_count--;
> >                 /*
> >                  * chan_count should never reach 0 as at least the primary
> > @@ -639,6 +643,8 @@ cifs_ses_add_channel(struct cifs_ses *ses,
> >                  */
> >                 WARN_ON(ses->chan_count < 1);
> >                 spin_unlock(&ses->chan_lock);
> > +
> > +               cifs_put_tcp_session(chan->server, 0);
> As soon as the lock is released, a new channel can be added in the
> slot indicated by the decremented chan_count. Then the final put will
> act on the new channel. Please save the server pointer under chan_lock
> and use that for cifs_put_tcp_session().

[PATCH v2] cifs: fix use-after-free of server info on cifs_ses_add_channel error

[Henry Martin]

The error path of cifs_ses_add_channel() calls cifs_put_tcp_session()
before cifs_chan_clear_need_reconnect().  The latter reaches
cifs_ses_get_chan_index(), which dereferences server->terminate; if
the put was the final reference, TCP_Server_Info is already freed and
this is a use-after-free.  Same put-then-use pattern as the recently
fixed sibling in commit 717e0a25036b ("cifs: Fix server use-after-free
in cifs_chan_skip_or_disable()").

Move the put last, but do not read chan->server after dropping
chan_lock: chan points into ses->chans[], and once chan_count is
decremented a concurrent add_channel can reuse the slot and overwrite
the pointer, so the put would act on the new channel's server.  Cache
the pointer under chan_lock, clear the reconnect bit and decrement
chan_count while still holding the reference, and release it after
the unlock.

This issue was discovered by Tencent CodeBuddy Security.

Cc: stable@vger.kernel.org
Fixes: ee1d21794e55a ("cifs: handle when server stops supporting multichannel")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---

v2: Cache chan->server in a local under chan_lock and use the cached
    pointer for cifs_put_tcp_session(); the slot can be reused by a
    concurrent add once chan_count is decremented.

 fs/smb/client/sess.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
index e095f41b58828..ba0b4057322c8 100644
--- a/fs/smb/client/sess.c
+++ b/fs/smb/client/sess.c
@@ -626,12 +626,19 @@ cifs_ses_add_channel(struct cifs_ses *ses,
 
 out:
 	if (rc && chan->server) {
-		cifs_put_tcp_session(chan->server, 0);
+		struct TCP_Server_Info *pserver;
 
 		spin_lock(&ses->chan_lock);
 
+		/*
+		 * Cache the server pointer while holding chan_lock: once
+		 * chan_count is decremented below, this slot can be reused
+		 * by a concurrent add_channel and chan->server overwritten.
+		 */
+		pserver = chan->server;
+
 		/* we rely on all bits beyond chan_count to be clear */
-		cifs_chan_clear_need_reconnect(ses, chan->server);
+		cifs_chan_clear_need_reconnect(ses, pserver);
 		ses->chan_count--;
 		/*
 		 * chan_count should never reach 0 as at least the primary
@@ -639,6 +646,8 @@ cifs_ses_add_channel(struct cifs_ses *ses,
 		 */
 		WARN_ON(ses->chan_count < 1);
 		spin_unlock(&ses->chan_lock);
+
+		cifs_put_tcp_session(pserver, 0);
 	}
 
 	kfree(ctx->UNC);
-- 
2.43.7