From patchwork Sat Nov 25 20:11:39 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Al Viro X-Patchwork-Id: 169785 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce62:0:b0:403:3b70:6f57 with SMTP id o2csp2136667vqx; Sat, 25 Nov 2023 12:11:57 -0800 (PST) X-Google-Smtp-Source: AGHT+IEW2ywDmf+VKVXOB/G91xRMCHM6MYAj2wGDNGR62iYpsZ2zjUQxkVs5IIg9pk6VKDC7lmRN X-Received: by 2002:a17:90b:4c05:b0:285:9d5d:709d with SMTP id na5-20020a17090b4c0500b002859d5d709dmr4564491pjb.49.1700943117683; Sat, 25 Nov 2023 12:11:57 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700943117; cv=none; d=google.com; s=arc-20160816; b=Cv+oaiAsdKiAaDlXifROzTwg3e0Rk7TZ/1+d+sSFSLRiDwciTmRsyjqEjOojmvD/yE gJwd0i1CRiOUFAFez6kGQxvihXCkzovk1qCdLPuymlnCbxh5eQY/tdvtX0c7JC8ZmKEm mQZdseOXDOX8vMAvcI5k+DdsEvCqzOnKNv7oov/IBCS8WFMjbP4sNJs0H6/TV3XcLtQ9 OuX496YVQnhjaNFCDZI+tGTvJqTAQxPeBk4ImhoOOWNjZoKo0taSSgQ5/GTX97N3lf7w kdcNErjaa5n+T0cBMmpIY/+vJk6hlPUOPP75dLnUjjlI4uhBH74PqiqPIrsS2FB5Q+kJ gZdg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=BlfFBoYl8WljiWMPq8jOBx3qNB8G8Xt5J/at8Fwypa8=; fh=W6BEtqt7iV8phMzKg0C2MCZmXBORzGPTeuIhW3f8iqY=; b=bib2tpRVRizVcIzs2z6EsqNapwVOLvVm/0OyxV8JiH/uwMSLOkKLUymsXpIbJU3BJC qOtYk0GgAyZyZWiwR8m72AxdtFiYjYCCh56mWHz5tJ82Ud/qIC3gbPzKhV4gcLz0/Y0h mS4vtaKuXQKcIYQiQ3FAqs8tbmWLxfhYlypzUo63FdGckjnx39K4mne9Q3b681keEhAr Cm5oeOSFBO4k+TK84AkoHvlnEFLA2xo33cmNJ9cG8wrE16YbMpueOPVXo/V/1cmkRHf1 wOwAnU8S1Rc2ZK0veY287fqvXyFiJUD46OxJtqibBRvDJV8cJ2F6gsfQFOAf/8g93L7v aD7w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=C98y4Itd; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from fry.vger.email (fry.vger.email. [23.128.96.38]) by mx.google.com with ESMTPS id v19-20020a17090a899300b00283bc49ad11si6298088pjn.183.2023.11.25.12.11.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 25 Nov 2023 12:11:57 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=C98y4Itd; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by fry.vger.email (Postfix) with ESMTP id 48BBC8093792; Sat, 25 Nov 2023 12:11:53 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at fry.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230037AbjKYULo (ORCPT + 99 others); Sat, 25 Nov 2023 15:11:44 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46800 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229507AbjKYULm (ORCPT ); Sat, 25 Nov 2023 15:11:42 -0500 Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [IPv6:2a03:a000:7:0:5054:ff:fe1c:15ff]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E0B73E5; Sat, 25 Nov 2023 12:11:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=BlfFBoYl8WljiWMPq8jOBx3qNB8G8Xt5J/at8Fwypa8=; b=C98y4ItdO5oDpwpMe8vrZrbVbM D2Dz8StOcdo6hcr/bl8o1sdV3YDfp9BBpAdKEa0WG3laJ3mWHsIF+NmNBwlXA08WAeHZWrRZKaMpl HwUwkQ0cIBI0YBtSt9VDa0m3YnfEi+6L/jb75SpkZyfYdZIxf9mbMWbw69p3o6QQZ6ndRhvTpwgEI tEa2B42/xzUHEiBtx9oytcQU6RAOYh+ej2q+mq965dl4/O4H+wqMaesBWMIZSP+v5b0Q4nSbnBKW5 00v5nMubhJHMlrbHkp9JL388JjYycImNR3OK1Tx98RhlCrBBiVUO8LYpajLMm1PlFdgfeSf6EojMw ei1spcww==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.96 #2 (Red Hat Linux)) id 1r6z0F-003A4V-1A; Sat, 25 Nov 2023 20:11:47 +0000 From: Al Viro To: linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Mo Zou , Jan Kara , linux-kernel@vger.kernel.org Subject: [PATCH v2 1/9] reiserfs: Avoid touching renamed directory if parent does not change Date: Sat, 25 Nov 2023 20:11:39 +0000 Message-Id: <20231125201147.753695-1-viro@zeniv.linux.org.uk> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20231125201015.GA38156@ZenIV> References: <20231125201015.GA38156@ZenIV> MIME-Version: 1.0 Sender: Al Viro X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Sat, 25 Nov 2023 12:11:53 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783568130685232625 X-GMAIL-MSGID: 1783568130685232625 From: Jan Kara The VFS will not be locking moved directory if its parent does not change. Change reiserfs rename code to avoid touching renamed directory if its parent does not change as without locking that can corrupt the filesystem. Signed-off-by: Jan Kara Signed-off-by: Al Viro --- fs/reiserfs/namei.c | 54 ++++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c index 994d6e6995ab..5996197ba40c 100644 --- a/fs/reiserfs/namei.c +++ b/fs/reiserfs/namei.c @@ -1324,8 +1324,8 @@ static int reiserfs_rename(struct mnt_idmap *idmap, struct inode *old_inode, *new_dentry_inode; struct reiserfs_transaction_handle th; int jbegin_count; - umode_t old_inode_mode; unsigned long savelink = 1; + bool update_dir_parent = false; if (flags & ~RENAME_NOREPLACE) return -EINVAL; @@ -1375,8 +1375,7 @@ static int reiserfs_rename(struct mnt_idmap *idmap, return -ENOENT; } - old_inode_mode = old_inode->i_mode; - if (S_ISDIR(old_inode_mode)) { + if (S_ISDIR(old_inode->i_mode)) { /* * make sure that directory being renamed has correct ".." * and that its new parent directory has not too many links @@ -1389,24 +1388,28 @@ static int reiserfs_rename(struct mnt_idmap *idmap, } } - /* - * directory is renamed, its parent directory will be changed, - * so find ".." entry - */ - dot_dot_de.de_gen_number_bit_string = NULL; - retval = - reiserfs_find_entry(old_inode, "..", 2, &dot_dot_entry_path, + if (old_dir != new_dir) { + /* + * directory is renamed, its parent directory will be + * changed, so find ".." entry + */ + dot_dot_de.de_gen_number_bit_string = NULL; + retval = + reiserfs_find_entry(old_inode, "..", 2, + &dot_dot_entry_path, &dot_dot_de); - pathrelse(&dot_dot_entry_path); - if (retval != NAME_FOUND) { - reiserfs_write_unlock(old_dir->i_sb); - return -EIO; - } + pathrelse(&dot_dot_entry_path); + if (retval != NAME_FOUND) { + reiserfs_write_unlock(old_dir->i_sb); + return -EIO; + } - /* inode number of .. must equal old_dir->i_ino */ - if (dot_dot_de.de_objectid != old_dir->i_ino) { - reiserfs_write_unlock(old_dir->i_sb); - return -EIO; + /* inode number of .. must equal old_dir->i_ino */ + if (dot_dot_de.de_objectid != old_dir->i_ino) { + reiserfs_write_unlock(old_dir->i_sb); + return -EIO; + } + update_dir_parent = true; } } @@ -1486,7 +1489,7 @@ static int reiserfs_rename(struct mnt_idmap *idmap, reiserfs_prepare_for_journal(old_inode->i_sb, new_de.de_bh, 1); - if (S_ISDIR(old_inode->i_mode)) { + if (update_dir_parent) { if ((retval = search_by_entry_key(new_dir->i_sb, &dot_dot_de.de_entry_key, @@ -1534,14 +1537,14 @@ static int reiserfs_rename(struct mnt_idmap *idmap, new_de.de_bh); reiserfs_restore_prepared_buffer(old_inode->i_sb, old_de.de_bh); - if (S_ISDIR(old_inode_mode)) + if (update_dir_parent) reiserfs_restore_prepared_buffer(old_inode-> i_sb, dot_dot_de. de_bh); continue; } - if (S_ISDIR(old_inode_mode)) { + if (update_dir_parent) { if (item_moved(&dot_dot_ih, &dot_dot_entry_path) || !entry_points_to_object("..", 2, &dot_dot_de, old_dir)) { @@ -1559,7 +1562,7 @@ static int reiserfs_rename(struct mnt_idmap *idmap, } } - RFALSE(S_ISDIR(old_inode_mode) && + RFALSE(update_dir_parent && !buffer_journal_prepared(dot_dot_de.de_bh), ""); break; @@ -1592,11 +1595,12 @@ static int reiserfs_rename(struct mnt_idmap *idmap, savelink = new_dentry_inode->i_nlink; } - if (S_ISDIR(old_inode_mode)) { + if (update_dir_parent) { /* adjust ".." of renamed directory */ set_ino_in_dir_entry(&dot_dot_de, INODE_PKEY(new_dir)); journal_mark_dirty(&th, dot_dot_de.de_bh); - + } + if (S_ISDIR(old_inode->i_mode)) { /* * there (in new_dir) was no directory, so it got new link * (".." of renamed directory) From patchwork Sat Nov 25 20:11:40 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Al Viro X-Patchwork-Id: 169786 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce62:0:b0:403:3b70:6f57 with SMTP id o2csp2136727vqx; Sat, 25 Nov 2023 12:12:07 -0800 (PST) X-Google-Smtp-Source: AGHT+IHTjMNrPp7DXTuhbSeLnztfJQPKZpPTiPXFzmZPX7obok5v5BR3NmrMA5kMGbp5OdxEfJyE X-Received: by 2002:a17:90a:d152:b0:280:29df:747e with SMTP id t18-20020a17090ad15200b0028029df747emr14069288pjw.1.1700943127347; Sat, 25 Nov 2023 12:12:07 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700943127; cv=none; d=google.com; s=arc-20160816; b=CwuorSi2UwXkxCz2f4IO+wRyRiKEaxu5P7dXHcYSBWlHNSTnSLJg0n6FYNidMLedMm HZtBrXV6lrlVE0DWB9jEsolx23e8bOzzGB60allLiFPWei7gn+9En7NJvvQkhjXZzAoB BD5sHMHF42Fzyx0b24ez3aw7si+lCZXPJRX4pTLiyCwnPhTmh7tRLAe0tHvCrqnf44iZ NJYmX0ja/Gfhy69SKLXRyRGaPiVr0g+WoDJfvvwI+lyZVP+CaeGbLJp7IhdPavt1fDJw CkoGmPJtfOOqWGTHvn9gf7+Jm4uJzoAZGoKFm03KKijFYB1qaqOommVou/LdPZxrGVfx Sf0g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=74v1Dg7yBE+4rxKY7xG9GKf0VfPvIUvY43ZeQBhdM3Q=; fh=W6BEtqt7iV8phMzKg0C2MCZmXBORzGPTeuIhW3f8iqY=; b=DdJeCBHXbJxe1I18e06bPQCQWb5668jvItF2i2ptvDVo68J+MYoBkfK49heHO9RCRD KA2s+cF9yDKdMuwqeFaZ+kU2Rss0tDRTmRwhBsxbZ6g+MWw7Hd8Ifm9MlAz1b7xTRZfQ Ksb2MxTmkagueejDF+2v5Bl/MaSwJvQw7XfEEbrsILwvEzJPvF27+jDpjjxndI4su78w ZcQWhwPGioM82ORXR+ITvflBNSCaBIMq8oty/6P8Sp2aYpAJfp0G8ktZY5zrzOMyj35r GBXF8flUZhwzB2/AjQditFGB3lE1/9s5fuXGV3yIHTrP1SoBymoZ1sCoT3+o/GtIehbi KxMg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=GCS6BehD; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from groat.vger.email (groat.vger.email. [23.128.96.35]) by mx.google.com with ESMTPS id mn1-20020a17090b188100b00274d6373fe1si6722493pjb.120.2023.11.25.12.12.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 25 Nov 2023 12:12:07 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=GCS6BehD; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by groat.vger.email (Postfix) with ESMTP id 3B9B68096FF2; Sat, 25 Nov 2023 12:12:00 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at groat.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232354AbjKYULq (ORCPT + 99 others); Sat, 25 Nov 2023 15:11:46 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46802 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229505AbjKYULm (ORCPT ); Sat, 25 Nov 2023 15:11:42 -0500 Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [IPv6:2a03:a000:7:0:5054:ff:fe1c:15ff]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0FAE7EE; Sat, 25 Nov 2023 12:11:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=74v1Dg7yBE+4rxKY7xG9GKf0VfPvIUvY43ZeQBhdM3Q=; b=GCS6BehDYyWhwoOEelY07lezeA PEyv/KLN0SbEvpS/e06wueHjustBQp/xYqLZrFBS/3TPvRbzUFDNTyvc9r3pO/IldhUTrK/fQAm4p +JRpbgM8V12YVvLT9sQ4pQQMw7PcU4sYLGXoMBLQV+g8B5RcAmO4wkvuhfobl02fnKp9jxWJcEHRH 0hXoxN7L7opPGgrTTHQkgA/gxwWkaiMbkbZoa98I5L58HPsWiheFeHi/m551I8Y1RVKOvc2lyCH79 N6vLzgPViUpshm3YPaMB1n905dumyHQOaRwZEgO8zjaIm4bmWae9rozlgKIkiIFGSNRhWPURjxbhE edwkpGgg==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.96 #2 (Red Hat Linux)) id 1r6z0F-003A4X-21; Sat, 25 Nov 2023 20:11:47 +0000 From: Al Viro To: linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Mo Zou , Jan Kara , linux-kernel@vger.kernel.org Subject: [PATCH v2 2/9] ocfs2: Avoid touching renamed directory if parent does not change Date: Sat, 25 Nov 2023 20:11:40 +0000 Message-Id: <20231125201147.753695-2-viro@zeniv.linux.org.uk> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20231125201147.753695-1-viro@zeniv.linux.org.uk> References: <20231125201015.GA38156@ZenIV> <20231125201147.753695-1-viro@zeniv.linux.org.uk> MIME-Version: 1.0 Sender: Al Viro X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Sat, 25 Nov 2023 12:12:00 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783568140718121033 X-GMAIL-MSGID: 1783568140718121033 From: Jan Kara The VFS will not be locking moved directory if its parent does not change. Change ocfs2 rename code to avoid touching renamed directory if its parent does not change as without locking that can corrupt the filesystem. Signed-off-by: Jan Kara Signed-off-by: Al Viro --- fs/ocfs2/namei.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c index 814733ba2f4b..9221a33f917b 100644 --- a/fs/ocfs2/namei.c +++ b/fs/ocfs2/namei.c @@ -1336,7 +1336,7 @@ static int ocfs2_rename(struct mnt_idmap *idmap, goto bail; } - if (S_ISDIR(old_inode->i_mode)) { + if (S_ISDIR(old_inode->i_mode) && new_dir != old_dir) { u64 old_inode_parent; update_dot_dot = 1; @@ -1353,8 +1353,7 @@ static int ocfs2_rename(struct mnt_idmap *idmap, goto bail; } - if (!new_inode && new_dir != old_dir && - new_dir->i_nlink >= ocfs2_link_max(osb)) { + if (!new_inode && new_dir->i_nlink >= ocfs2_link_max(osb)) { status = -EMLINK; goto bail; } @@ -1601,6 +1600,9 @@ static int ocfs2_rename(struct mnt_idmap *idmap, mlog_errno(status); goto bail; } + } + + if (S_ISDIR(old_inode->i_mode)) { drop_nlink(old_dir); if (new_inode) { drop_nlink(new_inode); From patchwork Sat Nov 25 20:11:41 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Al Viro X-Patchwork-Id: 169789 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce62:0:b0:403:3b70:6f57 with SMTP id o2csp2136816vqx; Sat, 25 Nov 2023 12:12:17 -0800 (PST) X-Google-Smtp-Source: AGHT+IFD/8ZM0MPmJKpPBN90ZkeIZsWV+XO/TOIy3srZyewRgWvAZydNG1IHdJtTOviWvuJ1khNC X-Received: by 2002:a17:902:dac5:b0:1cf:8a78:2207 with SMTP id q5-20020a170902dac500b001cf8a782207mr7447592plx.3.1700943137593; Sat, 25 Nov 2023 12:12:17 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700943137; cv=none; d=google.com; s=arc-20160816; b=Ig20CQgt6cpAuxdit/BoC2HfnOlTEn32qwTyAOAGfDZfeoDTF3oB6jded7v1sw55wG moObdOdYLyO60Fe21tyA7TUAo3XsCOQOAEjB/7SwstlaAg6OeGQkfvO8iS2MBOzDWrzX RZJqBl/w3C+zw91G+Vj0uk7Dzb1qCXNRjnurOmscVgpC8jmI5Qg06DT4R4UItO0RU7bz L6oJrwCFjTcmZNvukP+4US3bmdSk0YOgZzCW1jGeMZyDZoYhfp1dalv5wXmoy7pzlVaO +b7Iu0NBt7CHKPh00aAx9o5HWBG/Bo1wbaxncN1pHsVchyGF8eUiNDCuUu9OPK0vqqWn Go1g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=vBvpxhxAlJUtQr1e6w1WFsry5arE1IE06RFBsXpoeKs=; fh=W6BEtqt7iV8phMzKg0C2MCZmXBORzGPTeuIhW3f8iqY=; b=w5sgvYqeu7rpOeLmhDh4R4g+8NBiIS908u1HAl44USGNuLLWEPOD3z+BMK5e16Z4XL MVPyWBD3L4+scQgPNhfky8qfGCfGvObPW81Dcm62vRr27OVQKOUPtRU6XNOEdQSMYOum msNsrowmvtMo3wjSSDOb2ws56kgDW1LMr2qxRSdkTWmP+N1dTmUvZUo8WZj1f/JoCXvr 66yCrBP1/aLIGCsVe4JqRJFpG9TbeqGJKcMvSz9EUhjJwFLtJKp/ptOZsnOz/TBJX3dY gbppJMojc0mOqJ4pbjFFOAqROQJ4U2SGcJfLIIxxluOcrbZHek6cZHr+xULcL9vvp3qs d3YQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b="Zk8Q/7Dw"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from morse.vger.email (morse.vger.email. [2620:137:e000::3:1]) by mx.google.com with ESMTPS id b22-20020a170902d89600b001c9af7debb2si6133904plz.525.2023.11.25.12.12.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 25 Nov 2023 12:12:17 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b="Zk8Q/7Dw"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by morse.vger.email (Postfix) with ESMTP id 4A6DE803D836; Sat, 25 Nov 2023 12:12:08 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at morse.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232390AbjKYULv (ORCPT + 99 others); Sat, 25 Nov 2023 15:11:51 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46818 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229511AbjKYULn (ORCPT ); Sat, 25 Nov 2023 15:11:43 -0500 Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [IPv6:2a03:a000:7:0:5054:ff:fe1c:15ff]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 38F3D119; Sat, 25 Nov 2023 12:11:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=vBvpxhxAlJUtQr1e6w1WFsry5arE1IE06RFBsXpoeKs=; b=Zk8Q/7DwcMqd2zJ0tueEOZ9gDL 3yQkORmL6b0al28o8UxxSyrVjIj5JyimsAR9x0xYrxPo/pVxerkkwXJKNuKm/wN5VcYDxm6pQcMHD S/EcduOyxH9bB6Fq7X6v3aHEuZYWDp4WQkra8xjqKRZNFpVvetyPYVk7vhXT2bqMnF0JsjWasJmfW bzrMLn9IYE4Hs9BuSPJo5v54sknQjjEqeSaCEoX/rHP7uDCJD7guolRcclnDbgCdftQZVBpmjU7V0 zNUx9i+HrhO6dkgWGkouLvcFUO86gGvW1LY6FX6nn5Kx9vhXD98jbfjPZRX51jy/LFWir/d2yVnrb SNGVzTNg==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.96 #2 (Red Hat Linux)) id 1r6z0F-003A4b-2X; Sat, 25 Nov 2023 20:11:47 +0000 From: Al Viro To: linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Mo Zou , Jan Kara , linux-kernel@vger.kernel.org Subject: [PATCH v2 3/9] udf_rename(): only access the child content on cross-directory rename Date: Sat, 25 Nov 2023 20:11:41 +0000 Message-Id: <20231125201147.753695-3-viro@zeniv.linux.org.uk> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20231125201147.753695-1-viro@zeniv.linux.org.uk> References: <20231125201015.GA38156@ZenIV> <20231125201147.753695-1-viro@zeniv.linux.org.uk> MIME-Version: 1.0 Sender: Al Viro X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Sat, 25 Nov 2023 12:12:08 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783568151643628391 X-GMAIL-MSGID: 1783568151643628391 We can't really afford locking the source on same-directory rename; currently vfs_rename() tries to do that, but it will have to be changed. The logics in udf_rename() is lazy and goes looking for ".." in source even in same-directory case. It's not hard to get rid of that, leaving that behaviour only for cross-directory case; that VFS can get locks safely (and will keep doing that after the coming changes). Reviewed-by: Jan Kara Signed-off-by: Al Viro --- fs/udf/namei.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/udf/namei.c b/fs/udf/namei.c index 3508ac484da3..fac806a7a8d4 100644 --- a/fs/udf/namei.c +++ b/fs/udf/namei.c @@ -766,7 +766,7 @@ static int udf_rename(struct mnt_idmap *idmap, struct inode *old_dir, struct inode *old_inode = d_inode(old_dentry); struct inode *new_inode = d_inode(new_dentry); struct udf_fileident_iter oiter, niter, diriter; - bool has_diriter = false; + bool has_diriter = false, is_dir = false; int retval; struct kernel_lb_addr tloc; @@ -789,6 +789,9 @@ static int udf_rename(struct mnt_idmap *idmap, struct inode *old_dir, if (!empty_dir(new_inode)) goto out_oiter; } + is_dir = true; + } + if (is_dir && old_dir != new_dir) { retval = udf_fiiter_find_entry(old_inode, &dotdot_name, &diriter); if (retval == -ENOENT) { @@ -878,7 +881,9 @@ static int udf_rename(struct mnt_idmap *idmap, struct inode *old_dir, udf_dir_entry_len(&diriter.fi)); udf_fiiter_write_fi(&diriter, NULL); udf_fiiter_release(&diriter); + } + if (is_dir) { inode_dec_link_count(old_dir); if (new_inode) inode_dec_link_count(new_inode); From patchwork Sat Nov 25 20:11:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Al Viro X-Patchwork-Id: 169788 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce62:0:b0:403:3b70:6f57 with SMTP id o2csp2136791vqx; Sat, 25 Nov 2023 12:12:15 -0800 (PST) X-Google-Smtp-Source: AGHT+IG1E2ptFFcTSfssBTrPfw2yudQjj9Mnen3LhjoctojHep7TPh3ZOrOrqLURUitlbUbLgiv2 X-Received: by 2002:a17:90a:1cd:b0:285:657b:f17a with SMTP id 13-20020a17090a01cd00b00285657bf17amr6504205pjd.15.1700943135104; Sat, 25 Nov 2023 12:12:15 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700943135; cv=none; d=google.com; s=arc-20160816; b=Gjdb6HxuhxhTSBUUPNNswaUQ2YEV85YHbVpF5sKGkvL3mUE6mBc8YR7NdLNDVRlFsc Nie3X2YM4x1g4s5hvKkrf5BBZTDQNgmC7u0hKiKBsM5pDZHSYcTsO9Z6cGqwRZSjTZVc smXrVMwnghldgnuudWJcXiv/gDb21HvLx2f2SPfILQQQhdzRODvaQ+hgTaRpldnT/2fb kWtFQRdYtshCh9bOqir4t3lgP5+JNvtQavFZD7wcicHilH4v/reznU+sd00HNOdlSXZr YLO8IUwBHfXmR3k/LcPARalP42YV4xhlrgnmrmWNe/LJSNd+Tjl0JAj0qdQ2I38Y7b81 3F1g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=T08sUyzdKaybfEkZCNae4ocOGsjYgJj1qnspFJtuRWw=; fh=W6BEtqt7iV8phMzKg0C2MCZmXBORzGPTeuIhW3f8iqY=; b=du2nr06n7s032wJsIzULWdTutwMEgworGy6oAZEZOwRAwwF+V2t4jEK3z0kItdUBo3 D13eBoYwHnxPsZnJiWrk2e5HuYfYi0qu817n4q1QBb9SFMooIDEW1wj+7T5ZY4CJ2OaL hNKZEGVeMIsQkw/v5ozRb0bkLtx4XObJQqK4LlSv/aNTvuMXbl7npPG+XUBsmaYjrE4C 0ua1TXc30MFipPyToMMUltYtgVrUECIEOt8zRRA+adK0asldQjfdEbtfzAxWQxpqonAB 8X/nH+iTAIqdcx/4vt0Ap+6VbuQZursKfigdrSFcKXGhqkaTv/odUGDVqYeLSVBMqny4 QBWA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=fvIi7k46; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id t22-20020a634616000000b005b3bcd9d7f8si6459587pga.808.2023.11.25.12.12.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 25 Nov 2023 12:12:15 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=fvIi7k46; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 0C103804E799; Sat, 25 Nov 2023 12:12:13 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232467AbjKYUL5 (ORCPT + 99 others); Sat, 25 Nov 2023 15:11:57 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46830 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229645AbjKYULn (ORCPT ); Sat, 25 Nov 2023 15:11:43 -0500 Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [IPv6:2a03:a000:7:0:5054:ff:fe1c:15ff]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 657F3127; Sat, 25 Nov 2023 12:11:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=T08sUyzdKaybfEkZCNae4ocOGsjYgJj1qnspFJtuRWw=; b=fvIi7k46RHfJK7RTy5TGozS6Ua XAdcnLEO4sw2DJk9NFpGgleDZrmOVdFvPyviE+d6EH/kn7S6fvGNbfgLJ/E+AjrJwYynr6vqADksd J9xfEMbHbnp2gEWGmlnDhmI7keINg1U/P8lyVAX/PwYPXtFS9lUafIk8HHpfMyeUk8BxfIl1q24xM 2Gg00raJtCN1PVCh4NqJm7w9Xk5hA/bmPxJuopyulFZU4a7JcbcQKMt/v0/LEfK8R/BKH3lPTUI4X 4cKgtDVX69Lq1layYaICqnkS6tTZnUkgqq8DLSxwyESGcx5C/q+gWu/axyLhZdRTAzzR1i3B4dngF 7HDkW05A==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.96 #2 (Red Hat Linux)) id 1r6z0F-003A4f-35; Sat, 25 Nov 2023 20:11:48 +0000 From: Al Viro To: linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Mo Zou , Jan Kara , linux-kernel@vger.kernel.org Subject: [PATCH v2 4/9] ext2: Avoid reading renamed directory if parent does not change Date: Sat, 25 Nov 2023 20:11:42 +0000 Message-Id: <20231125201147.753695-4-viro@zeniv.linux.org.uk> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20231125201147.753695-1-viro@zeniv.linux.org.uk> References: <20231125201015.GA38156@ZenIV> <20231125201147.753695-1-viro@zeniv.linux.org.uk> MIME-Version: 1.0 Sender: Al Viro X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Sat, 25 Nov 2023 12:12:14 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783568148644215296 X-GMAIL-MSGID: 1783568148644215296 From: Jan Kara The VFS will not be locking moved directory if its parent does not change. Change ext2 rename code to avoid reading renamed directory if its parent does not change. Although it is currently harmless it is a bad practice to read directory contents without inode->i_rwsem. Signed-off-by: Jan Kara Signed-off-by: Al Viro --- fs/ext2/namei.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/fs/ext2/namei.c b/fs/ext2/namei.c index 65f702b1da5b..8346ab9534c1 100644 --- a/fs/ext2/namei.c +++ b/fs/ext2/namei.c @@ -325,6 +325,7 @@ static int ext2_rename (struct mnt_idmap * idmap, struct ext2_dir_entry_2 * dir_de = NULL; struct folio * old_folio; struct ext2_dir_entry_2 * old_de; + bool old_is_dir = S_ISDIR(old_inode->i_mode); int err; if (flags & ~RENAME_NOREPLACE) @@ -342,7 +343,7 @@ static int ext2_rename (struct mnt_idmap * idmap, if (IS_ERR(old_de)) return PTR_ERR(old_de); - if (S_ISDIR(old_inode->i_mode)) { + if (old_is_dir && old_dir != new_dir) { err = -EIO; dir_de = ext2_dotdot(old_inode, &dir_folio); if (!dir_de) @@ -354,7 +355,7 @@ static int ext2_rename (struct mnt_idmap * idmap, struct ext2_dir_entry_2 *new_de; err = -ENOTEMPTY; - if (dir_de && !ext2_empty_dir (new_inode)) + if (old_is_dir && !ext2_empty_dir(new_inode)) goto out_dir; new_de = ext2_find_entry(new_dir, &new_dentry->d_name, @@ -368,14 +369,14 @@ static int ext2_rename (struct mnt_idmap * idmap, if (err) goto out_dir; inode_set_ctime_current(new_inode); - if (dir_de) + if (old_is_dir) drop_nlink(new_inode); inode_dec_link_count(new_inode); } else { err = ext2_add_link(new_dentry, old_inode); if (err) goto out_dir; - if (dir_de) + if (old_is_dir) inode_inc_link_count(new_dir); } @@ -387,7 +388,7 @@ static int ext2_rename (struct mnt_idmap * idmap, mark_inode_dirty(old_inode); err = ext2_delete_entry(old_de, old_folio); - if (!err && dir_de) { + if (!err && old_is_dir) { if (old_dir != new_dir) err = ext2_set_link(old_inode, dir_de, dir_folio, new_dir, false); From patchwork Sat Nov 25 20:11:43 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Al Viro X-Patchwork-Id: 169791 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce62:0:b0:403:3b70:6f57 with SMTP id o2csp2136861vqx; Sat, 25 Nov 2023 12:12:25 -0800 (PST) X-Google-Smtp-Source: AGHT+IFOw1txA6kjf3pSorl+VS4DDMaBJgGsfALu1dA+s4hO1n/M6uK39RYVF/as1aZgKNeMeEFY X-Received: by 2002:a17:902:e5c7:b0:1cf:a53f:201e with SMTP id u7-20020a170902e5c700b001cfa53f201emr7774791plf.42.1700943144734; Sat, 25 Nov 2023 12:12:24 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700943144; cv=none; d=google.com; s=arc-20160816; b=DyhRJAjNs9/lwLqiNi+LwC+GNn7ix5fvgjGX2XtJAGX/R7ZzTvNwip8EA6XBHHe+Dj /ubhShnnaiVGdhjCVu3+MVnYQHUTjLferPiFWEgW/b9kPqnrylM5WvIzf3On+Bs+FBT4 3x7zlDZSUNBXCN2aZRSZ+PYawOqiLKyusZ3uUo/TSEHy63hW9e9aH24xuG0U23rlY/90 qUnc1INGmyOWSuZl1SUH5twGLSmnNaLfrDkYsboWAEW8L9kMeCdohmFGd8Z2H8s/yT7B kqCG2rxu7DU5wL02aKpOpYmykStgdGD/O74QGv9RSbWGUCDctXuWL18zz8HhHVuma1+I vSVg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=YsjVgw8ngtuO/NgOh1rsmIhgLaPmuciBhn09an+KSXg=; fh=W6BEtqt7iV8phMzKg0C2MCZmXBORzGPTeuIhW3f8iqY=; b=Qx6cRkeeaI6WxCdxL0NY9kF2l6QlYNUWKrO7TPEPhTr5ABRNQh4LDAR7lcYWdf6pG6 vTWeFBvJrW0w8/EXR+0ElHp7QEVibAc/HYP2AQh4CYVcBMNLgqS7kS4bySF+AF2yp3iZ PeQgDpfHFhETz0bx2oOB192f7MP9EnYBEtEF3nNkfNBpYmBSPOpBon2CGzc+CfhcgC28 MxdJhIN7W36JnyNdnXlNVamuFV3w2C5LYZyxehyhRazMNCebjZd9h6Uq5gJSLY8kHeLt aZjfD2RPDlIRtRHppZKb/n6XG16H8p/mHBYBr0He7pHvwPdtg9PGllhhGMp3idmICEnG Opeg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=mUtvtpLB; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from fry.vger.email (fry.vger.email. [23.128.96.38]) by mx.google.com with ESMTPS id iz15-20020a170902ef8f00b001cfb4d36d44si1869716plb.64.2023.11.25.12.12.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 25 Nov 2023 12:12:24 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=mUtvtpLB; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by fry.vger.email (Postfix) with ESMTP id 3801A80967B7; Sat, 25 Nov 2023 12:12:15 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at fry.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232373AbjKYULs (ORCPT + 99 others); Sat, 25 Nov 2023 15:11:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46832 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229521AbjKYULn (ORCPT ); Sat, 25 Nov 2023 15:11:43 -0500 Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [IPv6:2a03:a000:7:0:5054:ff:fe1c:15ff]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 72460133; Sat, 25 Nov 2023 12:11:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=YsjVgw8ngtuO/NgOh1rsmIhgLaPmuciBhn09an+KSXg=; b=mUtvtpLB5ut74CWBVRvXjm+GX4 4KUxA40HdkBvxubpdJcyMUFFc1lnhXvL/FDfSXzsjS7xiNhlBFMRQ4N3ZaI2lYtd5c6IYLvfgkDwY AmCYER9alg9t5PqTf+3BpBE2/08fWikTs31Finyh4zrfgy14k849DUUfHitjsodNcJ6NK4ra13LXT ZxCRYu7WultWtPr5JYuRmJIGROOCxTHRbqYn5jquf7XxHOtztE1Ph7Ew3wQDKpd4fYMmvvKeOTTID WpeqVITSKJYAHzHkKVkixaMk1KBgDYnMlxFQZ2E2T57We1Yrm61+HQMDZd7xqg0m8kjI1ZYKKdHmo Svm8syEA==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.96 #2 (Red Hat Linux)) id 1r6z0G-003A4j-0K; Sat, 25 Nov 2023 20:11:48 +0000 From: Al Viro To: linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Mo Zou , Jan Kara , linux-kernel@vger.kernel.org Subject: [PATCH v2 5/9] ext4: don't access the source subdirectory content on same-directory rename Date: Sat, 25 Nov 2023 20:11:43 +0000 Message-Id: <20231125201147.753695-5-viro@zeniv.linux.org.uk> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20231125201147.753695-1-viro@zeniv.linux.org.uk> References: <20231125201015.GA38156@ZenIV> <20231125201147.753695-1-viro@zeniv.linux.org.uk> MIME-Version: 1.0 Sender: Al Viro X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Sat, 25 Nov 2023 12:12:15 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783568159051527492 X-GMAIL-MSGID: 1783568159051527492 We can't really afford locking the source on same-directory rename; currently vfs_rename() tries to do that, but it will have to be changed. The logics in ext4 is lazy and goes looking for ".." in source even in same-directory case. It's not hard to get rid of that, leaving that behaviour only for cross-directory case; that VFS can get locks safely (and will keep doing that after the coming changes). Reviewed-by: Jan Kara Signed-off-by: Al Viro --- fs/ext4/namei.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index d252935f9c8a..467ba47a691c 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -3591,10 +3591,14 @@ struct ext4_renament { int dir_inlined; }; -static int ext4_rename_dir_prepare(handle_t *handle, struct ext4_renament *ent) +static int ext4_rename_dir_prepare(handle_t *handle, struct ext4_renament *ent, bool is_cross) { int retval; + ent->is_dir = true; + if (!is_cross) + return 0; + ent->dir_bh = ext4_get_first_dir_block(handle, ent->inode, &retval, &ent->parent_de, &ent->dir_inlined); @@ -3612,6 +3616,9 @@ static int ext4_rename_dir_finish(handle_t *handle, struct ext4_renament *ent, { int retval; + if (!ent->dir_bh) + return 0; + ent->parent_de->inode = cpu_to_le32(dir_ino); BUFFER_TRACE(ent->dir_bh, "call ext4_handle_dirty_metadata"); if (!ent->dir_inlined) { @@ -3900,7 +3907,7 @@ static int ext4_rename(struct mnt_idmap *idmap, struct inode *old_dir, if (new.dir != old.dir && EXT4_DIR_LINK_MAX(new.dir)) goto end_rename; } - retval = ext4_rename_dir_prepare(handle, &old); + retval = ext4_rename_dir_prepare(handle, &old, new.dir != old.dir); if (retval) goto end_rename; } @@ -3964,7 +3971,7 @@ static int ext4_rename(struct mnt_idmap *idmap, struct inode *old_dir, } inode_set_mtime_to_ts(old.dir, inode_set_ctime_current(old.dir)); ext4_update_dx_flag(old.dir); - if (old.dir_bh) { + if (old.is_dir) { retval = ext4_rename_dir_finish(handle, &old, new.dir->i_ino); if (retval) goto end_rename; @@ -3987,7 +3994,7 @@ static int ext4_rename(struct mnt_idmap *idmap, struct inode *old_dir, if (unlikely(retval)) goto end_rename; - if (S_ISDIR(old.inode->i_mode)) { + if (old.is_dir) { /* * We disable fast commits here that's because the * replay code is not yet capable of changing dot dot @@ -4114,14 +4121,12 @@ static int ext4_cross_rename(struct inode *old_dir, struct dentry *old_dentry, ext4_handle_sync(handle); if (S_ISDIR(old.inode->i_mode)) { - old.is_dir = true; - retval = ext4_rename_dir_prepare(handle, &old); + retval = ext4_rename_dir_prepare(handle, &old, new.dir != old.dir); if (retval) goto end_rename; } if (S_ISDIR(new.inode->i_mode)) { - new.is_dir = true; - retval = ext4_rename_dir_prepare(handle, &new); + retval = ext4_rename_dir_prepare(handle, &new, new.dir != old.dir); if (retval) goto end_rename; } From patchwork Sat Nov 25 20:11:44 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Al Viro X-Patchwork-Id: 169787 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce62:0:b0:403:3b70:6f57 with SMTP id o2csp2136789vqx; Sat, 25 Nov 2023 12:12:15 -0800 (PST) X-Google-Smtp-Source: AGHT+IHI6uNTO72ixAjEvY/bIyFxF73Qbu66hRf5OH6tPGGZWoFJ3hFTMuwV545oPfKIG/2h/aAi X-Received: by 2002:a05:6a00:22d2:b0:6cb:a434:b58f with SMTP id f18-20020a056a0022d200b006cba434b58fmr8922578pfj.33.1700943134890; Sat, 25 Nov 2023 12:12:14 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700943134; cv=none; d=google.com; s=arc-20160816; b=0jFrHFDK6je99w7kt1K4zW1U92LaRCyZq8x/Lzaf/nb/qeiRL7d2tLiJoPHUSwcMYZ aOGmbeH9D2PvcTzAEiuUGM1lf+VUIxsX8q+UODGoS7OFKgDHNBcMU4SgJAWuLIIitIRA hfxk6qX61SQtQCdMlAFfkAIrnO7x+UQuXMXG60qLlaBIFpW/4Scjw9gcN8acS+DDhqmy FYau4yxYhsOIeJND6+9mbZKhm0a97m0VWWUpUnddIHCKNI7SqcWARMcptNUoWPS66Qfo +4NcuTTLdIXbczkAMmi8riXDlaybXZN1mo46keJFwrH1hT8XgMAhvKAVNKyyBixdV7UW CyRw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=1kXdQTzP9QbMwip9HhQkODeDCTy6uPz4pJDbGLrfwH4=; fh=W6BEtqt7iV8phMzKg0C2MCZmXBORzGPTeuIhW3f8iqY=; b=QPFlPmA/dImeoYTAIWdztdQa4vdp4HvGZUO5K0KrqQea2nMV5OGO7l5TBrqFWk78Mr TwhO8imMzPzskXiWrxjOVcYqy8GnYWBbADjyNHTBLEFqiPC1WYnCP6XlsgJi0GmLbSzm 6Y1Q7//zj+w22Ef/nCTFiKdx4bJgwq/O7C7dtL7PBMWfgOHymN0/cutqlU3sy/shfASO GiSseX9t/7YtELhx7un8/uE8Z57cRB2VA9XvFtl5ol+h4NJGyCYqq4CBE/Yv22J4yjYj PtKpDrrn03vGy9mjnzWQLWye6uZhWo/D0NWSucVYuHTBT8KTC2q7g5TDrP0OiAl1ZeVU iDXw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=XcMyRMly; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from lipwig.vger.email (lipwig.vger.email. [23.128.96.33]) by mx.google.com with ESMTPS id r26-20020a6560da000000b005b8ee1c0c68si5976782pgv.605.2023.11.25.12.12.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 25 Nov 2023 12:12:14 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=XcMyRMly; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id 82E3780545B9; Sat, 25 Nov 2023 12:12:10 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232394AbjKYULy (ORCPT + 99 others); Sat, 25 Nov 2023 15:11:54 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46838 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229676AbjKYULn (ORCPT ); Sat, 25 Nov 2023 15:11:43 -0500 Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [IPv6:2a03:a000:7:0:5054:ff:fe1c:15ff]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B2CF413A; Sat, 25 Nov 2023 12:11:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=1kXdQTzP9QbMwip9HhQkODeDCTy6uPz4pJDbGLrfwH4=; b=XcMyRMlyK2J+etyO+6P/aIB6ik pLcqvGbY9qBE5zywvQUnOf0X3uLFW0H/OpUKVdiK04VPwNrsLqjvhPHBz9mOKokwiHL9YFDTv+FT2 xtdJNbOEYAiUQlO2MBcaUBIk5/NjtuNmJTJZJC7ASCA5Mh0608tTMX5KwYtwL62OAKo35eWjOkmWH D9vMKMvDO4yEXVLkvxxOMgzDA0Ofw5bVdAbdVMv+hfTKPEzIO0B572Fm+Xfe5WQ3qpZpThf6yTB0W xZ/ltCn0C1GTsqp7raqEB7lXB+1gCCvH1FIHNnzRvrqzcyTzX2pBtst2B70lIa6sSLgR+94M8A7HX O/s6m2Pw==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.96 #2 (Red Hat Linux)) id 1r6z0G-003A4n-0j; Sat, 25 Nov 2023 20:11:48 +0000 From: Al Viro To: linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Mo Zou , Jan Kara , linux-kernel@vger.kernel.org Subject: [PATCH v2 6/9] f2fs: Avoid reading renamed directory if parent does not change Date: Sat, 25 Nov 2023 20:11:44 +0000 Message-Id: <20231125201147.753695-6-viro@zeniv.linux.org.uk> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20231125201147.753695-1-viro@zeniv.linux.org.uk> References: <20231125201015.GA38156@ZenIV> <20231125201147.753695-1-viro@zeniv.linux.org.uk> MIME-Version: 1.0 Sender: Al Viro X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Sat, 25 Nov 2023 12:12:10 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783568148570988037 X-GMAIL-MSGID: 1783568148570988037 From: Jan Kara The VFS will not be locking moved directory if its parent does not change. Change f2fs rename code to avoid reading renamed directory if its parent does not change. Having it uninlined while we are reading it would cause trouble and we won't be able to rely upon ->i_rwsem on the directory being renamed in cases that do not alter its parent. Signed-off-by: Jan Kara Signed-off-by: Al Viro --- fs/f2fs/namei.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c index d0053b0284d8..fdc97df6bb85 100644 --- a/fs/f2fs/namei.c +++ b/fs/f2fs/namei.c @@ -963,6 +963,7 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct inode *old_dir, struct f2fs_dir_entry *old_dir_entry = NULL; struct f2fs_dir_entry *old_entry; struct f2fs_dir_entry *new_entry; + bool old_is_dir = S_ISDIR(old_inode->i_mode); int err; if (unlikely(f2fs_cp_error(sbi))) @@ -1017,7 +1018,7 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct inode *old_dir, goto out; } - if (S_ISDIR(old_inode->i_mode)) { + if (old_is_dir && old_dir != new_dir) { old_dir_entry = f2fs_parent_dir(old_inode, &old_dir_page); if (!old_dir_entry) { if (IS_ERR(old_dir_page)) @@ -1029,7 +1030,7 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct inode *old_dir, if (new_inode) { err = -ENOTEMPTY; - if (old_dir_entry && !f2fs_empty_dir(new_inode)) + if (old_is_dir && !f2fs_empty_dir(new_inode)) goto out_dir; err = -ENOENT; @@ -1054,7 +1055,7 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct inode *old_dir, inode_set_ctime_current(new_inode); f2fs_down_write(&F2FS_I(new_inode)->i_sem); - if (old_dir_entry) + if (old_is_dir) f2fs_i_links_write(new_inode, false); f2fs_i_links_write(new_inode, false); f2fs_up_write(&F2FS_I(new_inode)->i_sem); @@ -1074,12 +1075,12 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct inode *old_dir, goto out_dir; } - if (old_dir_entry) + if (old_is_dir) f2fs_i_links_write(new_dir, true); } f2fs_down_write(&F2FS_I(old_inode)->i_sem); - if (!old_dir_entry || whiteout) + if (!old_is_dir || whiteout) file_lost_pino(old_inode); else /* adjust dir's i_pino to pass fsck check */ @@ -1105,8 +1106,8 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct inode *old_dir, iput(whiteout); } - if (old_dir_entry) { - if (old_dir != new_dir && !whiteout) + if (old_is_dir) { + if (old_dir_entry && !whiteout) f2fs_set_link(old_inode, old_dir_entry, old_dir_page, new_dir); else From patchwork Sat Nov 25 20:11:45 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Al Viro X-Patchwork-Id: 169790 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce62:0:b0:403:3b70:6f57 with SMTP id o2csp2136852vqx; Sat, 25 Nov 2023 12:12:22 -0800 (PST) X-Google-Smtp-Source: AGHT+IFv4row3eadykKHduXCpjJ7/aGFuE0eRHwoCo9bc71CvddGKz4oTa2knqtCccbacGJJ6Wlt X-Received: by 2002:a17:903:22d2:b0:1cc:c273:603 with SMTP id y18-20020a17090322d200b001ccc2730603mr7136872plg.42.1700943142050; Sat, 25 Nov 2023 12:12:22 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700943142; cv=none; d=google.com; s=arc-20160816; b=jVPjvkCe4mxgEs1mKYQo2VlAYvqq0uvNtd2yqrHTzy0BaDFgBAH/wYE7pdPsqP/3rz ZKMaTWWRMLSoByvXGMseNB+4ZqBnaV8ozHsJ1y7znP62tDHe5V70h3ihD2UqJY2MyW0u TLX1Te06Zo90Qsvm68O8JfgsoI5Rv37bDoJcYY16IXdN5yPl8sjatGUIXA1HR4WnYwCi PVPxghCedLIX4ChFPjoXduwqqkjxvBAA4nM1udKA0V0INjW8BsfeJgTh4nnXFFAkouW1 Os97kN7bwh+j5Kj2tri5/2Z9GraKjLCeUSCl3leUtCf65m1dxTkIO3Uc3bjiPl2lrZGx IaPA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=XUgMZqPZEPLgKgaUi0hMtataaWNLmT1cmJUZyPdt/lg=; fh=W6BEtqt7iV8phMzKg0C2MCZmXBORzGPTeuIhW3f8iqY=; b=aBXYz27CPZ1M0FWOkVaUjq4tlsqP1dq6oNfdK7tkQx6X6ZC43CA4fq4iL2w+I6etjc +Oo3mXFG0+gxW2OUqa8oluU3oHwjekE3TW/ZRlrzutuy+TxNsXaY/Kc+bElrduA1c2LZ jhIWfkS8xM0I6FJx6sRSdLFupq4G5reU9pcoWM5Xu3sjbj7tWz+FldTwC3cld2HgoBaa KBu28/sSB3VUbzeB45I4WUdYxZEOiTiCSe/o5J7mZM2srLeHQPMzmUg4XzrQwGLdYblU 6jmmHl3hUpg+IuvdWs1p44k4eGKDYUkUZXU3pCZqOG/xJKuBTs6hQFB0wz4Nf+7Rl6K3 I5KA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=MlVaELPo; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from snail.vger.email (snail.vger.email. [23.128.96.37]) by mx.google.com with ESMTPS id j18-20020a170902da9200b001cf7c82c00asi6615133plx.499.2023.11.25.12.12.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 25 Nov 2023 12:12:22 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=MlVaELPo; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id EC1BB804E713; Sat, 25 Nov 2023 12:12:20 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232507AbjKYUMD (ORCPT + 99 others); Sat, 25 Nov 2023 15:12:03 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46856 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229882AbjKYULo (ORCPT ); Sat, 25 Nov 2023 15:11:44 -0500 Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [IPv6:2a03:a000:7:0:5054:ff:fe1c:15ff]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0781513E; Sat, 25 Nov 2023 12:11:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=XUgMZqPZEPLgKgaUi0hMtataaWNLmT1cmJUZyPdt/lg=; b=MlVaELPovpHGgnBvd7fJJKfSjH S25Pr2dLWiXgIeREsIEK/ilif/WucXEMQ/GdrNakD3XjHpn3k5r2T1DMDY/pPpz4KEEBoUzknOo7O aYku38a3ntkmjJEFzSkzfJaqvbq7XKW8rAmLkFRdl0h81d+V47hZgWWlzBLLTy6sTeMZUHUTtLPUz NbVC/DOkdHb7oj1h9yZqccPAZYKoTvtq+2rnuOTJxKv4E+EXli1wDp9uNK3GgwslC555ZsLMAw4Gi o6d2S6SCD5xVbpAHKXRliL4G+UIdaDfhdA35PXxbPT8IO37WpeTcBk0Rbziyr/wRSXj7TC6y7pqIt QpE9J5ug==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.96 #2 (Red Hat Linux)) id 1r6z0G-003A4t-1X; Sat, 25 Nov 2023 20:11:48 +0000 From: Al Viro To: linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Mo Zou , Jan Kara , linux-kernel@vger.kernel.org Subject: [PATCH v2 7/9] rename(): fix the locking of subdirectories Date: Sat, 25 Nov 2023 20:11:45 +0000 Message-Id: <20231125201147.753695-7-viro@zeniv.linux.org.uk> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20231125201147.753695-1-viro@zeniv.linux.org.uk> References: <20231125201015.GA38156@ZenIV> <20231125201147.753695-1-viro@zeniv.linux.org.uk> MIME-Version: 1.0 Sender: Al Viro X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Sat, 25 Nov 2023 12:12:21 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783568156376631229 X-GMAIL-MSGID: 1783568156376631229 We should never lock two subdirectories without having taken ->s_vfs_rename_mutex; inode pointer order or not, the "order" proposed in 28eceeda130f "fs: Lock moved directories" is not transitive, with the usual consequences. The rationale for locking renamed subdirectory in all cases was the possibility of race between rename modifying .. in a subdirectory to reflect the new parent and another thread modifying the same subdirectory. For a lot of filesystems that's not a problem, but for some it can lead to trouble (e.g. the case when short directory contents is kept in the inode, but creating a file in it might push it across the size limit and copy its contents into separate data block(s)). However, we need that only in case when the parent does change - otherwise ->rename() doesn't need to do anything with .. entry in the first place. Some instances are lazy and do a tautological update anyway, but it's really not hard to avoid. Amended locking rules for rename(): find the parent(s) of source and target if source and target have the same parent lock the common parent else lock ->s_vfs_rename_mutex lock both parents, in ancestor-first order; if neither is an ancestor of another, lock the parent of source first. find the source and target. if source and target have the same parent if operation is an overwriting rename of a subdirectory lock the target subdirectory else if source is a subdirectory lock the source if target is a subdirectory lock the target lock non-directories involved, in inode pointer order if both source and target are such. That way we are guaranteed that parents are locked (for obvious reasons), that any renamed non-directory is locked (nfsd relies upon that), that any victim is locked (emptiness check needs that, among other things) and subdirectory that changes parent is locked (needed to protect the update of .. entries). We are also guaranteed that any operation locking more than one directory either takes ->s_vfs_rename_mutex or locks a parent followed by its child. Cc: stable@vger.kernel.org Fixes: 28eceeda130f "fs: Lock moved directories" Reviewed-by: Jan Kara Signed-off-by: Al Viro --- .../filesystems/directory-locking.rst | 29 ++++----- Documentation/filesystems/locking.rst | 5 +- Documentation/filesystems/porting.rst | 18 ++++++ fs/namei.c | 60 ++++++++++++------- 4 files changed, 74 insertions(+), 38 deletions(-) diff --git a/Documentation/filesystems/directory-locking.rst b/Documentation/filesystems/directory-locking.rst index dccd61c7c5c3..193c22687851 100644 --- a/Documentation/filesystems/directory-locking.rst +++ b/Documentation/filesystems/directory-locking.rst @@ -22,13 +22,16 @@ exclusive. 3) object removal. Locking rules: caller locks parent, finds victim, locks victim and calls the method. Locks are exclusive. -4) rename() that is _not_ cross-directory. Locking rules: caller locks the -parent and finds source and target. We lock both (provided they exist). If we -need to lock two inodes of different type (dir vs non-dir), we lock directory -first. If we need to lock two inodes of the same type, lock them in inode -pointer order. Then call the method. All locks are exclusive. -NB: we might get away with locking the source (and target in exchange -case) shared. +4) rename() that is _not_ cross-directory. Locking rules: caller locks +the parent and finds source and target. Then we decide which of the +source and target need to be locked. Source needs to be locked if it's a +non-directory; target - if it's a non-directory or about to be removed. +Take the locks that need to be taken, in inode pointer order if need +to take both (that can happen only when both source and target are +non-directories - the source because it wouldn't be locked otherwise +and the target because mixing directory and non-directory is allowed +only with RENAME_EXCHANGE, and that won't be removing the target). +After the locks had been taken, call the method. All locks are exclusive. 5) link creation. Locking rules: @@ -44,20 +47,17 @@ rules: * lock the filesystem * lock parents in "ancestors first" order. If one is not ancestor of - the other, lock them in inode pointer order. + the other, lock the parent of source first. * find source and target. * if old parent is equal to or is a descendent of target fail with -ENOTEMPTY * if new parent is equal to or is a descendent of source fail with -ELOOP - * Lock both the source and the target provided they exist. If we - need to lock two inodes of different type (dir vs non-dir), we lock - the directory first. If we need to lock two inodes of the same type, - lock them in inode pointer order. + * Lock subdirectories involved (source before target). + * Lock non-directories involved, in inode pointer order. * call the method. -All ->i_rwsem are taken exclusive. Again, we might get away with locking -the source (and target in exchange case) shared. +All ->i_rwsem are taken exclusive. The rules above obviously guarantee that all directories that are going to be read, modified or removed by method will be locked by caller. @@ -67,6 +67,7 @@ If no directory is its own ancestor, the scheme above is deadlock-free. Proof: +[XXX: will be updated once we are done massaging the lock_rename()] First of all, at any moment we have a linear ordering of the objects - A < B iff (A is an ancestor of B) or (B is not an ancestor of A and ptr(A) < ptr(B)). diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst index 7be2900806c8..bd12f2f850ad 100644 --- a/Documentation/filesystems/locking.rst +++ b/Documentation/filesystems/locking.rst @@ -101,7 +101,7 @@ symlink: exclusive mkdir: exclusive unlink: exclusive (both) rmdir: exclusive (both)(see below) -rename: exclusive (all) (see below) +rename: exclusive (both parents, some children) (see below) readlink: no get_link: no setattr: exclusive @@ -123,6 +123,9 @@ get_offset_ctx no Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_rwsem exclusive on victim. cross-directory ->rename() has (per-superblock) ->s_vfs_rename_sem. + ->unlink() and ->rename() have ->i_rwsem exclusive on all non-directories + involved. + ->rename() has ->i_rwsem exclusive on any subdirectory that changes parent. See Documentation/filesystems/directory-locking.rst for more detailed discussion of the locking scheme for directory operations. diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst index 878e72b2f8b7..9100969e7de6 100644 --- a/Documentation/filesystems/porting.rst +++ b/Documentation/filesystems/porting.rst @@ -1061,3 +1061,21 @@ export_operations ->encode_fh() no longer has a default implementation to encode FILEID_INO32_GEN* file handles. Filesystems that used the default implementation may use the generic helper generic_encode_ino32_fh() explicitly. + +--- + +**mandatory** + +If ->rename() update of .. on cross-directory move needs an exclusion with +directory modifications, do *not* lock the subdirectory in question in your +->rename() - it's done by the caller now [that item should've been added in +28eceeda130f "fs: Lock moved directories"]. + +--- + +**mandatory** + +On same-directory ->rename() the (tautological) update of .. is not protected +by any locks; just don't do it if the old parent is the same as the new one. +We really can't lock two subdirectories in same-directory rename - not without +deadlocks. diff --git a/fs/namei.c b/fs/namei.c index 71c13b2990b4..29bafbdb44ca 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3021,20 +3021,14 @@ static struct dentry *lock_two_directories(struct dentry *p1, struct dentry *p2) p = d_ancestor(p2, p1); if (p) { inode_lock_nested(p2->d_inode, I_MUTEX_PARENT); - inode_lock_nested(p1->d_inode, I_MUTEX_CHILD); + inode_lock_nested(p1->d_inode, I_MUTEX_PARENT2); return p; } p = d_ancestor(p1, p2); - if (p) { - inode_lock_nested(p1->d_inode, I_MUTEX_PARENT); - inode_lock_nested(p2->d_inode, I_MUTEX_CHILD); - return p; - } - - lock_two_inodes(p1->d_inode, p2->d_inode, - I_MUTEX_PARENT, I_MUTEX_PARENT2); - return NULL; + inode_lock_nested(p1->d_inode, I_MUTEX_PARENT); + inode_lock_nested(p2->d_inode, I_MUTEX_PARENT2); + return p; } /* @@ -4716,11 +4710,12 @@ SYSCALL_DEFINE2(link, const char __user *, oldname, const char __user *, newname * * a) we can get into loop creation. * b) race potential - two innocent renames can create a loop together. - * That's where 4.4 screws up. Current fix: serialization on + * That's where 4.4BSD screws up. Current fix: serialization on * sb->s_vfs_rename_mutex. We might be more accurate, but that's another * story. - * c) we have to lock _four_ objects - parents and victim (if it exists), - * and source. + * c) we may have to lock up to _four_ objects - parents and victim (if it exists), + * and source (if it's a non-directory or a subdirectory that moves to + * different parent). * And that - after we got ->i_mutex on parents (until then we don't know * whether the target exists). Solution: try to be smart with locking * order for inodes. We rely on the fact that tree topology may change @@ -4752,6 +4747,7 @@ int vfs_rename(struct renamedata *rd) bool new_is_dir = false; unsigned max_links = new_dir->i_sb->s_max_links; struct name_snapshot old_name; + bool lock_old_subdir, lock_new_subdir; if (source == target) return 0; @@ -4805,15 +4801,32 @@ int vfs_rename(struct renamedata *rd) take_dentry_name_snapshot(&old_name, old_dentry); dget(new_dentry); /* - * Lock all moved children. Moved directories may need to change parent - * pointer so they need the lock to prevent against concurrent - * directory changes moving parent pointer. For regular files we've - * historically always done this. The lockdep locking subclasses are - * somewhat arbitrary but RENAME_EXCHANGE in particular can swap - * regular files and directories so it's difficult to tell which - * subclasses to use. + * Lock children. + * The source subdirectory needs to be locked on cross-directory + * rename or cross-directory exchange since its parent changes. + * The target subdirectory needs to be locked on cross-directory + * exchange due to parent change and on any rename due to becoming + * a victim. + * Non-directories need locking in all cases (for NFS reasons); + * they get locked after any subdirectories (in inode address order). + * + * NOTE: WE ONLY LOCK UNRELATED DIRECTORIES IN CROSS-DIRECTORY CASE. + * NEVER, EVER DO THAT WITHOUT ->s_vfs_rename_mutex. */ - lock_two_inodes(source, target, I_MUTEX_NORMAL, I_MUTEX_NONDIR2); + lock_old_subdir = new_dir != old_dir; + lock_new_subdir = new_dir != old_dir || !(flags & RENAME_EXCHANGE); + if (is_dir) { + if (lock_old_subdir) + inode_lock_nested(source, I_MUTEX_CHILD); + if (target && (!new_is_dir || lock_new_subdir)) + inode_lock(target); + } else if (new_is_dir) { + if (lock_new_subdir) + inode_lock_nested(target, I_MUTEX_CHILD); + inode_lock(source); + } else { + lock_two_nondirectories(source, target); + } error = -EPERM; if (IS_SWAPFILE(source) || (target && IS_SWAPFILE(target))) @@ -4861,8 +4874,9 @@ int vfs_rename(struct renamedata *rd) d_exchange(old_dentry, new_dentry); } out: - inode_unlock(source); - if (target) + if (!is_dir || lock_old_subdir) + inode_unlock(source); + if (target && (!new_is_dir || lock_new_subdir)) inode_unlock(target); dput(new_dentry); if (!error) { From patchwork Sat Nov 25 20:11:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Al Viro X-Patchwork-Id: 169793 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce62:0:b0:403:3b70:6f57 with SMTP id o2csp2136983vqx; Sat, 25 Nov 2023 12:12:46 -0800 (PST) X-Google-Smtp-Source: AGHT+IFsLBzLfWZvH7n1heM1EO4r9mN/O2F3E1t4/U9wmcR8x4pw9PRkZGeP0Biad5P/Xj7QNGG7 X-Received: by 2002:a05:6a20:baa4:b0:18b:82cb:4092 with SMTP id fb36-20020a056a20baa400b0018b82cb4092mr6255169pzb.11.1700943166238; Sat, 25 Nov 2023 12:12:46 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700943166; cv=none; d=google.com; s=arc-20160816; b=CCSjgOQDuqRaBzRmsa6ONbt1VAmjKtPm0PfaorTwWx62h1rBjDl5dxLHVZNHEDCtxH MfQZEYPBj7GqA+jMkk3Mq9MAIjsGLa5b7Enc00ByKV9JRYLtm2t7CvwDKwncjWAWDWoD rpfNSnWxR6aeCNFq1wy9DycHpYmsRdrXP+p6/hHjDyK4VbWE7518G54fjydbkuTIyuhV yThc34Ian91Blgnxla3t+iGfkB7/WDLoVXZdbDZRdnZLJmBjpKz55tqQJ/TzWokapp5k 0VlGTMMMvGzQc6pKboJ/5EuHjZZlUYLvcLWNvQenvu/WCvkha5hA9f/FlWtiqYX8gZVr 2LXA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=MXd1D7yrhiKOU6TnSrHtQJWPS/0VvoZfIFu1ppuy74Y=; fh=W6BEtqt7iV8phMzKg0C2MCZmXBORzGPTeuIhW3f8iqY=; b=EwbJ0y3eGP9KZvX3CtlEYWUpbI4f7crIyicJvKjFD6dmbym6mF0QulOFtuNLhAcb9x n1eun+gNjDUoR5abqcBg30LusD5XOPR4Oeg4FM2F+HlNfkjZTkjJrLrj0DLz1XjQdQuw jQvBBDW7DU7WcRfDPHhNNYESB0afJkcWUOR9SReJeEi/CNMljqHpC8G2pTphlUS20gO7 k/GwjcnmsXFuMvEECw9UazwkAv6iEumc46RmJYvzEt1/Jks05jSiy5VQaw/RdoyzAs9m GoBz1UAWMUAMrESxdlUI8Ih+513Bq9D9tOKSOUZwXEXq+WeTasdQgZQH8E3fRla7vGT4 WBOg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=N14uncTx; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from fry.vger.email (fry.vger.email. [23.128.96.38]) by mx.google.com with ESMTPS id be7-20020a170902aa0700b001cf6a664aa8si5906059plb.571.2023.11.25.12.12.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 25 Nov 2023 12:12:46 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=N14uncTx; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by fry.vger.email (Postfix) with ESMTP id 6A93A809679C; Sat, 25 Nov 2023 12:12:41 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at fry.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232484AbjKYUMA (ORCPT + 99 others); Sat, 25 Nov 2023 15:12:00 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46840 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229849AbjKYULn (ORCPT ); Sat, 25 Nov 2023 15:11:43 -0500 Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [IPv6:2a03:a000:7:0:5054:ff:fe1c:15ff]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 201E8182; Sat, 25 Nov 2023 12:11:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=MXd1D7yrhiKOU6TnSrHtQJWPS/0VvoZfIFu1ppuy74Y=; b=N14uncTx4JMNyU5OFWglHbH2bZ 0AgVwOC6P/HTaLgEyAMcnsSEHNPqqJ0PSE/6p1B/GZE2P4/ZgTrAigHx/9GIW0ChEIfdFEch7D5fS EGSRaN+92wJ955vDqvPJO9sGjRTxwn+b5xoE0eguizSQCrYL6RyGN/h26EwYfYNQabRqAjVLQcJjH ne+P2/ky55mbFPomJSmnCfz3TC190ihS+5jUc4fpzCMlSYK0S4uMfCzLNdbh7sGP5fopwTkW1XBa4 yXND41vhkQNjTyNM0vHFefNTDc8Dk58QYkXBgpeDW1ZZKTW+Z8a55b68RzQEc3Yn6k/VFoUNJr0EP sful3ZWw==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.96 #2 (Red Hat Linux)) id 1r6z0G-003A53-2T; Sat, 25 Nov 2023 20:11:48 +0000 From: Al Viro To: linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Mo Zou , Jan Kara , linux-kernel@vger.kernel.org Subject: [PATCH v2 8/9] kill lock_two_inodes() Date: Sat, 25 Nov 2023 20:11:46 +0000 Message-Id: <20231125201147.753695-8-viro@zeniv.linux.org.uk> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20231125201147.753695-1-viro@zeniv.linux.org.uk> References: <20231125201015.GA38156@ZenIV> <20231125201147.753695-1-viro@zeniv.linux.org.uk> MIME-Version: 1.0 Sender: Al Viro X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Sat, 25 Nov 2023 12:12:41 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783568181518805172 X-GMAIL-MSGID: 1783568181518805172 There's only one caller left (lock_two_nondirectories()), and it needs less complexity. Fold lock_two_inodes() in there and simplify. Reviewed-by: Jan Kara Signed-off-by: Al Viro --- fs/inode.c | 49 ++++++------------------------------------------- fs/internal.h | 2 -- 2 files changed, 6 insertions(+), 45 deletions(-) diff --git a/fs/inode.c b/fs/inode.c index edcd8a61975f..453d5be1a014 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -1087,48 +1087,6 @@ void discard_new_inode(struct inode *inode) } EXPORT_SYMBOL(discard_new_inode); -/** - * lock_two_inodes - lock two inodes (may be regular files but also dirs) - * - * Lock any non-NULL argument. The caller must make sure that if he is passing - * in two directories, one is not ancestor of the other. Zero, one or two - * objects may be locked by this function. - * - * @inode1: first inode to lock - * @inode2: second inode to lock - * @subclass1: inode lock subclass for the first lock obtained - * @subclass2: inode lock subclass for the second lock obtained - */ -void lock_two_inodes(struct inode *inode1, struct inode *inode2, - unsigned subclass1, unsigned subclass2) -{ - if (!inode1 || !inode2) { - /* - * Make sure @subclass1 will be used for the acquired lock. - * This is not strictly necessary (no current caller cares) but - * let's keep things consistent. - */ - if (!inode1) - swap(inode1, inode2); - goto lock; - } - - /* - * If one object is directory and the other is not, we must make sure - * to lock directory first as the other object may be its child. - */ - if (S_ISDIR(inode2->i_mode) == S_ISDIR(inode1->i_mode)) { - if (inode1 > inode2) - swap(inode1, inode2); - } else if (!S_ISDIR(inode1->i_mode)) - swap(inode1, inode2); -lock: - if (inode1) - inode_lock_nested(inode1, subclass1); - if (inode2 && inode2 != inode1) - inode_lock_nested(inode2, subclass2); -} - /** * lock_two_nondirectories - take two i_mutexes on non-directory objects * @@ -1144,7 +1102,12 @@ void lock_two_nondirectories(struct inode *inode1, struct inode *inode2) WARN_ON_ONCE(S_ISDIR(inode1->i_mode)); if (inode2) WARN_ON_ONCE(S_ISDIR(inode2->i_mode)); - lock_two_inodes(inode1, inode2, I_MUTEX_NORMAL, I_MUTEX_NONDIR2); + if (inode1 > inode2) + swap(inode1, inode2); + if (inode1) + inode_lock(inode1); + if (inode2 && inode2 != inode1) + inode_lock_nested(inode2, I_MUTEX_NONDIR2); } EXPORT_SYMBOL(lock_two_nondirectories); diff --git a/fs/internal.h b/fs/internal.h index 58e43341aebf..de67b02226e5 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -196,8 +196,6 @@ extern long prune_icache_sb(struct super_block *sb, struct shrink_control *sc); int dentry_needs_remove_privs(struct mnt_idmap *, struct dentry *dentry); bool in_group_or_capable(struct mnt_idmap *idmap, const struct inode *inode, vfsgid_t vfsgid); -void lock_two_inodes(struct inode *inode1, struct inode *inode2, - unsigned subclass1, unsigned subclass2); /* * fs-writeback.c From patchwork Sat Nov 25 20:11:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Al Viro X-Patchwork-Id: 169792 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce62:0:b0:403:3b70:6f57 with SMTP id o2csp2136872vqx; Sat, 25 Nov 2023 12:12:27 -0800 (PST) X-Google-Smtp-Source: AGHT+IECFj1OlclY0a+sv/eAFN7O9zkiFWLBb7C2K/CMS8o8QQ2DhpMRmM+6yF2XAlDxgnzpq7t+ X-Received: by 2002:a05:6a00:1387:b0:6cb:68d7:b1bc with SMTP id t7-20020a056a00138700b006cb68d7b1bcmr7142962pfg.30.1700943147326; Sat, 25 Nov 2023 12:12:27 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700943147; cv=none; d=google.com; s=arc-20160816; b=DB0y4bX0eJs6dtuAH0/kosTy7AsN4hbmRTvEVVJkYdWm4Jzd2/52qh8OXaafVyScYG PJ3+zWAFQUj/qPd5RyX7tCGkY8zR/ToXfgRxAtpqWldlgyRvoJDB2sQBlUwdxeQfizLV Co2dgx8YodcSR4cVN+ysCY2EvqDXLbRF6s538OrsKrOY6jLofgv6wciBD09IKQwp59rU I0SHiVYQNhtu+Nkto4cnNjM6TOSnLhVMs5YhICl9c2ZTPYMdmFge2F2m+rARnCzeZLW5 ktMNVb+hRVhnAwTlP7lJL4ZUiMWBaXy+WJXmmqnqIDDErZ9j/oAbIUStf2t+encoTECl G0+A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=f5aDp+zcGPtujBUoSm7966OQLopOua0j6cltQRU/GSs=; fh=W6BEtqt7iV8phMzKg0C2MCZmXBORzGPTeuIhW3f8iqY=; b=QaPWHDGFEESM/nNLFctzV79c8zLlW8y4ldcMTxCbDrxsE9S3gUP+iBjN/BFcTMC2ga Z9E9gu/9zzl62WBv9k+UpWrgW3ayGEM0rxyTcJEdkam9zhv8LjxLZ+9Cu8daqrC/Jaiv 4wbgsV1EfCreCPULVdnniuUGLCIai8UCSElMr+zE89BxXqETumyoM0Hh2H+L9S37XVP8 jeSG708A8Lem/WhBaD0hR62bgOrcTHBv+BPlQ9HJBmBfszds8n2geJCpk3vfps301mL6 6MSx/OEyEogaaGAw2OTZgPgR4OMrwTV9lNVoBzWUAwQCeS8CQNI1l3zSTy4RRmnR82Ak pm9A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=BEEdfeYa; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id v10-20020a056a00148a00b006cc040f386esi2227612pfu.60.2023.11.25.12.12.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 25 Nov 2023 12:12:27 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.org.uk header.s=zeniv-20220401 header.b=BEEdfeYa; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=zeniv.linux.org.uk Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id C053E804E73C; Sat, 25 Nov 2023 12:12:25 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232554AbjKYUMH (ORCPT + 99 others); Sat, 25 Nov 2023 15:12:07 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46866 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232360AbjKYULq (ORCPT ); Sat, 25 Nov 2023 15:11:46 -0500 Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [IPv6:2a03:a000:7:0:5054:ff:fe1c:15ff]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 50C4F189; Sat, 25 Nov 2023 12:11:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=f5aDp+zcGPtujBUoSm7966OQLopOua0j6cltQRU/GSs=; b=BEEdfeYatMVQupuizkEkaWd757 7ApB20OjxG9+H8gbgZOLRGbxALlEqdR1Zlt+BaI0DybD7VnxcmZfVYinBMiI7MeynP/k34efeYZga 4ezZulRchQw0aUDqEv61UO/94Ba6Mp8RdwyXHy0NrtL+d2M2gOzwfILt6PSS1wpKebfWuLJcKs1oA Nk4SDD+Em8Bz4kcHDkU0BVnH1WlT+KER2jHPWeyGvFsIVCqiLWI2Gnsz6ZZzhVMko1q5O7FOSeUXt +ZCGr/oYkD/dab1CuJ/nR0F1p/uRmY4Qp+1eX6T9wd17R1dKpgZIO92mWOJQGgqt2VwI6se7jB2tG xSqF7XDw==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.96 #2 (Red Hat Linux)) id 1r6z0G-003A5A-2o; Sat, 25 Nov 2023 20:11:48 +0000 From: Al Viro To: linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Mo Zou , Jan Kara , linux-kernel@vger.kernel.org Subject: [PATCH v2 9/9] rename(): avoid a deadlock in the case of parents having no common ancestor Date: Sat, 25 Nov 2023 20:11:47 +0000 Message-Id: <20231125201147.753695-9-viro@zeniv.linux.org.uk> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20231125201147.753695-1-viro@zeniv.linux.org.uk> References: <20231125201015.GA38156@ZenIV> <20231125201147.753695-1-viro@zeniv.linux.org.uk> MIME-Version: 1.0 Sender: Al Viro X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Sat, 25 Nov 2023 12:12:25 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783568161613806136 X-GMAIL-MSGID: 1783568161613806136 ... and fix the directory locking documentation and proof of correctness. Holding ->s_vfs_rename_mutex *almost* prevents ->d_parent changes; the case where we really don't want it is splicing the root of disconnected tree to somewhere. In other words, ->s_vfs_rename_mutex is sufficient to stabilize "X is an ancestor of Y" only if X and Y are already in the same tree. Otherwise it can go from false to true, and one can construct a deadlock on that. Make lock_two_directories() report an error in such case and update the callers of lock_rename()/lock_rename_child() to handle such errors. And yes, such conditions are not impossible to create ;-/ Reviewed-by: Jan Kara Signed-off-by: Al Viro --- .../filesystems/directory-locking.rst | 346 ++++++++++++------ Documentation/filesystems/porting.rst | 9 + fs/cachefiles/namei.c | 2 + fs/ecryptfs/inode.c | 2 + fs/namei.c | 37 +- fs/nfsd/vfs.c | 4 + fs/overlayfs/copy_up.c | 9 +- fs/overlayfs/dir.c | 4 + fs/overlayfs/super.c | 6 +- fs/overlayfs/util.c | 7 +- fs/smb/server/vfs.c | 5 + 11 files changed, 313 insertions(+), 118 deletions(-) diff --git a/Documentation/filesystems/directory-locking.rst b/Documentation/filesystems/directory-locking.rst index 193c22687851..05ea387bc9fb 100644 --- a/Documentation/filesystems/directory-locking.rst +++ b/Documentation/filesystems/directory-locking.rst @@ -11,130 +11,268 @@ When taking the i_rwsem on multiple non-directory objects, we always acquire the locks in order by increasing address. We'll call that "inode pointer" order in the following. -For our purposes all operations fall in 5 classes: -1) read access. Locking rules: caller locks directory we are accessing. -The lock is taken shared. +Primitives +========== -2) object creation. Locking rules: same as above, but the lock is taken -exclusive. +For our purposes all operations fall in 6 classes: -3) object removal. Locking rules: caller locks parent, finds victim, -locks victim and calls the method. Locks are exclusive. +1. read access. Locking rules: -4) rename() that is _not_ cross-directory. Locking rules: caller locks -the parent and finds source and target. Then we decide which of the -source and target need to be locked. Source needs to be locked if it's a -non-directory; target - if it's a non-directory or about to be removed. -Take the locks that need to be taken, in inode pointer order if need -to take both (that can happen only when both source and target are -non-directories - the source because it wouldn't be locked otherwise -and the target because mixing directory and non-directory is allowed -only with RENAME_EXCHANGE, and that won't be removing the target). -After the locks had been taken, call the method. All locks are exclusive. + * lock the directory we are accessing (shared) -5) link creation. Locking rules: +2. object creation. Locking rules: - * lock parent - * check that source is not a directory - * lock source - * call the method. + * lock the directory we are accessing (exclusive) -All locks are exclusive. +3. object removal. Locking rules: -6) cross-directory rename. The trickiest in the whole bunch. Locking -rules: + * lock the parent (exclusive) + * find the victim + * lock the victim (exclusive) - * lock the filesystem - * lock parents in "ancestors first" order. If one is not ancestor of - the other, lock the parent of source first. - * find source and target. - * if old parent is equal to or is a descendent of target - fail with -ENOTEMPTY - * if new parent is equal to or is a descendent of source - fail with -ELOOP - * Lock subdirectories involved (source before target). - * Lock non-directories involved, in inode pointer order. - * call the method. +4. link creation. Locking rules: + + * lock the parent (exclusive) + * check that the source is not a directory + * lock the source (exclusive; probably could be weakened to shared) -All ->i_rwsem are taken exclusive. +5. rename that is _not_ cross-directory. Locking rules: -The rules above obviously guarantee that all directories that are going to be -read, modified or removed by method will be locked by caller. + * lock the parent (exclusive) + * find the source and target + * decide which of the source and target need to be locked. + The source needs to be locked if it's a non-directory, target - if it's + a non-directory or about to be removed. + * take the locks that need to be taken (exlusive), in inode pointer order + if need to take both (that can happen only when both source and target + are non-directories - the source because it wouldn't need to be locked + otherwise and the target because mixing directory and non-directory is + allowed only with RENAME_EXCHANGE, and that won't be removing the target). +6. cross-directory rename. The trickiest in the whole bunch. Locking rules: + + * lock the filesystem + * if the parents don't have a common ancestor, fail the operation. + * lock the parents in "ancestors first" order (exclusive). If neither is an + ancestor of the other, lock the parent of source first. + * find the source and target. + * verify that the source is not a descendent of the target and + target is not a descendent of source; fail the operation otherwise. + * lock the subdirectories involved (exclusive), source before target. + * lock the non-directories involved (exclusive), in inode pointer order. + +The rules above obviously guarantee that all directories that are going +to be read, modified or removed by method will be locked by the caller. + + +Splicing +======== + +There is one more thing to consider - splicing. It's not an operation +in its own right; it may happen as part of lookup. We speak of the +operations on directory trees, but we obviously do not have the full +picture of those - especially for network filesystems. What we have +is a bunch of subtrees visible in dcache and locking happens on those. +Trees grow as we do operations; memory pressure prunes them. Normally +that's not a problem, but there is a nasty twist - what should we do +when one growing tree reaches the root of another? That can happen in +several scenarios, starting from "somebody mounted two nested subtrees +from the same NFS4 server and doing lookups in one of them has reached +the root of another"; there's also open-by-fhandle stuff, and there's a +possibility that directory we see in one place gets moved by the server +to another and we run into it when we do a lookup. + +For a lot of reasons we want to have the same directory present in dcache +only once. Multiple aliases are not allowed. So when lookup runs into +a subdirectory that already has an alias, something needs to be done with +dcache trees. Lookup is already holding the parent locked. If alias is +a root of separate tree, it gets attached to the directory we are doing a +lookup in, under the name we'd been looking for. If the alias is already +a child of the directory we are looking in, it changes name to the one +we'd been looking for. No extra locking is involved in these two cases. +However, if it's a child of some other directory, the things get trickier. +First of all, we verify that it is *not* an ancestor of our directory +and fail the lookup if it is. Then we try to lock the filesystem and the +current parent of the alias. If either trylock fails, we fail the lookup. +If trylocks succeed, we detach the alias from its current parent and +attach to our directory, under the name we are looking for. + +Note that splicing does *not* involve any modification of the filesystem; +all we change is the view in dcache. Moreover, holding a directory locked +exclusive prevents such changes involving its children and holding the +filesystem lock prevents any changes of tree topology, other than having a +root of one tree becoming a child of directory in another. In particular, +if two dentries have been found to have a common ancestor after taking +the filesystem lock, their relationship will remain unchanged until +the lock is dropped. So from the directory operations' point of view +splicing is almost irrelevant - the only place where it matters is one +step in cross-directory renames; we need to be careful when checking if +parents have a common ancestor. + + +Multiple-filesystem stuff +========================= + +For some filesystems a method can involve a directory operation on +another filesystem; it may be ecryptfs doing operation in the underlying +filesystem, overlayfs doing something to the layers, network filesystem +using a local one as a cache, etc. In all such cases the operations +on other filesystems must follow the same locking rules. Moreover, "a +directory operation on this filesystem might involve directory operations +on that filesystem" should be an asymmetric relation (or, if you will, +it should be possible to rank the filesystems so that directory operation +on a filesystem could trigger directory operations only on higher-ranked +ones - in these terms overlayfs ranks lower than its layers, network +filesystem ranks lower than whatever it caches on, etc.) + + +Deadlock avoidance +================== If no directory is its own ancestor, the scheme above is deadlock-free. Proof: -[XXX: will be updated once we are done massaging the lock_rename()] - First of all, at any moment we have a linear ordering of the - objects - A < B iff (A is an ancestor of B) or (B is not an ancestor - of A and ptr(A) < ptr(B)). - - That ordering can change. However, the following is true: - -(1) if object removal or non-cross-directory rename holds lock on A and - attempts to acquire lock on B, A will remain the parent of B until we - acquire the lock on B. (Proof: only cross-directory rename can change - the parent of object and it would have to lock the parent). - -(2) if cross-directory rename holds the lock on filesystem, order will not - change until rename acquires all locks. (Proof: other cross-directory - renames will be blocked on filesystem lock and we don't start changing - the order until we had acquired all locks). - -(3) locks on non-directory objects are acquired only after locks on - directory objects, and are acquired in inode pointer order. - (Proof: all operations but renames take lock on at most one - non-directory object, except renames, which take locks on source and - target in inode pointer order in the case they are not directories.) - -Now consider the minimal deadlock. Each process is blocked on -attempt to acquire some lock and already holds at least one lock. Let's -consider the set of contended locks. First of all, filesystem lock is -not contended, since any process blocked on it is not holding any locks. -Thus all processes are blocked on ->i_rwsem. - -By (3), any process holding a non-directory lock can only be -waiting on another non-directory lock with a larger address. Therefore -the process holding the "largest" such lock can always make progress, and -non-directory objects are not included in the set of contended locks. - -Thus link creation can't be a part of deadlock - it can't be -blocked on source and it means that it doesn't hold any locks. - -Any contended object is either held by cross-directory rename or -has a child that is also contended. Indeed, suppose that it is held by -operation other than cross-directory rename. Then the lock this operation -is blocked on belongs to child of that object due to (1). - -It means that one of the operations is cross-directory rename. -Otherwise the set of contended objects would be infinite - each of them -would have a contended child and we had assumed that no object is its -own descendent. Moreover, there is exactly one cross-directory rename -(see above). - -Consider the object blocking the cross-directory rename. One -of its descendents is locked by cross-directory rename (otherwise we -would again have an infinite set of contended objects). But that -means that cross-directory rename is taking locks out of order. Due -to (2) the order hadn't changed since we had acquired filesystem lock. -But locking rules for cross-directory rename guarantee that we do not -try to acquire lock on descendent before the lock on ancestor. -Contradiction. I.e. deadlock is impossible. Q.E.D. - +There is a ranking on the locks, such that all primitives take +them in order of non-decreasing rank. Namely, + + * rank ->i_rwsem of non-directories on given filesystem in inode pointer + order. + * put ->i_rwsem of all directories on a filesystem at the same rank, + lower than ->i_rwsem of any non-directory on the same filesystem. + * put ->s_vfs_rename_mutex at rank lower than that of any ->i_rwsem + on the same filesystem. + * among the locks on different filesystems use the relative + rank of those filesystems. + +For example, if we have NFS filesystem caching on a local one, we have + + 1. ->s_vfs_rename_mutex of NFS filesystem + 2. ->i_rwsem of directories on that NFS filesystem, same rank for all + 3. ->i_rwsem of non-directories on that filesystem, in order of + increasing address of inode + 4. ->s_vfs_rename_mutex of local filesystem + 5. ->i_rwsem of directories on the local filesystem, same rank for all + 6. ->i_rwsem of non-directories on local filesystem, in order of + increasing address of inode. + +It's easy to verify that operations never take a lock with rank +lower than that of an already held lock. + +Suppose deadlocks are possible. Consider the minimal deadlocked +set of threads. It is a cycle of several threads, each blocked on a lock +held by the next thread in the cycle. + +Since the locking order is consistent with the ranking, all +contended locks in the minimal deadlock will be of the same rank, +i.e. they all will be ->i_rwsem of directories on the same filesystem. +Moreover, without loss of generality we can assume that all operations +are done directly to that filesystem and none of them has actually +reached the method call. + +In other words, we have a cycle of threads, T1,..., Tn, +and the same number of directories (D1,...,Dn) such that + + T1 is blocked on D1 which is held by T2 + + T2 is blocked on D2 which is held by T3 + + ... + + Tn is blocked on Dn which is held by T1. + +Each operation in the minimal cycle must have locked at least +one directory and blocked on attempt to lock another. That leaves +only 3 possible operations: directory removal (locks parent, then +child), same-directory rename killing a subdirectory (ditto) and +cross-directory rename of some sort. + +There must be a cross-directory rename in the set; indeed, +if all operations had been of the "lock parent, then child" sort +we would have Dn a parent of D1, which is a parent of D2, which is +a parent of D3, ..., which is a parent of Dn. Relationships couldn't +have changed since the moment directory locks had been acquired, +so they would all hold simultaneously at the deadlock time and +we would have a loop. + +Since all operations are on the same filesystem, there can't be +more than one cross-directory rename among them. Without loss of +generality we can assume that T1 is the one doing a cross-directory +rename and everything else is of the "lock parent, then child" sort. + +In other words, we have a cross-directory rename that locked +Dn and blocked on attempt to lock D1, which is a parent of D2, which is +a parent of D3, ..., which is a parent of Dn. Relationships between +D1,...,Dn all hold simultaneously at the deadlock time. Moreover, +cross-directory rename does not get to locking any directories until it +has acquired filesystem lock and verified that directories involved have +a common ancestor, which guarantees that ancestry relationships between +all of them had been stable. + +Consider the order in which directories are locked by the +cross-directory rename; parents first, then possibly their children. +Dn and D1 would have to be among those, with Dn locked before D1. +Which pair could it be? + +It can't be the parents - indeed, since D1 is an ancestor of Dn, +it would be the first parent to be locked. Therefore at least one of the +children must be involved and thus neither of them could be a descendent +of another - otherwise the operation would not have progressed past +locking the parents. + +It can't be a parent and its child; otherwise we would've had +a loop, since the parents are locked before the children, so the parent +would have to be a descendent of its child. + +It can't be a parent and a child of another parent either. +Otherwise the child of the parent in question would've been a descendent +of another child. + +That leaves only one possibility - namely, both Dn and D1 are +among the children, in some order. But that is also impossible, since +neither of the children is a descendent of another. + +That concludes the proof, since the set of operations with the +properties requiered for a minimal deadlock can not exist. + +Note that the check for having a common ancestor in cross-directory +rename is crucial - without it a deadlock would be possible. Indeed, +suppose the parents are initially in different trees; we would lock the +parent of source, then try to lock the parent of target, only to have +an unrelated lookup splice a distant ancestor of source to some distant +descendent of the parent of target. At that point we have cross-directory +rename holding the lock on parent of source and trying to lock its +distant ancestor. Add a bunch of rmdir() attempts on all directories +in between (all of those would fail with -ENOTEMPTY, had they ever gotten +the locks) and voila - we have a deadlock. + +Loop avoidance +============== These operations are guaranteed to avoid loop creation. Indeed, the only operation that could introduce loops is cross-directory rename. -Since the only new (parent, child) pair added by rename() is (new parent, -source), such loop would have to contain these objects and the rest of it -would have to exist before rename(). I.e. at the moment of loop creation -rename() responsible for that would be holding filesystem lock and new parent -would have to be equal to or a descendent of source. But that means that -new parent had been equal to or a descendent of source since the moment when -we had acquired filesystem lock and rename() would fail with -ELOOP in that -case. +Suppose after the operation there is a loop; since there hadn't been such +loops before the operation, at least on of the nodes in that loop must've +had its parent changed. In other words, the loop must be passing through +the source or, in case of exchange, possibly the target. + +Since the operation has succeeded, neither source nor target could have +been ancestors of each other. Therefore the chain of ancestors starting +in the parent of source could not have passed through the target and +vice versa. On the other hand, the chain of ancestors of any node could +not have passed through the node itself, or we would've had a loop before +the operation. But everything other than source and target has kept +the parent after the operation, so the operation does not change the +chains of ancestors of (ex-)parents of source and target. In particular, +those chains must end after a finite number of steps. + +Now consider the loop created by the operation. It passes through either +source or target; the next node in the loop would be the ex-parent of +target or source resp. After that the loop would follow the chain of +ancestors of that parent. But as we have just shown, that chain must +end after a finite number of steps, which means that it can't be a part +of any loop. Q.E.D. While this locking scheme works for arbitrary DAGs, it relies on ability to check that directory is a descendent of another object. Current diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst index 9100969e7de6..33cd56e2ca1a 100644 --- a/Documentation/filesystems/porting.rst +++ b/Documentation/filesystems/porting.rst @@ -1079,3 +1079,12 @@ On same-directory ->rename() the (tautological) update of .. is not protected by any locks; just don't do it if the old parent is the same as the new one. We really can't lock two subdirectories in same-directory rename - not without deadlocks. + +--- + +**mandatory** + +lock_rename() and lock_rename_child() may fail in cross-directory case, if +their arguments do not have a common ancestor. In that case ERR_PTR(-EXDEV) +is returned, with no locks taken. In-tree users updated; out-of-tree ones +would need to do so. diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c index 7bf7a5fcc045..7ade836beb58 100644 --- a/fs/cachefiles/namei.c +++ b/fs/cachefiles/namei.c @@ -305,6 +305,8 @@ int cachefiles_bury_object(struct cachefiles_cache *cache, /* do the multiway lock magic */ trap = lock_rename(cache->graveyard, dir); + if (IS_ERR(trap)) + return PTR_ERR(trap); /* do some checks before getting the grave dentry */ if (rep->d_parent != dir || IS_DEADDIR(d_inode(rep))) { diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index a25dd3d20008..8efd20dc902b 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -599,6 +599,8 @@ ecryptfs_rename(struct mnt_idmap *idmap, struct inode *old_dir, target_inode = d_inode(new_dentry); trap = lock_rename(lower_old_dir_dentry, lower_new_dir_dentry); + if (IS_ERR(trap)) + return PTR_ERR(trap); dget(lower_new_dentry); rc = -EINVAL; if (lower_old_dentry->d_parent != lower_old_dir_dentry) diff --git a/fs/namei.c b/fs/namei.c index 29bafbdb44ca..6b0302ac80d1 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3014,21 +3014,37 @@ static inline int may_create(struct mnt_idmap *idmap, return inode_permission(idmap, dir, MAY_WRITE | MAY_EXEC); } +// p1 != p2, both are on the same filesystem, ->s_vfs_rename_mutex is held static struct dentry *lock_two_directories(struct dentry *p1, struct dentry *p2) { - struct dentry *p; + struct dentry *p = p1, *q = p2, *r; - p = d_ancestor(p2, p1); - if (p) { + while ((r = p->d_parent) != p2 && r != p) + p = r; + if (r == p2) { + // p is a child of p2 and an ancestor of p1 or p1 itself inode_lock_nested(p2->d_inode, I_MUTEX_PARENT); inode_lock_nested(p1->d_inode, I_MUTEX_PARENT2); return p; } - - p = d_ancestor(p1, p2); - inode_lock_nested(p1->d_inode, I_MUTEX_PARENT); - inode_lock_nested(p2->d_inode, I_MUTEX_PARENT2); - return p; + // p is the root of connected component that contains p1 + // p2 does not occur on the path from p to p1 + while ((r = q->d_parent) != p1 && r != p && r != q) + q = r; + if (r == p1) { + // q is a child of p1 and an ancestor of p2 or p2 itself + inode_lock_nested(p1->d_inode, I_MUTEX_PARENT); + inode_lock_nested(p2->d_inode, I_MUTEX_PARENT2); + return q; + } else if (likely(r == p)) { + // both p2 and p1 are descendents of p + inode_lock_nested(p1->d_inode, I_MUTEX_PARENT); + inode_lock_nested(p2->d_inode, I_MUTEX_PARENT2); + return NULL; + } else { // no common ancestor at the time we'd been called + mutex_unlock(&p1->d_sb->s_vfs_rename_mutex); + return ERR_PTR(-EXDEV); + } } /* @@ -4947,6 +4963,10 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd, retry_deleg: trap = lock_rename(new_path.dentry, old_path.dentry); + if (IS_ERR(trap)) { + error = PTR_ERR(trap); + goto exit_lock_rename; + } old_dentry = lookup_one_qstr_excl(&old_last, old_path.dentry, lookup_flags); @@ -5014,6 +5034,7 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd, dput(old_dentry); exit3: unlock_rename(new_path.dentry, old_path.dentry); +exit_lock_rename: if (delegated_inode) { error = break_deleg_wait(&delegated_inode); if (!error) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index fbbea7498f02..a99260c3f9bc 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -1813,6 +1813,10 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen, } trap = lock_rename(tdentry, fdentry); + if (IS_ERR(trap)) { + err = (rqstp->rq_vers == 2) ? nfserr_acces : nfserr_xdev; + goto out; + } err = fh_fill_pre_attrs(ffhp); if (err != nfs_ok) goto out_unlock; diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 4382881b0709..e44dc5f66161 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -722,7 +722,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c) struct inode *inode; struct inode *udir = d_inode(c->destdir), *wdir = d_inode(c->workdir); struct path path = { .mnt = ovl_upper_mnt(ofs) }; - struct dentry *temp, *upper; + struct dentry *temp, *upper, *trap; struct ovl_cu_creds cc; int err; struct ovl_cattr cattr = { @@ -760,9 +760,11 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c) * If temp was moved, abort without the cleanup. */ ovl_start_write(c->dentry); - if (lock_rename(c->workdir, c->destdir) != NULL || - temp->d_parent != c->workdir) { + trap = lock_rename(c->workdir, c->destdir); + if (trap || temp->d_parent != c->workdir) { err = -EIO; + if (IS_ERR(trap)) + goto out; goto unlock; } else if (err) { goto cleanup; @@ -803,6 +805,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c) ovl_set_flag(OVL_WHITEOUTS, inode); unlock: unlock_rename(c->workdir, c->destdir); +out: ovl_end_write(c->dentry); return err; diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index aab3f5d93556..0f8b4a719237 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -1180,6 +1180,10 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, } trap = lock_rename(new_upperdir, old_upperdir); + if (IS_ERR(trap)) { + err = PTR_ERR(trap); + goto out_revert_creds; + } olddentry = ovl_lookup_upper(ofs, old->d_name.name, old_upperdir, old->d_name.len); diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index a0967bb25003..fc3a6ff648bd 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -439,8 +439,10 @@ static bool ovl_workdir_ok(struct dentry *workdir, struct dentry *upperdir) bool ok = false; if (workdir != upperdir) { - ok = (lock_rename(workdir, upperdir) == NULL); - unlock_rename(workdir, upperdir); + struct dentry *trap = lock_rename(workdir, upperdir); + if (!IS_ERR(trap)) + unlock_rename(workdir, upperdir); + ok = (trap == NULL); } return ok; } diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 50a201e9cd39..7b667345e673 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -1198,12 +1198,17 @@ void ovl_nlink_end(struct dentry *dentry) int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir) { + struct dentry *trap; + /* Workdir should not be the same as upperdir */ if (workdir == upperdir) goto err; /* Workdir should not be subdir of upperdir and vice versa */ - if (lock_rename(workdir, upperdir) != NULL) + trap = lock_rename(workdir, upperdir); + if (IS_ERR(trap)) + goto err; + if (trap) goto err_unlock; return 0; diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c index c53dea5598fc..4cf8523ad038 100644 --- a/fs/smb/server/vfs.c +++ b/fs/smb/server/vfs.c @@ -708,6 +708,10 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path, goto out2; trap = lock_rename_child(old_child, new_path.dentry); + if (IS_ERR(trap)) { + err = PTR_ERR(trap); + goto out_drop_write; + } old_parent = dget(old_child->d_parent); if (d_unhashed(old_child)) { @@ -770,6 +774,7 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path, out3: dput(old_parent); unlock_rename(old_parent, new_path.dentry); +out_drop_write: mnt_drop_write(old_path->mnt); out2: path_put(&new_path);