From patchwork Wed Jul 10 02:26:27 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zhihao Cheng X-Patchwork-Id: 1958647 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=PrEJVDl9; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.infradead.org (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org; envelope-from=linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org; receiver=patchwork.ozlabs.org) Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WJhdP6xYJz20MN for ; Wed, 10 Jul 2024 12:29:28 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:CC:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=HfBq4prx5TGRvetvcQIbN9MLdbTNvhPt7GN+u6TM5QU=; b=PrEJVDl9mTKIyE j4e/WU9D2WcrS4LbwgkV5fiad0wRHJjR5BwSU6i4jGNishhQjKkZMg4Syg2cyOf8nWGx2HeExueIS +FMKZE+hDqDs21faO7WQBFMPOsr7Fd3VVVXyDaY2r0FNuzuuQDCIno/g9x5lhrY3GNLumYI+w15Z9 38nBaW1JgxRDS7l82z30LMlXBz5Jto+QVLStnKO3eCX9EQ3E2fWV1tLgj3/X0xWf6Qqyst42Jmpzq SSTmE1xK9DD7Hm12EJMkgFZxHktCS++VXw6SItF6ckFmWNqArOv/y6ODFcCcSqOWkpVX3K/4kmhd8 CqsnPeNuH7dyRzaaN8kQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sRN4w-00000009BmX-35qV; Wed, 10 Jul 2024 02:29:10 +0000 Received: from szxga08-in.huawei.com ([45.249.212.255]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sRN4t-00000009Bks-0Mcj for linux-mtd@lists.infradead.org; Wed, 10 Jul 2024 02:29:08 +0000 Received: from mail.maildlp.com (unknown [172.19.163.174]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4WJhWJ03bpz1T4k8; Wed, 10 Jul 2024 10:24:12 +0800 (CST) Received: from kwepemm000013.china.huawei.com (unknown [7.193.23.81]) by mail.maildlp.com (Postfix) with ESMTPS id 15D9E140428; Wed, 10 Jul 2024 10:28:54 +0800 (CST) Received: from huawei.com (10.175.104.67) by kwepemm000013.china.huawei.com (7.193.23.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Wed, 10 Jul 2024 10:28:52 +0800 From: Zhihao Cheng To: CC: Subject: [PATCH 1/2] ubifs: ubifs_jnl_write_inode: Only check once for the limitation of xattr count Date: Wed, 10 Jul 2024 10:26:27 +0800 Message-ID: <20240710022628.1227996-2-chengzhihao1@huawei.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240710022628.1227996-1-chengzhihao1@huawei.com> References: <20240710022628.1227996-1-chengzhihao1@huawei.com> MIME-Version: 1.0 X-Originating-IP: [10.175.104.67] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To kwepemm000013.china.huawei.com (7.193.23.81) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240709_192907_316598_A3BAD28B X-CRM114-Status: GOOD ( 12.28 ) X-Spam-Score: -4.2 (----) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: No need to check the limitation of xattr count every time in function ubifs_jnl_write_inode(), because the 'ui->xattr_cnt' won't be modified by others in the inode evicting process. Signed-off-by: Zhihao Cheng --- fs/ubifs/journal.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) Content analysis details: (-4.2 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at https://www.dnswl.org/, medium trust [45.249.212.255 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-mtd" Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org No need to check the limitation of xattr count every time in function ubifs_jnl_write_inode(), because the 'ui->xattr_cnt' won't be modified by others in the inode evicting process. Signed-off-by: Zhihao Cheng --- fs/ubifs/journal.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c index 4a35f9e8f668..a5c7c499cc29 100644 --- a/fs/ubifs/journal.c +++ b/fs/ubifs/journal.c @@ -981,6 +981,14 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) dbg_jnl("ino %lu, nlink %u", inode->i_ino, inode->i_nlink); + if (kill_xattrs) { + if (ui->xattr_cnt > ubifs_xattr_max_cnt(c)) { + ubifs_err(c, "Cannot delete inode, it has too much xattrs!"); + ubifs_ro_mode(c, err); + return -EPERM; + } + } + /* * If the inode is being deleted, do not write the attached data. No * need to synchronize the write-buffer either. @@ -1012,12 +1020,6 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) struct inode *xino; struct ubifs_dent_node *xent, *pxent = NULL; - if (ui->xattr_cnt > ubifs_xattr_max_cnt(c)) { - err = -EPERM; - ubifs_err(c, "Cannot delete inode, it has too much xattrs!"); - goto out_release; - } - lowest_xent_key(c, &key, inode->i_ino); while (1) { xent = ubifs_tnc_next_ent(c, &key, &nm); From patchwork Wed Jul 10 02:26:28 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Zhihao Cheng X-Patchwork-Id: 1958645 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=yrB2CRu6; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.infradead.org (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org; envelope-from=linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org; receiver=patchwork.ozlabs.org) Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WJhdP6sHGz1xqc for ; Wed, 10 Jul 2024 12:29:28 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:CC:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=oy6KRg78RSddlZWx80KyF4+NfyJYwlszd8z0bMLu3tE=; b=yrB2CRu65Y3YTU RtpJlSeaZJApYBYPGbqz2Ap/LM0SihXO0zoT0nhjMELLxPpKpaUNjO7slzMTmwKDGBo5IaAHKFLxg tf5lI4EkhHlbk3dm+Ayc9eXvwv/zagS7hoGqj++gdxI7wcPLKFbql/WhtkMb/flgBiCuMtyYhjv7q k1cRuncYtdTPtgkB4xXzJ5deY+dSNGrEKErKIeSnxs9UcfTnB6hGVAMYNogtQ1Csd6fIbhpJa+mcQ wih9IMJvNWVNaZfJxIhpLfmGa/y+4MpqRPW4UG2AgdzXVK2f4HdDoY+OKWW+3ybF8Qa25Bfv0COKa yR3+ZNTNhzBNtFLyICKQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sRN4v-00000009BmJ-0hCq; Wed, 10 Jul 2024 02:29:09 +0000 Received: from szxga08-in.huawei.com ([45.249.212.255]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sRN4r-00000009Bku-0QFw for linux-mtd@lists.infradead.org; Wed, 10 Jul 2024 02:29:07 +0000 Received: from mail.maildlp.com (unknown [172.19.163.174]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4WJhWJ0Tslz1T5yP; Wed, 10 Jul 2024 10:24:12 +0800 (CST) Received: from kwepemm000013.china.huawei.com (unknown [7.193.23.81]) by mail.maildlp.com (Postfix) with ESMTPS id 23BDC14041B; Wed, 10 Jul 2024 10:28:54 +0800 (CST) Received: from huawei.com (10.175.104.67) by kwepemm000013.china.huawei.com (7.193.23.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Wed, 10 Jul 2024 10:28:53 +0800 From: Zhihao Cheng To: CC: Subject: [PATCH 2/2] ubifs: Fix ABBA deadlock between inode reclaiming and deleted inode writing Date: Wed, 10 Jul 2024 10:26:28 +0800 Message-ID: <20240710022628.1227996-3-chengzhihao1@huawei.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240710022628.1227996-1-chengzhihao1@huawei.com> References: <20240710022628.1227996-1-chengzhihao1@huawei.com> MIME-Version: 1.0 X-Originating-IP: [10.175.104.67] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To kwepemm000013.china.huawei.com (7.193.23.81) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240709_192905_509926_52CD6E2D X-CRM114-Status: GOOD ( 18.59 ) X-Spam-Score: -4.2 (----) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: The inodes reclaiming process(See function prune_icache_sb) collects all reclaimable inodes and mark them with I_FREEING flag at first, at that time, other processes will be stuck if they try getting [...] Content analysis details: (-4.2 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at https://www.dnswl.org/, medium trust [45.249.212.255 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-mtd" Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org The inodes reclaiming process(See function prune_icache_sb) collects all reclaimable inodes and mark them with I_FREEING flag at first, at that time, other processes will be stuck if they try getting these inodes(See function find_inode_fast), then the reclaiming process destroy the inodes. In deleted inode writing function ubifs_jnl_write_inode(), UBIFS holds BASEHD's wbuf->io_mutex while traversing all xattr inodes, which could race with inodes reclaiming process(The reclaiming process could try locking BASEHD's wbuf->io_mutex in inode evicting function), then an ABBA deadlock problem would happens as following: 1. File A has inode ia and a xattr(with inode ixa), regular file B has inode ib and a xattr. 2. getfattr(A, xattr_buf) // ixa is added into lru // lru->ixa 3. Then, following three processes running like this: PA PB PC echo 2 > /proc/sys/vm/drop_caches // ib and ia area added into lru, lru->ixa->ib->ia shrink_slab prune_icache_sb list_lru_walk_one inode_lru_isolate ixa->inode->i_state |= I_FREEING // set inode state inode_lru_isolate __iget(ib) spin_unlock(&ib->i_lock) spin_unlock(lru_lock) rm file B ib->nlink = 0 iput(ib) rm file A iput(ia) ubifs_evict_inode(ia) ubifs_jnl_delete_inode(ia) ubifs_jnl_write_inode(ia) make_reservation(BASEHD) // Lock wbuf->io_mutex ubifs_iget(ixa->i_ino) iget_locked find_inode_fast __wait_on_freeing_inode(ixa) | iput(ib) // ib->nlink is 0, do evict | ubifs_evict_inode | ubifs_jnl_delete_inode(ib) ↓ ubifs_jnl_write_inode ABBA deadlock ←-----make_reservation(BASEHD) dispose_list // cannot be executed by prune_icache_sb wake_up_bit(&inode->i_state) The root cause is that UBIFS tries getting xattr inode with holding BASEHD's wbuf->io_mutex, so the problem can be fixed by getting all xattr inodes before locking BASEHD's wbuf->io_mutex. Fixes: 7959cf3a7506 ("ubifs: journal: Handle xattrs like files") Cc: stable@vger.kernel.org Link: https://bugzilla.kernel.org/show_bug.cgi?id=219022 Signed-off-by: Zhihao Cheng --- fs/ubifs/journal.c | 110 +++++++++++++++++++++++++++------------------ 1 file changed, 67 insertions(+), 43 deletions(-) diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c index a5c7c499cc29..7556d87934b7 100644 --- a/fs/ubifs/journal.c +++ b/fs/ubifs/journal.c @@ -971,7 +971,8 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode, */ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) { - int err, lnum, offs; + int err, lnum, offs, i = 0; + struct inode **xinos = NULL; struct ubifs_ino_node *ino, *ino_start; struct ubifs_inode *ui = ubifs_inode(inode); int sync = 0, write_len = 0, ilen = UBIFS_INO_NODE_SZ; @@ -982,44 +983,24 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) dbg_jnl("ino %lu, nlink %u", inode->i_ino, inode->i_nlink); if (kill_xattrs) { + union ubifs_key key; + struct fscrypt_name nm = {0}; + struct ubifs_dent_node *xent, *pxent = NULL; + struct super_block *sb = c->vfs_sb; + if (ui->xattr_cnt > ubifs_xattr_max_cnt(c)) { ubifs_err(c, "Cannot delete inode, it has too much xattrs!"); ubifs_ro_mode(c, err); return -EPERM; } - } - - /* - * If the inode is being deleted, do not write the attached data. No - * need to synchronize the write-buffer either. - */ - if (!last_reference) { - ilen += ui->data_len; - sync = IS_SYNC(inode); - } else if (kill_xattrs) { - write_len += UBIFS_INO_NODE_SZ * ui->xattr_cnt; - } - - if (ubifs_authenticated(c)) - write_len += ALIGN(ilen, 8) + ubifs_auth_node_sz(c); - else - write_len += ilen; - ino_start = ino = kmalloc(write_len, GFP_NOFS); - if (!ino) - return -ENOMEM; - - /* Make reservation before allocating sequence numbers */ - err = make_reservation(c, BASEHD, write_len); - if (err) - goto out_free; - - if (kill_xattrs) { - union ubifs_key key; - struct fscrypt_name nm = {0}; - struct inode *xino; - struct ubifs_dent_node *xent, *pxent = NULL; + xinos = kmalloc(sizeof(*xinos) * ui->xattr_cnt, GFP_NOFS); + if (!xinos) { + ubifs_err(c, "Cannot allocate memory for xattr inodes"); + return -ENOMEM; + } + i = 0; lowest_xent_key(c, &key, inode->i_ino); while (1) { xent = ubifs_tnc_next_ent(c, &key, &nm); @@ -1029,34 +1010,71 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) break; kfree(pxent); - goto out_release; + goto out_iput; } fname_name(&nm) = xent->name; fname_len(&nm) = le16_to_cpu(xent->nlen); - xino = ubifs_iget(c->vfs_sb, le64_to_cpu(xent->inum)); - if (IS_ERR(xino)) { - err = PTR_ERR(xino); + xinos[i] = ubifs_iget(sb, le64_to_cpu(xent->inum)); + if (IS_ERR(xinos[i])) { + err = PTR_ERR(xinos[i]); ubifs_err(c, "dead directory entry '%s', error %d", xent->name, err); ubifs_ro_mode(c, err); kfree(pxent); kfree(xent); - goto out_release; + goto out_iput; } - ubifs_assert(c, ubifs_inode(xino)->xattr); - - clear_nlink(xino); - pack_inode(c, ino, xino, 0); - ino = (void *)ino + UBIFS_INO_NODE_SZ; - iput(xino); + ubifs_assert(c, ubifs_inode(xinos[i])->xattr); + ubifs_assert(c, i < ui->xattr_cnt); kfree(pxent); pxent = xent; key_read(c, &xent->key, &key); + i++; } kfree(pxent); + ubifs_assert(c, i == ui->xattr_cnt); + } + + /* + * If the inode is being deleted, do not write the attached data. No + * need to synchronize the write-buffer either. + */ + if (!last_reference) { + ilen += ui->data_len; + sync = IS_SYNC(inode); + } else if (kill_xattrs) { + write_len += UBIFS_INO_NODE_SZ * ui->xattr_cnt; + } + + if (ubifs_authenticated(c)) + write_len += ALIGN(ilen, 8) + ubifs_auth_node_sz(c); + else + write_len += ilen; + + ino_start = ino = kmalloc(write_len, GFP_NOFS); + if (!ino) { + err = -ENOMEM; + goto out_iput; + } + + /* Make reservation before allocating sequence numbers */ + err = make_reservation(c, BASEHD, write_len); + if (err) + goto out_free; + + if (kill_xattrs) { + for (i = 0; i < ui->xattr_cnt; i++) { + clear_nlink(xinos[i]); + pack_inode(c, ino, xinos[i], 0); + ino = (void *)ino + UBIFS_INO_NODE_SZ; + iput(xinos[i]); + } + + kfree(xinos); + xinos = NULL; } pack_inode(c, ino, inode, 1); @@ -1103,6 +1121,12 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) finish_reservation(c); out_free: kfree(ino_start); +out_iput: + if (xinos) { + while (--i >= 0) + iput(xinos[i]); + kfree(xinos); + } return err; }