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

[Frank Sorenson <sorenson@redhat.com>]

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