cifs: fix potential use-after-free bugs in TCP_Server_Info::hostname
authorPaulo Alcantara <pc@manguebit.com>
Fri, 21 Apr 2023 18:52:32 +0000 (15:52 -0300)
committerSteve French <stfrench@microsoft.com>
Thu, 1 Jun 2023 04:45:56 +0000 (23:45 -0500)
TCP_Server_Info::hostname may be updated once or many times during
reconnect, so protect its access outside reconnect path as well and
then prevent any potential use-after-free bugs.

Cc: stable@vger.kernel.org
Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
fs/cifs/cifs_debug.c
fs/cifs/cifs_debug.h
fs/cifs/connect.c
fs/cifs/sess.c

index e9c8c088d948ccb44fa110f64ab99729a50d1e2f..d4ed200a9471492e8e171019d45619b7c599b22a 100644 (file)
@@ -280,8 +280,10 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
                seq_printf(m, "\n%d) ConnectionId: 0x%llx ",
                        c, server->conn_id);
 
+               spin_lock(&server->srv_lock);
                if (server->hostname)
                        seq_printf(m, "Hostname: %s ", server->hostname);
+               spin_unlock(&server->srv_lock);
 #ifdef CONFIG_CIFS_SMB_DIRECT
                if (!server->rdma)
                        goto skip_rdma;
@@ -623,10 +625,13 @@ static int cifs_stats_proc_show(struct seq_file *m, void *v)
                                server->fastest_cmd[j],
                                server->slowest_cmd[j]);
                for (j = 0; j < NUMBER_OF_SMB2_COMMANDS; j++)
-                       if (atomic_read(&server->smb2slowcmd[j]))
+                       if (atomic_read(&server->smb2slowcmd[j])) {
+                               spin_lock(&server->srv_lock);
                                seq_printf(m, "  %d slow responses from %s for command %d\n",
                                        atomic_read(&server->smb2slowcmd[j]),
                                        server->hostname, j);
+                               spin_unlock(&server->srv_lock);
+                       }
 #endif /* STATS2 */
                list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
                        list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
index d44808263cfba74fb8aa299f9682e902e19c6191..ce5cfd236fdb8add8b1f13c19181781e2069d173 100644 (file)
@@ -81,19 +81,19 @@ do {                                                                        \
 
 #define cifs_server_dbg_func(ratefunc, type, fmt, ...)                 \
 do {                                                                   \
-       const char *sn = "";                                            \
-       if (server && server->hostname)                                 \
-               sn = server->hostname;                                  \
+       spin_lock(&server->srv_lock);                                   \
        if ((type) & FYI && cifsFYI & CIFS_INFO) {                      \
                pr_debug_ ## ratefunc("%s: \\\\%s " fmt,                \
-                                     __FILE__, sn, ##__VA_ARGS__);     \
+                                     __FILE__, server->hostname,       \
+                                     ##__VA_ARGS__);                   \
        } else if ((type) & VFS) {                                      \
                pr_err_ ## ratefunc("VFS: \\\\%s " fmt,                 \
-                                   sn, ##__VA_ARGS__);                 \
+                                   server->hostname, ##__VA_ARGS__);   \
        } else if ((type) & NOISY && (NOISY != 0)) {                    \
                pr_debug_ ## ratefunc("\\\\%s " fmt,                    \
-                                     sn, ##__VA_ARGS__);               \
+                                     server->hostname, ##__VA_ARGS__); \
        }                                                               \
+       spin_unlock(&server->srv_lock);                                 \
 } while (0)
 
 #define cifs_server_dbg(type, fmt, ...)                                        \
index 53382e78ca97c18eb6c2292507ffb577306f9b3d..b891f47eb1434f34211dd1c7b5a0a939109f1ee6 100644 (file)
@@ -403,8 +403,10 @@ static int __reconnect_target_unlocked(struct TCP_Server_Info *server, const cha
                if (server->hostname != target) {
                        hostname = extract_hostname(target);
                        if (!IS_ERR(hostname)) {
+                               spin_lock(&server->srv_lock);
                                kfree(server->hostname);
                                server->hostname = hostname;
+                               spin_unlock(&server->srv_lock);
                        } else {
                                cifs_dbg(FYI, "%s: couldn't extract hostname or address from dfs target: %ld\n",
                                         __func__, PTR_ERR(hostname));
@@ -561,9 +563,7 @@ cifs_echo_request(struct work_struct *work)
                goto requeue_echo;
 
        rc = server->ops->echo ? server->ops->echo(server) : -ENOSYS;
-       if (rc)
-               cifs_dbg(FYI, "Unable to send echo request to server: %s\n",
-                        server->hostname);
+       cifs_server_dbg(FYI, "send echo request: rc = %d\n", rc);
 
        /* Check witness registrations */
        cifs_swn_check();
@@ -1403,6 +1403,8 @@ static int match_server(struct TCP_Server_Info *server, struct smb3_fs_context *
 {
        struct sockaddr *addr = (struct sockaddr *)&ctx->dstaddr;
 
+       lockdep_assert_held(&server->srv_lock);
+
        if (ctx->nosharesock)
                return 0;
 
@@ -1821,7 +1823,9 @@ cifs_setup_ipc(struct cifs_ses *ses, struct smb3_fs_context *ctx)
        if (tcon == NULL)
                return -ENOMEM;
 
+       spin_lock(&server->srv_lock);
        scnprintf(unc, sizeof(unc), "\\\\%s\\IPC$", server->hostname);
+       spin_unlock(&server->srv_lock);
 
        xid = get_xid();
        tcon->ses = ses;
index d2cbae4b5d211933110f35f5b4fa48a49d0cee0b..335c078c42fb5eabe3ed76a926200ac7d3337678 100644 (file)
@@ -159,6 +159,7 @@ cifs_chan_is_iface_active(struct cifs_ses *ses,
 /* returns number of channels added */
 int cifs_try_adding_channels(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses)
 {
+       struct TCP_Server_Info *server = ses->server;
        int old_chan_count, new_chan_count;
        int left;
        int rc = 0;
@@ -178,16 +179,16 @@ int cifs_try_adding_channels(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses)
                return 0;
        }
 
-       if (ses->server->dialect < SMB30_PROT_ID) {
+       if (server->dialect < SMB30_PROT_ID) {
                spin_unlock(&ses->chan_lock);
                cifs_dbg(VFS, "multichannel is not supported on this protocol version, use 3.0 or above\n");
                return 0;
        }
 
-       if (!(ses->server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) {
+       if (!(server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) {
                ses->chan_max = 1;
                spin_unlock(&ses->chan_lock);
-               cifs_dbg(VFS, "server %s does not support multichannel\n", ses->server->hostname);
+               cifs_server_dbg(VFS, "no multichannel support\n");
                return 0;
        }
        spin_unlock(&ses->chan_lock);