[PATCH] smb: client: fix potential use-after-free with TCP_Server_Info->hostname

[PATCH] smb: client: fix potential use-after-free with TCP_Server_Info->hostname

[Frank Sorenson]

The TCP_Server_Info->hostname pointer may be updated during a reconnect
event. Accessing ses->server->hostname outside of the srv_lock is
unsafe and can lead to a use-after-free if another thread reallocates
the hostname string.

Fix this by replacing instances of cifs_dbg() that print the hostname
with cifs_server_dbg() across multiple files (smb2ops.c, sess.c, and
smb2pdu.c).  cifs_server_dbg() correctly acquires the srv_lock before
printing and standardizes the output by prepending the server hostname
to the message.

Fixes: 4659f01e3cd9 ("smb3: do not log confusing message when server returns no network interfaces")
Fixes: a6d8fb54a515 ("cifs: distribute channels across interfaces based on speed")
Fixes: f591062bdbf4 ("cifs: handle servers that still advertise multichannel after disabling")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/sess.c    | 5 ++---
 fs/smb/client/smb2ops.c | 7 +++----
 fs/smb/client/smb2pdu.c | 5 ++---
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
index e095f41b5882..5c2c9cabf7ef 100644
--- a/fs/smb/client/sess.c
+++ b/fs/smb/client/sess.c
@@ -192,8 +192,7 @@ int cifs_try_adding_channels(struct cifs_ses *ses)
 		spin_lock(&ses->iface_lock);
 		if (!ses->iface_count) {
 			spin_unlock(&ses->iface_lock);
-			cifs_dbg(ONCE, "server %s does not advertise interfaces\n",
-				      ses->server->hostname);
+			cifs_server_dbg(ONCE, "server does not advertise interfaces\n");
 			break;
 		}
 
@@ -395,7 +394,7 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
 	spin_lock(&ses->iface_lock);
 	if (!ses->iface_count) {
 		spin_unlock(&ses->iface_lock);
-		cifs_dbg(ONCE, "server %s does not advertise interfaces\n", ses->server->hostname);
+		cifs_server_dbg(ONCE, "server does not advertise interfaces\n");
 		return;
 	}
 
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 3464470d3297..08ea340f01b6 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -632,6 +632,7 @@ static int
 parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
 			size_t buf_len, struct cifs_ses *ses, bool in_mount)
 {
+	struct TCP_Server_Info *server = ses->server;
 	struct network_interface_info_ioctl_rsp *p;
 	struct sockaddr_in *addr4;
 	struct sockaddr_in6 *addr6;
@@ -666,10 +667,8 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
 	if (bytes_left == 0) {
 		/* avoid spamming logs every 10 minutes, so log only in mount */
 		if ((ses->chan_max > 1) && in_mount)
-			cifs_dbg(VFS,
-				 "multichannel not available\n"
-				 "Empty network interface list returned by server %s\n",
-				 ses->server->hostname);
+			cifs_server_dbg(VFS,
+					"multichannel not available - Empty network interface list returned by server\n");
 		rc = -EOPNOTSUPP;
 		goto out;
 	}
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index dea05aeb53a1..bb30cfa156aa 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -173,9 +173,8 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
 	unsigned int chan_index;
 
 	if (SERVER_IS_CHAN(server)) {
-		cifs_dbg(VFS,
-			"server %s does not support multichannel anymore. Skip secondary channel\n",
-			 ses->server->hostname);
+		cifs_server_dbg(VFS,
+				"server does not support multichannel anymore. Skip secondary channel\n");
 
 		spin_lock(&ses->chan_lock);
 		chan_index = cifs_ses_get_chan_index(ses, server);
-- 
2.55.0

Re: [PATCH] smb: client: fix potential use-after-free with TCP_Server_Info->hostname

[Frank Sorenson]

Please disregard this patch.  Two of the cifs_server_dbg() are called
with 'type=ONCE', for which cifs_dbg_func() doesn't ever print.

I will re-evaluate the rest of the patch and may resubmit the
other hunks later.

Frank


On 9/17/26 7:33 PM, Frank Sorenson wrote:
> The TCP_Server_Info->hostname pointer may be updated during a reconnect
> event. Accessing ses->server->hostname outside of the srv_lock is
> unsafe and can lead to a use-after-free if another thread reallocates
> the hostname string.
>
> Fix this by replacing instances of cifs_dbg() that print the hostname
> with cifs_server_dbg() across multiple files (smb2ops.c, sess.c, and
> smb2pdu.c).  cifs_server_dbg() correctly acquires the srv_lock before
> printing and standardizes the output by prepending the server hostname
> to the message.
>
> Fixes: 4659f01e3cd9 ("smb3: do not log confusing message when server returns no network interfaces")
> Fixes: a6d8fb54a515 ("cifs: distribute channels across interfaces based on speed")
> Fixes: f591062bdbf4 ("cifs: handle servers that still advertise multichannel after disabling")
> Cc: stable@vger.kernel.org
> Signed-off-by: Frank Sorenson <sorenson@redhat.com>
> ---
>   fs/smb/client/sess.c    | 5 ++---
>   fs/smb/client/smb2ops.c | 7 +++----
>   fs/smb/client/smb2pdu.c | 5 ++---
>   3 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
> index e095f41b5882..5c2c9cabf7ef 100644
> --- a/fs/smb/client/sess.c
> +++ b/fs/smb/client/sess.c
> @@ -192,8 +192,7 @@ int cifs_try_adding_channels(struct cifs_ses *ses)
>   		spin_lock(&ses->iface_lock);
>   		if (!ses->iface_count) {
>   			spin_unlock(&ses->iface_lock);
> -			cifs_dbg(ONCE, "server %s does not advertise interfaces\n",
> -				      ses->server->hostname);
> +			cifs_server_dbg(ONCE, "server does not advertise interfaces\n");
>   			break;
>   		}
>   
> @@ -395,7 +394,7 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
>   	spin_lock(&ses->iface_lock);
>   	if (!ses->iface_count) {
>   		spin_unlock(&ses->iface_lock);
> -		cifs_dbg(ONCE, "server %s does not advertise interfaces\n", ses->server->hostname);
> +		cifs_server_dbg(ONCE, "server does not advertise interfaces\n");
>   		return;
>   	}
>   
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 3464470d3297..08ea340f01b6 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -632,6 +632,7 @@ static int
>   parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
>   			size_t buf_len, struct cifs_ses *ses, bool in_mount)
>   {
> +	struct TCP_Server_Info *server = ses->server;
>   	struct network_interface_info_ioctl_rsp *p;
>   	struct sockaddr_in *addr4;
>   	struct sockaddr_in6 *addr6;
> @@ -666,10 +667,8 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
>   	if (bytes_left == 0) {
>   		/* avoid spamming logs every 10 minutes, so log only in mount */
>   		if ((ses->chan_max > 1) && in_mount)
> -			cifs_dbg(VFS,
> -				 "multichannel not available\n"
> -				 "Empty network interface list returned by server %s\n",
> -				 ses->server->hostname);
> +			cifs_server_dbg(VFS,
> +					"multichannel not available - Empty network interface list returned by server\n");
>   		rc = -EOPNOTSUPP;
>   		goto out;
>   	}
> diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> index dea05aeb53a1..bb30cfa156aa 100644
> --- a/fs/smb/client/smb2pdu.c
> +++ b/fs/smb/client/smb2pdu.c
> @@ -173,9 +173,8 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
>   	unsigned int chan_index;
>   
>   	if (SERVER_IS_CHAN(server)) {
> -		cifs_dbg(VFS,
> -			"server %s does not support multichannel anymore. Skip secondary channel\n",
> -			 ses->server->hostname);
> +		cifs_server_dbg(VFS,
> +				"server does not support multichannel anymore. Skip secondary channel\n");
>   
>   		spin_lock(&ses->chan_lock);
>   		chan_index = cifs_ses_get_chan_index(ses, server);

-- 
Frank Sorenson
sorenson@redhat.com
Principal Software Maintenance Engineer, filesystems
Red Hat

Re: [PATCH] smb: client: fix potential use-after-free with TCP_Server_Info->hostname

[Namjae Jeon]

> diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> index dea05aeb53a1..bb30cfa156aa 100644
> --- a/fs/smb/client/smb2pdu.c
> +++ b/fs/smb/client/smb2pdu.c
> @@ -173,9 +173,8 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
>         unsigned int chan_index;
>
>         if (SERVER_IS_CHAN(server)) {
> -               cifs_dbg(VFS,
> -                       "server %s does not support multichannel anymore. Skip secondary channel\n",
> -                        ses->server->hostname);
> +               cifs_server_dbg(VFS,
> +                               "server does not support multichannel anymore. Skip secondary channel\n");
Since SERVER_IS_CHAN(server) means that server is a secondary channel,
Is it intentional that this now logs the secondary hostname instead of
the primary hostname?

Re: [PATCH] smb: client: fix potential use-after-free with TCP_Server_Info->hostname

[Frank Sorenson]

On 9/17/26 8:47 PM, Namjae Jeon wrote:
>> diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
>> index dea05aeb53a1..bb30cfa156aa 100644
>> --- a/fs/smb/client/smb2pdu.c
>> +++ b/fs/smb/client/smb2pdu.c
>> @@ -173,9 +173,8 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
>>          unsigned int chan_index;
>>
>>          if (SERVER_IS_CHAN(server)) {
>> -               cifs_dbg(VFS,
>> -                       "server %s does not support multichannel anymore. Skip secondary channel\n",
>> -                        ses->server->hostname);
>> +               cifs_server_dbg(VFS,
>> +                               "server does not support multichannel anymore. Skip secondary channel\n");
> Since SERVER_IS_CHAN(server) means that server is a secondary channel,
> Is it intentional that this now logs the secondary hostname instead of
> the primary hostname?

No, not intentional... I missed that server != ses->server at this
point, so the patch is wrong.

Frank

-- 
Frank Sorenson
sorenson@redhat.com
Principal Software Maintenance Engineer, filesystems
Red Hat