From patchwork Wed Oct 18 12:16:18 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Wang Zhaolong X-Patchwork-Id: 1850775 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=gTOUW7PC; 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 4S9V921q1Gz20Zj for ; Wed, 18 Oct 2023 23:12:36 +1100 (AEDT) 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: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:In-Reply-To:References: List-Owner; bh=UYUjN5/xKf15ET1W3hSvRg5UEGYddstsS5ARU8sxTjc=; b=gTOUW7PC+rRN0y Ix5+erdZfGVBN9/Nw4sP19TgdTbEKqJqcta0BUnQ2KaT5tdkRHRn4iyZNDBXEkdlNS2n8sTnWaQhn cNQ7f6Vn4YdkyV5gcZq0D2ix95pDaN3wh8YUdAo8Z152BTPkz/E6lxgR3tBgIVGxWpajhaoGZSYK7 0NO1fttgsa/1UO+o1WdYLENIT3M6KV6tM3FAyTWzhOqHzmuex0jEjAH2OZfLaZfOTlxTNDbIC/o6l EI1FieCAjUuJClbSTxfXIb7/GvO5LZDBVGSMYn6YSLTO3Z06MOmuYs3C3irOZDl3sxq0B2nOVeoYq YYhJbrFs7QEMx8dNGIdQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qt5Oz-00Eczd-2V; Wed, 18 Oct 2023 12:11:53 +0000 Received: from szxga01-in.huawei.com ([45.249.212.187]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qt5Ov-00EcxC-2l for linux-mtd@lists.infradead.org; Wed, 18 Oct 2023 12:11:52 +0000 Received: from dggpemm500009.china.huawei.com (unknown [172.30.72.53]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4S9V2F6kF4zvQJ3; Wed, 18 Oct 2023 20:06:45 +0800 (CST) Received: from huawei.com (10.175.127.227) by dggpemm500009.china.huawei.com (7.185.36.225) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.31; Wed, 18 Oct 2023 20:11:30 +0800 From: ZhaoLong Wang To: , , , , CC: , , , , , Subject: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier Date: Wed, 18 Oct 2023 20:16:18 +0800 Message-ID: <20231018121618.778385-1-wangzhaolong1@huawei.com> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 X-Originating-IP: [10.175.127.227] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To dggpemm500009.china.huawei.com (7.185.36.225) X-CFilter-Loop: Reflected X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231018_051150_358571_FB8AE4F4 X-CRM114-Status: GOOD ( 20.77 ) X-Spam-Score: -2.3 (--) X-Spam-Report: =?unknown-8bit?q?Spam_detection_software=2C_running_on_the_sy?= =?unknown-8bit?q?stem_=22bombadil=2Einfradead=2Eorg=22=2C?= =?unknown-8bit?q?_has_NOT_identified_this_incoming_email_as_spam=2E__The_ori?= =?unknown-8bit?q?ginal?= =?unknown-8bit?q?_message_has_been_attached_to_this_so_you_can_view_it_or_la?= =?unknown-8bit?q?bel?= =?unknown-8bit?q?_similar_future_email=2E__If_you_have_any_questions=2C_see?= =?unknown-8bit?q?_the_administrator_of_that_system_for_details=2E?= =?unknown-8bit?q?_?= =?unknown-8bit?q?_Content_preview=3A__If_both_flt=2Eko_and_gluebi=2Eko_are_l?= =?unknown-8bit?q?oaded=2C_the_notiier_of_ftl?= =?unknown-8bit?q?_triggers_NULL_pointer_dereference_when_trying_to_access_?= =?unknown-8bit?q?=E2=80=98gluebi-=3Edesc=E2=80=99?= =?unknown-8bit?q?_in_gluebi=5Fread=28=29=2E_ubi=5Fgluebi=5Finit_ubi=5Fregist?= =?unknown-8bit?q?er=5Fvolume=5Fnotifier_ubi=5Fenumerate=5Fvolumes?= =?unknown-8bit?q?_ubi=5Fnotify=5Fall_gluebi=5Fnotify_nb-=3Enotifier=5Fcall?= =?unknown-8bit?q?=28=29_gluebi=5Fcreate_mtd=5Fdevice=5Fregister?= =?unknown-8bit?q?_mtd=5Fdevice=5Fparse=5Fregister_add=5Fmtd=5Fdevice_blktran?= =?unknown-8bit?q?s=5F_=5B=2E=2E=2E=5D_?= =?unknown-8bit?q?_?= =?unknown-8bit?q?_Content_analysis_details=3A___=28-2=2E3_points=2C_5=2E0_re?= =?unknown-8bit?q?quired=29?= =?unknown-8bit?q?_?= =?unknown-8bit?q?_pts_rule_name______________description?= =?unknown-8bit?q?_----_----------------------_------------------------------?= =?unknown-8bit?q?--------------------?= =?unknown-8bit?q?_-2=2E3_RCVD=5FIN=5FDNSWL=5FMED______RBL=3A_Sender_listed_a?= =?unknown-8bit?q?t_https=3A//www=2Ednswl=2Eorg/=2C?= =?unknown-8bit?q?_medium_trust?= =?unknown-8bit?q?_=5B45=2E249=2E212=2E187_listed_in_list=2Ednswl=2Eorg=5D?= =?unknown-8bit?q?_-0=2E0_SPF=5FPASS_______________SPF=3A_sender_matches_SPF_?= =?unknown-8bit?q?record?= =?unknown-8bit?q?_0=2E0_SPF=5FHELO=5FNONE__________SPF=3A_HELO_does_not_publ?= =?unknown-8bit?q?ish_an_SPF_Record?= =?unknown-8bit?q?_0=2E0_RCVD=5FIN=5FMSPIKE=5FH5______RBL=3A_Excellent_reputa?= =?unknown-8bit?q?tion_=28+5=29?= =?unknown-8bit?q?_=5B45=2E249=2E212=2E187_listed_in_wl=2Emailspike=2Enet=5D?= =?unknown-8bit?q?_0=2E0_RCVD=5FIN=5FMSPIKE=5FWL______Mailspike_good_senders?= 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 If both flt.ko and gluebi.ko are loaded, the notiier of ftl triggers NULL pointer dereference when trying to access ‘gluebi->desc’ in gluebi_read(). ubi_gluebi_init ubi_register_volume_notifier ubi_enumerate_volumes ubi_notify_all gluebi_notify nb->notifier_call() gluebi_create mtd_device_register mtd_device_parse_register add_mtd_device blktrans_notify_add not->add() ftl_add_mtd tr->add_mtd() scan_header mtd_read mtd_read mtd_read_oob gluebi_read mtd->read() gluebi->desc - NULL Detailed reproduction information available at the link[1], In the normal case, obtain gluebi->desc in the gluebi_get_device(), and accesses gluebi->desc in the gluebi_read(). However, gluebi_get_device() is not executed in advance in the ftl_add_mtd() process, which leads to NULL pointer dereference. The value of gluebi->desc may also be a negative error code, which triggers the page fault error. This patch has the following modifications: 1. Do not assign gluebi->desc to the error code. Use the NULL instead. 2. Always check the validity of gluebi->desc in gluebi_read() If the gluebi->desc is NULL, try to get MTD device. Such a modification currently works because the mutex "mtd_table_mutex" is held on all necessary paths, including the ftl_add_mtd() call path, open and close paths. Therefore, many race condition can be avoided. Fixes: 2ba3d76a1e29 ("UBI: make gluebi a separate module") Link: https://bugzilla.kernel.org/show_bug.cgi?id=217992 [1] Signed-off-by: ZhaoLong Wang Reviewed-by: Zhihao Cheng --- drivers/mtd/ubi/gluebi.c | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/drivers/mtd/ubi/gluebi.c b/drivers/mtd/ubi/gluebi.c index 1b980d15d9fb..0ca7f104adbf 100644 --- a/drivers/mtd/ubi/gluebi.c +++ b/drivers/mtd/ubi/gluebi.c @@ -85,6 +85,7 @@ static int gluebi_get_device(struct mtd_info *mtd) { struct gluebi_device *gluebi; int ubi_mode = UBI_READONLY; + struct ubi_volume_desc *vdesc; if (mtd->flags & MTD_WRITEABLE) ubi_mode = UBI_READWRITE; @@ -109,12 +110,14 @@ static int gluebi_get_device(struct mtd_info *mtd) * This is the first reference to this UBI volume via the MTD device * interface. Open the corresponding volume in read-write mode. */ - gluebi->desc = ubi_open_volume(gluebi->ubi_num, gluebi->vol_id, + vdesc = ubi_open_volume(gluebi->ubi_num, gluebi->vol_id, ubi_mode); - if (IS_ERR(gluebi->desc)) { + if (IS_ERR(vdesc)) { + gluebi->desc = NULL; mutex_unlock(&devices_mutex); - return PTR_ERR(gluebi->desc); + return PTR_ERR(vdesc); } + gluebi->desc = vdesc; gluebi->refcnt += 1; mutex_unlock(&devices_mutex); return 0; @@ -134,8 +137,10 @@ static void gluebi_put_device(struct mtd_info *mtd) gluebi = container_of(mtd, struct gluebi_device, mtd); mutex_lock(&devices_mutex); gluebi->refcnt -= 1; - if (gluebi->refcnt == 0) + if (gluebi->refcnt == 0) { ubi_close_volume(gluebi->desc); + gluebi->desc = NULL; + } mutex_unlock(&devices_mutex); } @@ -154,9 +159,26 @@ static int gluebi_read(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen, unsigned char *buf) { int err = 0, lnum, offs, bytes_left; - struct gluebi_device *gluebi; + struct gluebi_device *gluebi = container_of(mtd, struct gluebi_device, + mtd); + int no_desc = gluebi->desc == NULL ? 1 : 0; + + /** + * In normal case, the UBI volume desc has been initialized by + * ->_get_device(). However, in the ftl notifier process, the + * ->_get_device() is not executed in advance and the MTD device + * is directly scanned which cause NULL pointer dereference. + * Therefore, try to get the MTD device here. + */ + if (unlikely(no_desc)) { + err = __get_mtd_device(mtd); + if (err) { + err_msg("cannot get MTD device %d, UBI device %d, volume %d, error %d", + mtd->index, gluebi->ubi_num, gluebi->vol_id, err); + return err; + } + } - gluebi = container_of(mtd, struct gluebi_device, mtd); lnum = div_u64_rem(from, mtd->erasesize, &offs); bytes_left = len; while (bytes_left) { @@ -176,6 +198,9 @@ static int gluebi_read(struct mtd_info *mtd, loff_t from, size_t len, } *retlen = len - bytes_left; + + if (unlikely(no_desc)) + __put_mtd_device(mtd); return err; }