Received: by 2002:a05:6a10:206:0:0:0:0 with SMTP id 6csp172880pxj; Tue, 15 Jun 2021 23:10:24 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzeBgh7L9K6YDeSM6uc1iXyE3AlSPIHOT8VXGHDujDEz5OL5k7ykK00HETMkBcD3QPvLVCk X-Received: by 2002:a02:335c:: with SMTP id k28mr2660394jak.4.1623823824639; Tue, 15 Jun 2021 23:10:24 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1623823824; cv=none; d=google.com; s=arc-20160816; b=O+HczM+3tzxMbh0UtUz1qWo+keSJnOX1dDcnIOzjgvJKPDW6aWfWofVUgTMqITGJhl QaAEoOxHANRhdfAAv5kapIubyB6XcHqqyiXljJCA7URq264lVi7rEKbkHsHGgwO3iF7O FbgbKRTo5zoOLy1AHzg7X2cjmjKJ/CK4BlJGVqbYZpxF3hHrbrpNKSui0z3MeGZxzXuf ROD65I6yBUiMX75x//tjhYpehymFutaZTT09cJnulF3x98+E3CjyPazLQoQm6x2UDMjR 7FSul3u0cpZg9fO02XNHkDyM+0h4qsCHqVMWROTJsOBFkDLSaLq0ranxCyNLEFQhI52m KXRw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=KxC5nTXTGywsFfFP80F8r7S66ACFSaP2lRpWZ/+uxEA=; b=UTqfsZE/I8RRf5GQo6Naoo1hNefElqqknEe4qkUqSQzIPxpyjnAzVC+GZvSlxcx61j Z0Xjwslh7h5rKl2IIG9ySJXs12+LpZPwgepfOaZYrOWp2n0QFSL72Y1QHqwr6snXrYn4 Sl2MwqQMSnJ4y0I3p1xnRXLfdaKcSBXN1ovH5lLt1HPNXibY7jMKlf7KWnwZvHeuP4L3 w4Z+LerYlsDW811F3jOOZdm4xPUZ9pfuJvPqJ1c2UrGbTr0pobS5VWVc5Mgfd3YzjzL0 tKJ8GMskH6R39e/JyQaoDKevumvZ7qmAycwgrH78L/xQplCpsjKDSUwZ/AdQXyQVzraB KlPQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=Ceh6VHeS; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id y5si1476534ily.21.2021.06.15.23.10.11; Tue, 15 Jun 2021 23:10:24 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=Ceh6VHeS; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230319AbhFPGLo (ORCPT + 99 others); Wed, 16 Jun 2021 02:11:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52486 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229543AbhFPGLo (ORCPT ); Wed, 16 Jun 2021 02:11:44 -0400 Received: from mail-pl1-x62c.google.com (mail-pl1-x62c.google.com [IPv6:2607:f8b0:4864:20::62c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B8139C061574; Tue, 15 Jun 2021 23:09:37 -0700 (PDT) Received: by mail-pl1-x62c.google.com with SMTP id o21so572595pll.6; Tue, 15 Jun 2021 23:09:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=KxC5nTXTGywsFfFP80F8r7S66ACFSaP2lRpWZ/+uxEA=; b=Ceh6VHeSRbD/KZlHbz/UYOPdumZdASc/KYIx7lG5Qy+5k+C/FuuVr1mu76GKRcVXPq fybyCuUVKKEqkxuTnBLFjrCQ+jCNQZzZhrL3+mz2ivhMjASFH9pZfEOrtIzMki97jZHt FOXALYv8AObdHv5gpgYaxHqmfg98biusEjZ1G6BAu6TgobOnQ3wmWe9pbnvopCNoe192 3SLhihR5lOKoDEluHPDJWBrpl1nbYK1MmF40z8TV1Ba/NdyS3hY6yAKhh+SaY4fItRBh R1c5xXoCaTSWpYVEWlvgco9qsYXuRnzn4bPh9mQS4yfnxSdKvtB0YL48u3BscSE1aVPs KINQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=KxC5nTXTGywsFfFP80F8r7S66ACFSaP2lRpWZ/+uxEA=; b=V0QIa2b0+KZNRDKlhK9Ae/iXv+wJC+YaS6Qe/q2pnL3S5DiE6lTxN1mhjaCm3T581R jWO60PaWdiUiB237NSMfIcfQ/dKJ79qnVhIJ/ogErm+QcyizrTZ+qujn15rRXBaNTmOB i4oyhwBVvJK+/0Fe9oocexD9/qpUdTedhVqhZhRgQiFK7/YKnu6b93gXMiHjn9YEJFo0 Z3T4IDdPe8I/NPnEv4ev/jQKjT+9mkRIXXAARJCuoN19QPEMYn7X7YzQO2T1Mew4nas9 rCE+c0/a9Zi+qn/qWD8RMoMYnru8t/m8MgzCgxqSHnm6K0STIb3Etnw3M0NjuCIQUqL1 UBxg== X-Gm-Message-State: AOAM532W2V4DnwGkHVLQB9TUp1aIBSOQid4Q9dbxVxnDBgnsPe8g3Zrd eqQkWu2x8SLCPVlzUi9dK9zG2yzSZ+d8BCONYCYiG6cb X-Received: by 2002:a17:902:e751:b029:106:65ba:5c80 with SMTP id p17-20020a170902e751b029010665ba5c80mr6054587plf.36.1623823777191; Tue, 15 Jun 2021 23:09:37 -0700 (PDT) MIME-Version: 1.0 References: <20210615164256.173715-1-trbecker@gmail.com> In-Reply-To: <20210615164256.173715-1-trbecker@gmail.com> From: ronnie sahlberg Date: Wed, 16 Jun 2021 16:09:25 +1000 Message-ID: Subject: Re: [PATCH] cifs: retry lookup and readdir when EAGAIN is returned. To: Thiago Rafael Becker Cc: linux-cifs , Steve French , LKML Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Looks good to me. Acked-by: Ronnie Sahlberg On Wed, Jun 16, 2021 at 2:44 AM Thiago Rafael Becker wrote: > > According to the investigation performed by Jacob Shivers at Red Hat, > cifs_lookup and cifs_readdir leak EAGAIN when the user session is > deleted on the server. Fix this issue by implementing a retry with > limits, as is implemented in cifs_revalidate_dentry_attr. > > Reproducer based on the work by Jacob Shivers: > > ~~~ > $ cat readdir-cifs-test.sh > #!/bin/bash > > # Install and configure powershell and sshd on the windows > # server as descibed in > # https://docs.microsoft.com/en-us/windows-server/administration/openssh/openssh_overview > # This script uses expect(1) > > USER=dude > SERVER=192.168.0.2 > RPATH=root > PASS='password' > > function debug_funcs { > for line in $@ ; do > echo "func $line +p" > /sys/kernel/debug/dynamic_debug/control > done > } > > function setup { > echo 1 > /proc/fs/cifs/cifsFYI > debug_funcs wait_for_compound_request \ > smb2_query_dir_first cifs_readdir \ > compound_send_recv cifs_reconnect_tcon \ > generic_ip_connect cifs_reconnect \ > smb2_reconnect_server smb2_reconnect \ > cifs_readv_from_socket cifs_readv_receive > tcpdump -i eth0 -w cifs.pcap host 192.168.2.182 & sleep 5 > dmesg -C > } > > function test_call { > if [[ $1 == 1 ]] ; then > tracer="strace -tt -f -s 4096 -o trace-$(date -Iseconds).txt" > fi > # Change the command here to anything apropriate > $tracer ls $2 > /dev/null > res=$? > if [[ $1 == 1 ]] ; then > if [[ $res == 0 ]] ; then > 1>&2 echo success > else > 1>&2 echo "failure ($res)" > fi > fi > } > > mountpoint /mnt > /dev/null || mount -t cifs -o username=$USER,pass=$PASS //$SERVER/$RPATH /mnt > > test_call 0 /mnt/ > > /usr/bin/expect << EOF > set timeout 60 > > spawn ssh $USER@$SERVER > > expect "yes/no" { > send "yes\r" > expect "*?assword" { send "$PASS\r" } > } "*?assword" { send "$PASS\r" } > > expect ">" { send "powershell close-smbsession -force\r" } > expect ">" { send "exit\r" } > expect eof > EOF > > sysctl -w vm.drop_caches=2 > /dev/null > sysctl -w vm.drop_caches=2 > /dev/null > > setup > > test_call 1 /mnt/ > ~~~ > > Signed-off-by: Thiago Rafael Becker > --- > fs/cifs/dir.c | 4 ++++ > fs/cifs/smb2ops.c | 5 +++++ > 2 files changed, 9 insertions(+) > > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c > index 6bcd3e8f7cda..7c641f9a3dac 100644 > --- a/fs/cifs/dir.c > +++ b/fs/cifs/dir.c > @@ -630,6 +630,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry, > struct inode *newInode = NULL; > const char *full_path; > void *page; > + int retry_count = 0; > > xid = get_xid(); > > @@ -673,6 +674,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry, > cifs_dbg(FYI, "Full path: %s inode = 0x%p\n", > full_path, d_inode(direntry)); > > +again: > if (pTcon->posix_extensions) > rc = smb311_posix_get_inode_info(&newInode, full_path, parent_dir_inode->i_sb, xid); > else if (pTcon->unix_ext) { > @@ -687,6 +689,8 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry, > /* since paths are not looked up by component - the parent > directories are presumed to be good here */ > renew_parental_timestamps(direntry); > + } else if (rc == -EAGAIN && retry_count++ < 10) { > + goto again; > } else if (rc == -ENOENT) { > cifs_set_time(direntry, jiffies); > newInode = NULL; > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index 21ef51d338e0..d241e6af8fe4 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -2325,6 +2325,7 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon, > struct smb2_query_directory_rsp *qd_rsp = NULL; > struct smb2_create_rsp *op_rsp = NULL; > struct TCP_Server_Info *server = cifs_pick_channel(tcon->ses); > + int retry_count = 0; > > utf16_path = cifs_convert_path_to_utf16(path, cifs_sb); > if (!utf16_path) > @@ -2372,10 +2373,14 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon, > > smb2_set_related(&rqst[1]); > > +again: > rc = compound_send_recv(xid, tcon->ses, server, > flags, 2, rqst, > resp_buftype, rsp_iov); > > + if (rc == -EAGAIN && retry_count++ < 10) > + goto again; > + > /* If the open failed there is nothing to do */ > op_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base; > if (op_rsp == NULL || op_rsp->sync_hdr.Status != STATUS_SUCCESS) { > -- > 2.31.1 >