From patchwork Mon Jul 31 09:09:03 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Miquel Raynal X-Patchwork-Id: 1814879 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org 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=) 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=1e5TDW6H; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=bootlin.com header.i=@bootlin.com header.a=rsa-sha256 header.s=gm1 header.b=ks963aFF; dkim-atps=neutral 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 (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4RDsrN0B61z20G9 for ; Mon, 31 Jul 2023 19:09:40 +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: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=5aSCzvtm5AUdSdaitgxCpP35UJP9SNIkfJvIUvUDwA0=; b=1e5TDW6Hns46YN MXGyRaqCaIVGOSMDsWw5f3nAzJn+Sse5oWKx/72YoPbt5NcUIMs670OvETpArFei0q5NfOf8CHSOo 1fvGYYi3ttd7iotLt/TE/1gsP4JBCk9eX9Z3fjPHXE9los5dCL4ZwX3VmxIjeaTCVAHfk8NLl6ccL XhZ8yp0pdPQ39JBdpoSNBJ3JPHqylBDd0OZ3ojBFVbikQf0vNXYTahnyOhGNX+K2NusPKFequpMCl pUG8zucIIDGpFQEyWVXwOS2FshZEX0bAcGqauA75vzPorj/pcx8vLIHGf7YjasdZbZtvZmYKQJrQW M9qWQHFBUtYzBWBKbQow==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qQOtu-00EiiU-32; Mon, 31 Jul 2023 09:09:14 +0000 Received: from relay1-d.mail.gandi.net ([2001:4b98:dc4:8::221]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qQOtr-00EigH-1o for linux-mtd@lists.infradead.org; Mon, 31 Jul 2023 09:09:13 +0000 Received: by mail.gandi.net (Postfix) with ESMTPSA id 1B16624000C; Mon, 31 Jul 2023 09:09:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1690794547; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=0zygv9OONeF6w5wsQcuKYwJZsD4TU+ixmH4WXl55vos=; b=ks963aFFPRnkvTahy7pXrEJXzcJTm9danfhusAgNm88hBkk8FImIQ6Ky+H2RU889ghBe/j qcIkyDwAtdG1kP05bn4LA94760KnxS4uNEx3ALRUGU/kOo/0Zhc+Geq7A/V8oAOUHfM/Jk h+0Gar1qmgpugeCb3RReyZ9o98YIyJp9WP3J2eO2YXxCCuvALRvV0s1Tx7EMmKrVTf4bKT xziutgSpSZHnoDwhnU4yazFtj+OsAd+DbCVhmZOim+o36/mAHkbRH2D6PknGgqTuqpL4pN vTphC+o0mUqAxM96nv3sc1MobriJ6ITwmfxZevcnx9XmE3ZicucFbxoyv161eg== From: Miquel Raynal To: Richard Weinberger , Vignesh Raghavendra , Tudor Ambarus , Pratyush Yadav , Michael Walle , Cc: Miquel Raynal , Tomas Winkler , Alexander Usyskin , Zhang Xiaoxu Subject: [PATCH v2] mtd: Fix refcounting with MTD_PARTITIONED_MASTER Date: Mon, 31 Jul 2023 11:09:03 +0200 Message-Id: <20230731090903.770277-1-miquel.raynal@bootlin.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-GND-Sasl: miquel.raynal@bootlin.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230731_020911_881247_96FCCDF6 X-CRM114-Status: GOOD ( 15.49 ) X-Spam-Score: -0.9 (/) 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 logic is way too convoluted, let's clean the kref_get/put section to clarify what this block does, hopefully solving the refcounting issue when using CONFIG_MTD_PARTITIONED_MASTER at the same time [...] Content analysis details: (-0.9 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at https://www.dnswl.org/, low trust [2001:4b98:dc4:8:0:0:0:221 listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain 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 logic is way too convoluted, let's clean the kref_get/put section to clarify what this block does, hopefully solving the refcounting issue when using CONFIG_MTD_PARTITIONED_MASTER at the same time: - Iterate through all the parent mtd devices - Grab a reference over them all but the master - Only grab the master whith CONFIG_MTD_PARTITIONED_MASTER Same logic must apply in the put path, otherwise it would be broken. Cc: Tomas Winkler Cc: Alexander Usyskin Cc: Zhang Xiaoxu Fixes: 19bfa9ebebb5 ("mtd: use refcount to prevent corruption") Signed-off-by: Miquel Raynal Tested-by: Alexander Usyskin --- Hello, Alexander, Zhang, please test this version, it is close to what Zhang produced while not strictly identical. This compile-tested only, please check on you side whether it fixes the refcounting issue with and without PARTITIONED_MASTER, as well as with Kasan. Alexander, maybe there is something else to fix, in all cases the logic here was broken so let's start by this one and see if we need something else on top. Changes in v2: * Grab the parent before calling kref_put() to avoid accessing a freed mtd pointer. Thanks, Miquèl --- drivers/mtd/mtdcore.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index 2466ea466466..9e8ff3a999de 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -1241,14 +1241,15 @@ int __get_mtd_device(struct mtd_info *mtd) return -ENODEV; } - kref_get(&mtd->refcnt); - - while (mtd->parent) { - if (IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) || mtd->parent != master) - kref_get(&mtd->parent->refcnt); + while (mtd) { + if (mtd != master) + kref_get(&mtd->refcnt); mtd = mtd->parent; } + if (IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER)) + kref_get(&master->refcnt); + return 0; } EXPORT_SYMBOL_GPL(__get_mtd_device); @@ -1332,10 +1333,12 @@ void __put_mtd_device(struct mtd_info *mtd) { struct mtd_info *master = mtd_get_master(mtd); - while (mtd != master) { + while (mtd) { + /* kref_put() can relese mtd, so keep a reference mtd->parent */ struct mtd_info *parent = mtd->parent; - kref_put(&mtd->refcnt, mtd_device_release); + if (mtd != master) + kref_put(&mtd->refcnt, mtd_device_release); mtd = parent; }