From patchwork Fri Sep 27 19:30:50 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Cabaj X-Patchwork-Id: 1990405 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=185.125.189.65; helo=lists.ubuntu.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=patchwork.ozlabs.org) Received: from lists.ubuntu.com (lists.ubuntu.com [185.125.189.65]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4XFgYy1j0vz1xtK for ; Sat, 28 Sep 2024 05:31:17 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=lists.ubuntu.com) by lists.ubuntu.com with esmtp (Exim 4.86_2) (envelope-from ) id 1suGgA-0006wT-LN; Fri, 27 Sep 2024 19:31:02 +0000 Received: from smtp-relay-internal-1.internal ([10.131.114.114] helo=smtp-relay-internal-1.canonical.com) by lists.ubuntu.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1suGg6-0006vU-E0 for kernel-team@lists.ubuntu.com; Fri, 27 Sep 2024 19:30:58 +0000 Received: from mail-io1-f72.google.com (mail-io1-f72.google.com [209.85.166.72]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-1.canonical.com (Postfix) with ESMTPS id CD8F13F180 for ; Fri, 27 Sep 2024 19:30:57 +0000 (UTC) Received: by mail-io1-f72.google.com with SMTP id ca18e2360f4ac-82aa94d4683so287632439f.3 for ; Fri, 27 Sep 2024 12:30:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727465457; x=1728070257; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=edxQ/lewty7YO+StQyFZKVA9SYDIklFjKrumKkgpo60=; b=F2V5PE3EtxbfgBVo5EygI87ymJttFrsjbN/k/4XRUZLGwVW2Kwsd+t+LYoetMFTLNy Sg1Gh+0nDaLFFdVElvr3iKcWPB0TZYPUILAHA02ao9VGhHxQJ2Vd6PpDL54weVBa5O/q HP2ie/zaxAsuJjMa5VkcMtgeVeHoFWEGUFbD1wkHnj0i4a9YYLnqrNFaV/nIRIDnJ3ph Ta30HTo1n2TI5oyaKx30vu+QlLd4lPTFIxK5bDgRizPSUnsTS8duo6E0GtiBTfm7Omo8 DXibSbJiGZZnhoYYwrqiAJXVkKex5Rcp5v9GbjFCGHYhkgDg+2CL1EM2uT66ENwfT1Xj mYIQ== X-Gm-Message-State: AOJu0YziTqWPCw8gxDvVSsnX5TAaL496AVF6IKtca7f4yWmb24EGFynT GVtb+37L9qdYlPlv7JxmE+0KBvvee4fFbEHC2cIkb8gg7N4BqS1zZe3ovZt7t8Kd8Y+P2UZt/4y xBTJzl/bl9dMc5AXQXqzh5PUon9Ehqw/JnNCMbqt7oYnTPyYsgZNNB6N4AlZVhToI/4fpdVnsBV gtLp9ydUoovw== X-Received: by 2002:a05:6e02:1a2e:b0:3a1:a20f:c09c with SMTP id e9e14a558f8ab-3a3451b9f82mr43148995ab.22.1727465456766; Fri, 27 Sep 2024 12:30:56 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHPN5CbehguVWCygx9wDt4YNvYsN5jsVA1F02y7hydkfdxlNKcD/jIn3UQSmmEB1tB3cuUpnA== X-Received: by 2002:a05:6e02:1a2e:b0:3a1:a20f:c09c with SMTP id e9e14a558f8ab-3a3451b9f82mr43148795ab.22.1727465456392; Fri, 27 Sep 2024 12:30:56 -0700 (PDT) Received: from smtp.gmail.com (h208-73-92-250.mdtnwi.broadband.dynamic.tds.net. [208.73.92.250]) by smtp.gmail.com with ESMTPSA id e9e14a558f8ab-3a344d82ac7sm7710305ab.23.2024.09.27.12.30.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 Sep 2024 12:30:56 -0700 (PDT) From: John Cabaj To: kernel-team@lists.ubuntu.com Subject: [SRU][jammy:azure][PATCH 1/1] cifs: prevent updating file size from server if we have a read/write lease Date: Fri, 27 Sep 2024 14:30:50 -0500 Message-ID: <20240927193051.1552498-2-john.cabaj@canonical.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240927193051.1552498-1-john.cabaj@canonical.com> References: <20240927193051.1552498-1-john.cabaj@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Bharath SM BugLink: https://bugs.launchpad.net/bugs/2071744 In cases of large directories, the readdir operation may span multiple round trips to retrieve contents. This introduces a potential race condition in case of concurrent write and readdir operations. If the readdir operation initiates before a write has been processed by the server, it may update the file size attribute to an older value. Address this issue by avoiding file size updates from readdir when we have read/write lease. Scenario: 1) process1: open dir xyz 2) process1: readdir instance 1 on xyz 3) process2: create file.txt for write 4) process2: write x bytes to file.txt 5) process2: close file.txt 6) process2: open file.txt for read 7) process1: readdir 2 - overwrites file.txt inode size to 0 8) process2: read contents of file.txt - bug, short read with 0 bytes Cc: stable@vger.kernel.org Reviewed-by: Shyam Prasad N Signed-off-by: Bharath SM Signed-off-by: Steve French (backported from commit e4b61f3b1c67f5068590965f64ea6e8d5d5bd961) [john-cabaj: context changes between the 6.9 release and 5.15 kernel] Signed-off-by: John Cabaj --- fs/cifs/cifsproto.h | 6 ++++-- fs/cifs/file.c | 8 +++++--- fs/cifs/inode.c | 17 +++++++++-------- fs/cifs/readdir.c | 2 +- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index 81b559f3b182..0d2d562e893a 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -143,7 +143,8 @@ extern int cifs_reconnect(struct TCP_Server_Info *server, extern int checkSMB(char *buf, unsigned int len, struct TCP_Server_Info *srvr); extern bool is_valid_oplock_break(char *, struct TCP_Server_Info *); extern bool backup_cred(struct cifs_sb_info *); -extern bool is_size_safe_to_change(struct cifsInodeInfo *, __u64 eof); +extern bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 eof, + bool from_readdir); extern void cifs_update_eof(struct cifsInodeInfo *cifsi, loff_t offset, unsigned int bytes_written); extern struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *, int); @@ -200,7 +201,8 @@ extern void cifs_unix_basic_to_fattr(struct cifs_fattr *fattr, struct cifs_sb_info *cifs_sb); extern void cifs_dir_info_to_fattr(struct cifs_fattr *, FILE_DIRECTORY_INFO *, struct cifs_sb_info *); -extern int cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr); +extern int cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr, + bool from_readdir); extern struct inode *cifs_iget(struct super_block *sb, struct cifs_fattr *fattr); diff --git a/fs/cifs/file.c b/fs/cifs/file.c index e2174842bd4b..50ab5d24fbb1 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -200,7 +200,7 @@ int cifs_posix_open(const char *full_path, struct inode **pinode, } } else { cifs_revalidate_mapping(*pinode); - rc = cifs_fattr_to_inode(*pinode, &fattr); + rc = cifs_fattr_to_inode(*pinode, &fattr, false); } posix_open_ret: @@ -4986,12 +4986,14 @@ static int is_inode_writable(struct cifsInodeInfo *cifs_inode) refreshing the inode only on increases in the file size but this is tricky to do without racing with writebehind page caching in the current Linux kernel design */ -bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file) +bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file, + bool from_readdir) { if (!cifsInode) return true; - if (is_inode_writable(cifsInode)) { + if (is_inode_writable(cifsInode) || + ((cifsInode->oplock & CIFS_CACHE_RW_FLG) != 0 && from_readdir)) { /* This inode is open for write at least once */ struct cifs_sb_info *cifs_sb; diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 7d7d4d250862..d0b3d53843c6 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -145,7 +145,8 @@ cifs_nlink_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr) /* populate an inode with info from a cifs_fattr struct */ int -cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr) +cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr, + bool from_readdir) { struct cifsInodeInfo *cifs_i = CIFS_I(inode); struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb); @@ -196,7 +197,7 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr) * Can't safely change the file size here if the client is writing to * it due to potential races. */ - if (is_size_safe_to_change(cifs_i, fattr->cf_eof)) { + if (is_size_safe_to_change(cifs_i, fattr->cf_eof, from_readdir)) { i_size_write(inode, fattr->cf_eof); /* @@ -374,7 +375,7 @@ cifs_get_file_info_unix(struct file *filp) } else goto cifs_gfiunix_out; - rc = cifs_fattr_to_inode(inode, &fattr); + rc = cifs_fattr_to_inode(inode, &fattr, false); cifs_gfiunix_out: free_xid(xid); @@ -453,7 +454,7 @@ int cifs_get_inode_info_unix(struct inode **pinode, } /* if filetype is different, return error */ - rc = cifs_fattr_to_inode(*pinode, &fattr); + rc = cifs_fattr_to_inode(*pinode, &fattr, false); } cgiiu_exit: @@ -827,7 +828,7 @@ cifs_get_file_info(struct file *filp) fattr.cf_uniqueid = CIFS_I(inode)->uniqueid; fattr.cf_flags |= CIFS_FATTR_NEED_REVAL; /* if filetype is different, return error */ - rc = cifs_fattr_to_inode(inode, &fattr); + rc = cifs_fattr_to_inode(inode, &fattr, false); cgfi_exit: cifs_free_open_info(&data); free_xid(xid); @@ -1146,7 +1147,7 @@ int cifs_get_inode_info(struct inode **inode, const char *full_path, goto out; } /* if filetype is different, return error */ - rc = cifs_fattr_to_inode(*inode, &fattr); + rc = cifs_fattr_to_inode(*inode, &fattr, false); } out: cifs_buf_release(smb1_backup_rsp_buf); @@ -1250,7 +1251,7 @@ smb311_posix_get_inode_info(struct inode **inode, } /* if filetype is different, return error */ - rc = cifs_fattr_to_inode(*inode, &fattr); + rc = cifs_fattr_to_inode(*inode, &fattr, false); } out: cifs_put_tlink(tlink); @@ -1347,7 +1348,7 @@ cifs_iget(struct super_block *sb, struct cifs_fattr *fattr) } /* can't fail - see cifs_find_inode() */ - cifs_fattr_to_inode(inode, fattr); + cifs_fattr_to_inode(inode, fattr, false); if (sb->s_flags & SB_NOATIME) inode->i_flags |= S_NOATIME | S_NOCMTIME; if (inode->i_state & I_NEW) { diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c index ef638086d734..81856c18e3b0 100644 --- a/fs/cifs/readdir.c +++ b/fs/cifs/readdir.c @@ -107,7 +107,7 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name, /* update inode in place * if both i_ino and i_mode didn't change */ if (CIFS_I(inode)->uniqueid == fattr->cf_uniqueid && - cifs_fattr_to_inode(inode, fattr) == 0) { + cifs_fattr_to_inode(inode, fattr, true) == 0) { dput(dentry); return; }