From patchwork Thu Nov 10 23:31:23 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pavel Shilovsky X-Patchwork-Id: 9422203 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id EB75C60512 for ; Thu, 10 Nov 2016 23:31:34 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CBDD42981C for ; Thu, 10 Nov 2016 23:31:34 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BEDAA28484; Thu, 10 Nov 2016 23:31:34 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 14A8C28484 for ; Thu, 10 Nov 2016 23:31:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934975AbcKJXbd (ORCPT ); Thu, 10 Nov 2016 18:31:33 -0500 Received: from mail-pf0-f194.google.com ([209.85.192.194]:32882 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934845AbcKJXbc (ORCPT ); Thu, 10 Nov 2016 18:31:32 -0500 Received: by mail-pf0-f194.google.com with SMTP id 144so90429pfv.0 for ; Thu, 10 Nov 2016 15:31:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:subject:date:message-id; bh=w+3+tMqzN2LZe8pidg07vyTyHFaohxowGYSRi/77AWo=; b=tx3wawv8bqZC+IbEwrgkaPAdDWH+TP6quGhk/JD0yB5uR4dKH74VUfjxxnASgGGpaI 69EESsETzLUlJ6MZQPAw2nOGPnqjIUIfN/tjOwnXmqxiX1/pTBM7WfFHpAyo1bomAyYj dkpCrUv5cyW+vQ4rdH0I47NG9zmspifFzkt4I0uEoYhUQaVY3BsW/pZyNOx3WR++R8CP Qm+8JK+r7yx73oYgrsTONr6nlP0y25QU7UvJ2gWEO4qayO2tPjVVaGDPa4ifb9m+ttfg BSIgricpccDfvB4ZCKYappJr/BCaEiBeBeny1h7e+m0gyWyBsd82dPnrwvHlAC00kq+v FQiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id; bh=w+3+tMqzN2LZe8pidg07vyTyHFaohxowGYSRi/77AWo=; b=Btzwr0NUpZUaUKntZ2tsgjQOmCK22axk/6LcFghibs6z4TJsgVpFZT9X9qvwTtEp0J RTOD/1jy40+rbs61R2w4DaXgkOKxK00IeItXW0Bz1IT8zSZfgaVhS0LiBqULRCG7zlAK no5U5ZZLifOmZnnecAJTl5Nn5KQuq5L21SmIQDes6xlvPqEjTRFw/8o4guhztpPBHHBC IpRXWMcp2LgUei5Y8E/A9i66G4NVj02lQ7o6qMc5fyP1To4HgzcMHKfp/wHFgdQWLg+2 cWVpJ6Pfnz5EHh3wTDByKjFb9B32atyjQVHVY+5Xw06Xox4LQAc/W7gwzp1MzG+kz6Ia +ksA== X-Gm-Message-State: ABUngvfUjL/yDYJsEix+q/Fg02w1YhNtvbjxk+6egdsPh+Njk50HYAQONQMnLBOLKVf7Zg== X-Received: by 10.99.114.15 with SMTP id n15mr37387141pgc.7.1478820690461; Thu, 10 Nov 2016 15:31:30 -0800 (PST) Received: from ubuntu-vm.corp.microsoft.com ([2001:4898:80e8:9::63b]) by smtp.gmail.com with ESMTPSA id bc9sm9747140pad.34.2016.11.10.15.31.29 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 10 Nov 2016 15:31:29 -0800 (PST) From: Pavel Shilovsky X-Google-Original-From: Pavel Shilovsky To: linux-cifs@vger.kernel.org Subject: [PATCH v2] CIFS: Fix a possible memory corruption during reconnect Date: Thu, 10 Nov 2016 15:31:23 -0800 Message-Id: <1478820683-2954-1-git-send-email-pshilov@microsoft.com> X-Mailer: git-send-email 2.7.4 Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP We can not unlock/lock cifs_tcp_ses_lock while walking through ses and tcon lists because it can corrupt list iterator pointers and a tcon structure can be released if we don't hold an extra reference. Fix it by moving a reconnect process to a separate delayed work and acquiring a reference to every tcon that needs to be reconnected. Also do not send an echo request on newly established connections. CC: Stable Signed-off-by: Pavel Shilovsky Tested-by: Aurelien Aptel --- fs/cifs/cifsglob.h | 3 +++ fs/cifs/cifsproto.h | 3 +++ fs/cifs/connect.c | 34 +++++++++++++++++++----- fs/cifs/smb2pdu.c | 75 ++++++++++++++++++++++++++++++++++++----------------- fs/cifs/smb2proto.h | 1 + 5 files changed, 85 insertions(+), 31 deletions(-) diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index 1f17f6b..6dcefc8 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -632,6 +632,7 @@ struct TCP_Server_Info { bool sec_mskerberos; /* supports legacy MS Kerberos */ bool large_buf; /* is current buffer large? */ struct delayed_work echo; /* echo ping workqueue job */ + struct delayed_work reconnect; /* reconnect workqueue job */ char *smallbuf; /* pointer to current "small" buffer */ char *bigbuf; /* pointer to current "big" buffer */ unsigned int total_read; /* total amount of data read in this pass */ @@ -648,6 +649,7 @@ struct TCP_Server_Info { __u8 preauth_hash[512]; #endif /* CONFIG_CIFS_SMB2 */ unsigned long echo_interval; + struct mutex reconnect_mutex; /* prevent simultaneous reconnects */ }; static inline unsigned int @@ -850,6 +852,7 @@ struct cifs_tcon { struct list_head tcon_list; int tc_count; struct list_head openFileList; + struct list_head rlist; /* reconnect list */ spinlock_t open_file_lock; /* protects list above */ struct cifs_ses *ses; /* pointer to session associated with */ char treeName[MAX_TREE_SIZE + 1]; /* UNC name of resource in ASCII */ diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index ced0e42..cd8025a 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -206,6 +206,9 @@ extern void cifs_add_pending_open_locked(struct cifs_fid *fid, struct tcon_link *tlink, struct cifs_pending_open *open); extern void cifs_del_pending_open(struct cifs_pending_open *open); +extern void cifs_put_tcp_session(struct TCP_Server_Info *server, + int from_reconnect); +extern void cifs_put_tcon(struct cifs_tcon *tcon); #if IS_ENABLED(CONFIG_CIFS_DFS_UPCALL) extern void cifs_dfs_release_automount_timer(void); diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 4547aed..75503af 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -52,6 +52,9 @@ #include "nterr.h" #include "rfc1002pdu.h" #include "fscache.h" +#ifdef CONFIG_CIFS_SMB2 +#include "smb2proto.h" +#endif #define CIFS_PORT 445 #define RFC1001_PORT 139 @@ -2100,8 +2103,8 @@ cifs_find_tcp_session(struct smb_vol *vol) return NULL; } -static void -cifs_put_tcp_session(struct TCP_Server_Info *server) +void +cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect) { struct task_struct *task; @@ -2118,6 +2121,19 @@ cifs_put_tcp_session(struct TCP_Server_Info *server) cancel_delayed_work_sync(&server->echo); +#ifdef CONFIG_CIFS_SMB2 + if (from_reconnect) + /* + * Avoid deadlock here: reconnect work calls + * cifs_put_tcp_session() at its end. Need to be sure + * that reconnect work does nothing with server pointer after + * that step. + */ + cancel_delayed_work(&server->reconnect); + else + cancel_delayed_work_sync(&server->reconnect); +#endif + spin_lock(&GlobalMid_Lock); server->tcpStatus = CifsExiting; spin_unlock(&GlobalMid_Lock); @@ -2171,6 +2187,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info) init_waitqueue_head(&tcp_ses->request_q); INIT_LIST_HEAD(&tcp_ses->pending_mid_q); mutex_init(&tcp_ses->srv_mutex); + mutex_init(&tcp_ses->reconnect_mutex); memcpy(tcp_ses->workstation_RFC1001_name, volume_info->source_rfc1001_name, RFC1001_NAME_LEN_WITH_NULL); memcpy(tcp_ses->server_RFC1001_name, @@ -2182,6 +2199,9 @@ cifs_get_tcp_session(struct smb_vol *volume_info) INIT_LIST_HEAD(&tcp_ses->tcp_ses_list); INIT_LIST_HEAD(&tcp_ses->smb_ses_list); INIT_DELAYED_WORK(&tcp_ses->echo, cifs_echo_request); +#ifdef CONFIG_CIFS_SMB2 + INIT_DELAYED_WORK(&tcp_ses->reconnect, smb2_reconnect_server); +#endif memcpy(&tcp_ses->srcaddr, &volume_info->srcaddr, sizeof(tcp_ses->srcaddr)); memcpy(&tcp_ses->dstaddr, &volume_info->dstaddr, @@ -2340,7 +2360,7 @@ cifs_put_smb_ses(struct cifs_ses *ses) spin_unlock(&cifs_tcp_ses_lock); sesInfoFree(ses); - cifs_put_tcp_session(server); + cifs_put_tcp_session(server, 0); } #ifdef CONFIG_KEYS @@ -2514,7 +2534,7 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info) mutex_unlock(&ses->session_mutex); /* existing SMB ses has a server reference already */ - cifs_put_tcp_session(server); + cifs_put_tcp_session(server, 0); free_xid(xid); return ses; } @@ -2604,7 +2624,7 @@ cifs_find_tcon(struct cifs_ses *ses, const char *unc) return NULL; } -static void +void cifs_put_tcon(struct cifs_tcon *tcon) { unsigned int xid; @@ -3792,7 +3812,7 @@ cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *volume_info) else if (ses) cifs_put_smb_ses(ses); else - cifs_put_tcp_session(server); + cifs_put_tcp_session(server, 0); bdi_destroy(&cifs_sb->bdi); } @@ -4103,7 +4123,7 @@ cifs_construct_tcon(struct cifs_sb_info *cifs_sb, kuid_t fsuid) ses = cifs_get_smb_ses(master_tcon->ses->server, vol_info); if (IS_ERR(ses)) { tcon = (struct cifs_tcon *)ses; - cifs_put_tcp_session(master_tcon->ses->server); + cifs_put_tcp_session(master_tcon->ses->server, 0); goto out; } diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 5ca5ea46..950dbf7 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1972,6 +1972,54 @@ smb2_echo_callback(struct mid_q_entry *mid) add_credits(server, credits_received, CIFS_ECHO_OP); } +void smb2_reconnect_server(struct work_struct *work) +{ + struct TCP_Server_Info *server = container_of(work, + struct TCP_Server_Info, reconnect.work); + struct cifs_ses *ses; + struct cifs_tcon *tcon, *tcon2; + struct list_head tmp_list; + int tcon_exist = false; + + /* Prevent simultaneous reconnects that can corrupt tcon->rlist list */ + mutex_lock(&server->reconnect_mutex); + + INIT_LIST_HEAD(&tmp_list); + cifs_dbg(FYI, "Need negotiate, reconnecting tcons\n"); + + spin_lock(&cifs_tcp_ses_lock); + list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) { + list_for_each_entry(tcon, &ses->tcon_list, tcon_list) { + if (tcon && tcon->need_reconnect) { + tcon->tc_count++; + list_add_tail(&tcon->rlist, &tmp_list); + tcon_exist = true; + } + } + } + /* + * Get the reference to server struct to be sure that the last call of + * cifs_put_tcon() in the loop below won't release the server pointer. + */ + if (tcon_exist) + server->srv_count++; + + spin_unlock(&cifs_tcp_ses_lock); + + list_for_each_entry_safe(tcon, tcon2, &tmp_list, rlist) { + smb2_reconnect(SMB2_ECHO, tcon); + list_del_init(&tcon->rlist); + cifs_put_tcon(tcon); + } + + cifs_dbg(FYI, "Reconnecting tcons finished\n"); + mutex_unlock(&server->reconnect_mutex); + + /* now we can safely release srv struct */ + if (tcon_exist) + cifs_put_tcp_session(server, 1); +} + int SMB2_echo(struct TCP_Server_Info *server) { @@ -1984,32 +2032,11 @@ SMB2_echo(struct TCP_Server_Info *server) cifs_dbg(FYI, "In echo request\n"); if (server->tcpStatus == CifsNeedNegotiate) { - struct list_head *tmp, *tmp2; - struct cifs_ses *ses; - struct cifs_tcon *tcon; - - cifs_dbg(FYI, "Need negotiate, reconnecting tcons\n"); - spin_lock(&cifs_tcp_ses_lock); - list_for_each(tmp, &server->smb_ses_list) { - ses = list_entry(tmp, struct cifs_ses, smb_ses_list); - list_for_each(tmp2, &ses->tcon_list) { - tcon = list_entry(tmp2, struct cifs_tcon, - tcon_list); - /* add check for persistent handle reconnect */ - if (tcon && tcon->need_reconnect) { - spin_unlock(&cifs_tcp_ses_lock); - rc = smb2_reconnect(SMB2_ECHO, tcon); - spin_lock(&cifs_tcp_ses_lock); - } - } - } - spin_unlock(&cifs_tcp_ses_lock); + /* No need to send echo on newly established connections */ + mod_delayed_work(cifsiod_wq, &server->reconnect, 0); + return rc; } - /* if no session, renegotiate failed above */ - if (server->tcpStatus == CifsNeedNegotiate) - return -EIO; - rc = small_smb2_init(SMB2_ECHO, NULL, (void **)&req); if (rc) return rc; diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h index eb2cde2..f2d511a 100644 --- a/fs/cifs/smb2proto.h +++ b/fs/cifs/smb2proto.h @@ -96,6 +96,7 @@ extern int smb2_open_file(const unsigned int xid, extern int smb2_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock, const unsigned int xid); extern int smb2_push_mandatory_locks(struct cifsFileInfo *cfile); +extern void smb2_reconnect_server(struct work_struct *work); /* * SMB2 Worker functions - most of protocol specific implementation details