From patchwork Tue Jan 24 09:57:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Sachin Prabhu X-Patchwork-Id: 9534699 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 2D7F460434 for ; Tue, 24 Jan 2017 09:58:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 20BAC25D99 for ; Tue, 24 Jan 2017 09:58:09 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 156B726B41; Tue, 24 Jan 2017 09:58:09 +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.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_TVD_MIME_EPI 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 7A03926907 for ; Tue, 24 Jan 2017 09:58:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750751AbdAXJ6H (ORCPT ); Tue, 24 Jan 2017 04:58:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37072 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750736AbdAXJ6G (ORCPT ); Tue, 24 Jan 2017 04:58:06 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0D5FF81241; Tue, 24 Jan 2017 09:58:03 +0000 (UTC) Received: from sprabhu-lp ([10.76.1.79]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0O9w019030705; Tue, 24 Jan 2017 04:58:01 -0500 Message-ID: <1485251879.17488.14.camel@redhat.com> Subject: Re: Linux CIFS client module: login rate limiting From: Sachin Prabhu To: Steve French , Valentin Hilbig Cc: "linux-cifs@vger.kernel.org" In-Reply-To: <1485169046.17488.5.camel@redhat.com> References: <58806F39.9010801@muenchen.de> <1485169046.17488.5.camel@redhat.com> Date: Tue, 24 Jan 2017 15:27:59 +0530 Mime-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Tue, 24 Jan 2017 09:58:03 +0000 (UTC) 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 On Mon, 2017-01-23 at 16:27 +0530, Sachin Prabhu wrote: > On Fri, 2017-01-20 at 15:30 -0600, Steve French wrote: > > A couple quick questions: > > 1) I would not expect "hard" vs "soft" mount option makes no > > difference here, but just doublechecking > > 2) How does smb2 reconnect behave in the same scenario (because we > > prefer smb3 to be used if the server is non-Samba)? > > > > Looks like a fix is doable - see line 1464-1465 of fs/cifs/sess.c > > > >     while (sess_data->func) > >         sess_data->func(sess_data); > > > > looking at cifs_reconnect in the case where the ip address is not > > available we wait 3 seconds (if needed to retry), and when that > > succeeds we schedule delayed work to issue an "echo" (see > > cifs_reconnect) and then as we do cifs_reconnect_tcon we could wait > > up > > to 10 seconds at a time for the socket to come back. If socket is > > ok > > we do a negotiate protocol which is not necessarily retried on > > failure > > (depending on the request it can return EAGAIN - e.g. > > read/write/lock/close).  If the negprot succeeds we get to your > > case > > where we call cifs_setup_session in fs/cifs/connect.c which calls > > CIFS_SessSetup (in fs/cifs/sess.c) which looks like it will loop on > > the sessionsetup retry for the cifs case - which should as you note > > rate limit (especially on bad password case). > > > > I also would like Sachin's feedback as he made some significant > > cleanup of session establishment for cifs and rewrote this - wanted > > to > > see if he wanted to move the throttling of retries differently > > I think the suggestion is perfectly valid and would be a nice > addition > to the cifs module. Maybe a better place to add this change would be > at > > cifs_reconnect_tcon() > { > .. >         mutex_lock(&ses->session_mutex); >         rc = cifs_negotiate_protocol(0, ses); >         if (rc == 0 && ses->need_reconnect) >                 rc = cifs_setup_session(0, ses, nls_codepage); > .. > } > Where in case of EACCES, we can setup a delayed work to unlock ses- > > session_mutex set to run after the required interval. > Having given it another look, since it is unlikely to recover automatically, I think it is better to cache the lookup and return the cached lookup as long as the cache is still valid. I am also in favour of a longer cache interval. Attached is a patch which can work in this case. I use a cache interval of 10 seconds which can be extended further. From 7ca9125be5522679c777a8e9a27a0af22a3d273d Mon Sep 17 00:00:00 2001 From: Sachin Prabhu Date: Tue, 24 Jan 2017 12:43:03 +0530 Subject: [PATCH] cifs: Cache Access denied errors when reconnecting If he account credentials on a mounted share is changed while still mounted, remounts will fail with -EACCES. Since a new session setup call is made every time an attempt is made to access this share, a large number of failed session setup calls are made. This causes problems with certain server setups which consider it as an attack on the account and block further access from the account. To avoid this, cache all -EACCES errors and avoid this problem. Signed-off-by: Sachin Prabhu --- fs/cifs/cifsglob.h | 4 ++++ fs/cifs/cifssmb.c | 20 ++++++++++++++++++-- fs/cifs/connect.c | 14 ++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index 7ea8a33..3c7c0c6 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -75,6 +75,8 @@ #define SMB_ECHO_INTERVAL_MAX 600 #define SMB_ECHO_INTERVAL_DEFAULT 60 +#define SMB_NEGATIVE_CACHE_INTERVAL 10 + /* * Default number of credits to keep available for SMB3. * This value is chosen somewhat arbitrarily. The Windows client @@ -832,6 +834,8 @@ struct cifs_ses { bool sign; /* is signing required? */ bool need_reconnect:1; /* connection reset, uid now invalid */ bool domainAuto:1; + bool cached_rc; + struct delayed_work clear_cached_rc; #ifdef CONFIG_CIFS_SMB2 __u16 session_flags; __u8 smb3signingkey[SMB3_SIGN_KEY_SIZE]; diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index b472618..2196d16 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -179,8 +179,24 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) */ mutex_lock(&ses->session_mutex); rc = cifs_negotiate_protocol(0, ses); - if (rc == 0 && ses->need_reconnect) - rc = cifs_setup_session(0, ses, nls_codepage); + if (rc) { + mutex_unlock(&ses->session_mutex); + goto out; + } + + if (ses->need_reconnect) { + if (ses->cached_rc) { + rc = ses->cached_rc; + } else { + rc = cifs_setup_session(0, ses, nls_codepage); + if (rc == -EACCES) { + queue_delayed_work(cifsiod_wq, + &ses->clear_cached_rc, + SMB_NEGATIVE_CACHE_INTERVAL * HZ); + ses->cached_rc = rc; + } + } + } /* do we need to reconnect tcon? */ if (rc || !tcon->need_reconnect) { diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 35ae49e..f82b280 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -2375,6 +2375,7 @@ cifs_put_smb_ses(struct cifs_ses *ses) list_del_init(&ses->smb_ses_list); spin_unlock(&cifs_tcp_ses_lock); + cancel_delayed_work_sync(&ses->clear_cached_rc); sesInfoFree(ses); cifs_put_tcp_session(server, 0); } @@ -2510,6 +2511,16 @@ cifs_set_cifscreds(struct smb_vol *vol __attribute__((unused)), } #endif /* CONFIG_KEYS */ +static void clear_cached_rc(struct work_struct *work) +{ + struct cifs_ses *ses = container_of(work, struct cifs_ses, + clear_cached_rc.work); + + mutex_lock(&ses->session_mutex); + ses->cached_rc = 0; + mutex_unlock(&ses->session_mutex); +} + static struct cifs_ses * cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info) { @@ -2592,6 +2603,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info) ses->sectype = volume_info->sectype; ses->sign = volume_info->sign; + ses->cached_rc = 0; + INIT_DELAYED_WORK(&ses->clear_cached_rc, clear_cached_rc); + mutex_lock(&ses->session_mutex); rc = cifs_negotiate_protocol(xid, ses); if (!rc) -- 2.9.3