From nobody Tue Sep 9 21:32:27 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C4C99C7EE2D for ; Wed, 1 Mar 2023 12:06:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229806AbjCAMGS (ORCPT ); Wed, 1 Mar 2023 07:06:18 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43394 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229653AbjCAMGN (ORCPT ); Wed, 1 Mar 2023 07:06:13 -0500 Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F008236FCF for ; Wed, 1 Mar 2023 04:06:11 -0800 (PST) Received: from kwepemm600013.china.huawei.com (unknown [172.30.72.57]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4PRXv00sPBz16NxP; Wed, 1 Mar 2023 20:03:24 +0800 (CST) Received: from huawei.com (10.175.127.227) by kwepemm600013.china.huawei.com (7.193.23.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.21; Wed, 1 Mar 2023 20:06:02 +0800 From: Zhihao Cheng To: , , CC: , , Subject: [PATCH -next 1/2] Revert "ubifs: dirty_cow_znode: Fix memleak in error handling path" Date: Wed, 1 Mar 2023 20:29:18 +0800 Message-ID: <20230301122919.266818-2-chengzhihao1@huawei.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20230301122919.266818-1-chengzhihao1@huawei.com> References: <20230301122919.266818-1-chengzhihao1@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Originating-IP: [10.175.127.227] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To kwepemm600013.china.huawei.com (7.193.23.68) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" This reverts commit 122deabfe1428 (ubifs: dirty_cow_znode: Fix memleak in error handling path). After commit 122deabfe1428 applied, if insert_old_idx() failed, old index neither exists in TNC nor in old-index tree. Which means that old index node could be overwritten in layout_leb_in_gaps(), then ubifs image will be corrupted in power-cut. Fixes: 122deabfe1428 (ubifs: dirty_cow_znode: Fix memleak ... path) Signed-off-by: Zhihao Cheng --- fs/ubifs/tnc.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c index 2469f72eeaab..2df56bbc6865 100644 --- a/fs/ubifs/tnc.c +++ b/fs/ubifs/tnc.c @@ -267,18 +267,11 @@ static struct ubifs_znode *dirty_cow_znode(struct ubi= fs_info *c, if (zbr->len) { err =3D insert_old_idx(c, zbr->lnum, zbr->offs); if (unlikely(err)) - /* - * Obsolete znodes will be freed by tnc_destroy_cnext() - * or free_obsolete_znodes(), copied up znodes should - * be added back to tnc and freed by - * ubifs_destroy_tnc_subtree(). - */ - goto out; + return ERR_PTR(err); err =3D add_idx_dirt(c, zbr->lnum, zbr->len); } else err =3D 0; =20 -out: zbr->znode =3D zn; zbr->lnum =3D 0; zbr->offs =3D 0; --=20 2.31.1 From nobody Tue Sep 9 21:32:27 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C9497C64EC7 for ; Wed, 1 Mar 2023 12:06:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229788AbjCAMGV (ORCPT ); Wed, 1 Mar 2023 07:06:21 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43404 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229699AbjCAMGO (ORCPT ); Wed, 1 Mar 2023 07:06:14 -0500 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E348136FCC for ; Wed, 1 Mar 2023 04:06:11 -0800 (PST) Received: from kwepemm600013.china.huawei.com (unknown [172.30.72.55]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4PRXtj1XHPzSkRq; Wed, 1 Mar 2023 20:03:09 +0800 (CST) Received: from huawei.com (10.175.127.227) by kwepemm600013.china.huawei.com (7.193.23.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.21; Wed, 1 Mar 2023 20:06:03 +0800 From: Zhihao Cheng To: , , CC: , , Subject: [PATCH -next 2/2] ubifs: dirty_cow_znode: Fix memleak when insert_old_idx() failed Date: Wed, 1 Mar 2023 20:29:19 +0800 Message-ID: <20230301122919.266818-3-chengzhihao1@huawei.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20230301122919.266818-1-chengzhihao1@huawei.com> References: <20230301122919.266818-1-chengzhihao1@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Originating-IP: [10.175.127.227] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To kwepemm600013.china.huawei.com (7.193.23.68) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" Following process will cause a memleak for copied up znode: dirty_cow_znode zn =3D copy_znode(c, znode); err =3D insert_old_idx(c, zbr->lnum, zbr->offs); if (unlikely(err)) return ERR_PTR(err); // No one refers to zn. Fetch a reproducer in [Link]. Function copy_znode() is split into 2 parts: resource allocation and znode replacement, insert_old_idx() is split in similar way, so resource cleanup could be done in error handling path without corrupting metadata(mem & disk). It's okay that old index inserting is put behind of add_idx_dirt(), old index is used in layout_leb_in_gaps(), so the two processes do not depend on each other. Link: https://bugzilla.kernel.org/show_bug.cgi?id=3D216705 Fixes: 1e51764a3c2a ("UBIFS: add new flash file system") Signed-off-by: Zhihao Cheng --- fs/ubifs/tnc.c | 137 +++++++++++++++++++++++++++++++------------------ 1 file changed, 87 insertions(+), 50 deletions(-) diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c index 2df56bbc6865..6b7d95b65f4b 100644 --- a/fs/ubifs/tnc.c +++ b/fs/ubifs/tnc.c @@ -44,6 +44,33 @@ enum { NOT_ON_MEDIA =3D 3, }; =20 +static void do_insert_old_idx(struct ubifs_info *c, + struct ubifs_old_idx *old_idx) +{ + struct ubifs_old_idx *o; + struct rb_node **p, *parent =3D NULL; + + p =3D &c->old_idx.rb_node; + while (*p) { + parent =3D *p; + o =3D rb_entry(parent, struct ubifs_old_idx, rb); + if (old_idx->lnum < o->lnum) + p =3D &(*p)->rb_left; + else if (old_idx->lnum > o->lnum) + p =3D &(*p)->rb_right; + else if (old_idx->offs < o->offs) + p =3D &(*p)->rb_left; + else if (old_idx->offs > o->offs) + p =3D &(*p)->rb_right; + else { + ubifs_err(c, "old idx added twice!"); + kfree(old_idx); + } + } + rb_link_node(&old_idx->rb, parent, p); + rb_insert_color(&old_idx->rb, &c->old_idx); +} + /** * insert_old_idx - record an index node obsoleted since the last commit s= tart. * @c: UBIFS file-system description object @@ -69,35 +96,15 @@ enum { */ static int insert_old_idx(struct ubifs_info *c, int lnum, int offs) { - struct ubifs_old_idx *old_idx, *o; - struct rb_node **p, *parent =3D NULL; + struct ubifs_old_idx *old_idx; =20 old_idx =3D kmalloc(sizeof(struct ubifs_old_idx), GFP_NOFS); if (unlikely(!old_idx)) return -ENOMEM; old_idx->lnum =3D lnum; old_idx->offs =3D offs; + do_insert_old_idx(c, old_idx); =20 - p =3D &c->old_idx.rb_node; - while (*p) { - parent =3D *p; - o =3D rb_entry(parent, struct ubifs_old_idx, rb); - if (lnum < o->lnum) - p =3D &(*p)->rb_left; - else if (lnum > o->lnum) - p =3D &(*p)->rb_right; - else if (offs < o->offs) - p =3D &(*p)->rb_left; - else if (offs > o->offs) - p =3D &(*p)->rb_right; - else { - ubifs_err(c, "old idx added twice!"); - kfree(old_idx); - return 0; - } - } - rb_link_node(&old_idx->rb, parent, p); - rb_insert_color(&old_idx->rb, &c->old_idx); return 0; } =20 @@ -199,23 +206,6 @@ static struct ubifs_znode *copy_znode(struct ubifs_inf= o *c, __set_bit(DIRTY_ZNODE, &zn->flags); __clear_bit(COW_ZNODE, &zn->flags); =20 - ubifs_assert(c, !ubifs_zn_obsolete(znode)); - __set_bit(OBSOLETE_ZNODE, &znode->flags); - - if (znode->level !=3D 0) { - int i; - const int n =3D zn->child_cnt; - - /* The children now have new parent */ - for (i =3D 0; i < n; i++) { - struct ubifs_zbranch *zbr =3D &zn->zbranch[i]; - - if (zbr->znode) - zbr->znode->parent =3D zn; - } - } - - atomic_long_inc(&c->dirty_zn_cnt); return zn; } =20 @@ -233,6 +223,42 @@ static int add_idx_dirt(struct ubifs_info *c, int lnum= , int dirt) return ubifs_add_dirt(c, lnum, dirt); } =20 +/** + * replace_znode - replace old znode with new znode. + * @c: UBIFS file-system description object + * @new_zn: new znode + * @old_zn: old znode + * @zbr: the branch of parent znode + * + * Replace old znode with new znode in TNC. + */ +static void replace_znode(struct ubifs_info *c, struct ubifs_znode *new_zn, + struct ubifs_znode *old_zn, struct ubifs_zbranch *zbr) +{ + ubifs_assert(c, !ubifs_zn_obsolete(old_zn)); + __set_bit(OBSOLETE_ZNODE, &old_zn->flags); + + if (old_zn->level !=3D 0) { + int i; + const int n =3D new_zn->child_cnt; + + /* The children now have new parent */ + for (i =3D 0; i < n; i++) { + struct ubifs_zbranch *child =3D &new_zn->zbranch[i]; + + if (child->znode) + child->znode->parent =3D new_zn; + } + } + + zbr->znode =3D new_zn; + zbr->lnum =3D 0; + zbr->offs =3D 0; + zbr->len =3D 0; + + atomic_long_inc(&c->dirty_zn_cnt); +} + /** * dirty_cow_znode - ensure a znode is not being committed. * @c: UBIFS file-system description object @@ -265,21 +291,32 @@ static struct ubifs_znode *dirty_cow_znode(struct ubi= fs_info *c, return zn; =20 if (zbr->len) { - err =3D insert_old_idx(c, zbr->lnum, zbr->offs); - if (unlikely(err)) - return ERR_PTR(err); + struct ubifs_old_idx *old_idx; + + old_idx =3D kmalloc(sizeof(struct ubifs_old_idx), GFP_NOFS); + if (unlikely(!old_idx)) { + err =3D -ENOMEM; + goto out; + } + old_idx->lnum =3D zbr->lnum; + old_idx->offs =3D zbr->offs; + err =3D add_idx_dirt(c, zbr->lnum, zbr->len); - } else - err =3D 0; + if (err) { + kfree(old_idx); + goto out; + } =20 - zbr->znode =3D zn; - zbr->lnum =3D 0; - zbr->offs =3D 0; - zbr->len =3D 0; + do_insert_old_idx(c, old_idx); + } + + replace_znode(c, zn, znode, zbr); =20 - if (unlikely(err)) - return ERR_PTR(err); return zn; + +out: + kfree(zn); + return ERR_PTR(err); } =20 /** --=20 2.31.1