From mboxrd@z Thu Jan 1 00:00:00 1970 Delivery-date: Wed, 07 Jun 2023 11:01:04 +0200 Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by lore.white.stw.pengutronix.de with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1q6p2P-00E73a-Bw for lore@lore.pengutronix.de; Wed, 07 Jun 2023 11:01:04 +0200 Received: from localhost ([127.0.0.1] helo=metis.ext.pengutronix.de) by metis.ext.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1q6p2M-0000dH-19; Wed, 07 Jun 2023 11:01:02 +0200 Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=[127.0.0.1]) by metis.ext.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1q6p2J-0000cq-WA; Wed, 07 Jun 2023 11:01:00 +0200 Message-ID: Date: Wed, 7 Jun 2023 11:00:59 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.1 Content-Language: en-US To: =?UTF-8?Q?Uwe_Kleine-K=c3=b6nig?= References: <20230607080818.715192-1-a.fatoum@pengutronix.de> <20230607085508.xl6nsx2ajidfnghp@pengutronix.de> From: Ahmad Fatoum In-Reply-To: <20230607085508.xl6nsx2ajidfnghp@pengutronix.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: Re: [OSS-Tools] [PATCH] libdt: fix of_get_devicepath looking up sibling if device unavailable X-BeenThere: oss-tools@pengutronix.de X-Mailman-Version: 2.1.29 Precedence: list List-Id: Pengutronix Public Open-Source-Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: oss-tools@pengutronix.de Sender: "OSS-Tools" X-SA-Exim-Connect-IP: 127.0.0.1 X-SA-Exim-Mail-From: oss-tools-bounces@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false Hello Uwe, On 07.06.23 10:55, Uwe Kleine-König wrote: > Hello Ahmad, > > On Wed, Jun 07, 2023 at 10:08:18AM +0200, Ahmad Fatoum wrote: >> of_get_devicepath code flow is split into two: >> >> A) Either the device tree node in question has a direct udev_device >> associated with it >> >> B) Or we assume it's a partition and lookup udev_device for the parent >> first, before finding a child udev_device or setting a partition >> offset within the parent udev_device. >> >> Since v2017.03.0, we have had a fallthrough from case A into case B: >> If we have a udev_device, but it's neither a EEPROMs, MTDs or block >> device, we just consider it a partition. This is problematic, because >> this may result in us pointing at a very different device: >> >> - backend points at a SD-Card host. Host is enabled, but SD-Card >> is not inserted, so no block device >> >> - case A fails, so it's assumed it's a partition and case B >> uses parent SoC bus to lookup appropriate device >> >> - We fall through into the second device_find_block_device, which >> will take the first matching block device across the SoC. So >> we could end up with the eMMC: a completely different device >> than what was pointed at. > > So another surprise is that device_find_block_device() recurses to find > a device when starting on /soc, isn't it? Is this worth addressing? I don't know what I'd break if I limit iteration depth and I don't know what else I could do to curtail this.. > >> Fixes: 929ed64cb42f ("of_get_devicepath: make partition finding more robust") >> Signed-off-by: Ahmad Fatoum >> --- >> src/libdt.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/src/libdt.c b/src/libdt.c >> index e54d7fb5649d..7b99efe5b2de 100644 >> --- a/src/libdt.c >> +++ b/src/libdt.c >> @@ -2492,9 +2492,11 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t >> } >> >> /* >> - * If we found a device but couldn't classify it above, we fall >> - * through. >> + * If we find a udev_device but couldn't classify it above, >> + * it's an error. Falling through would mean to handle it as a >> + * partition and could lead us to return an arbitrary sibling device >> */ >> + return -ENODEV; > > I don't remember the details of 929ed64cb42f any more, but probably I > didn't have a specific case that were fixed by that commit. Your > rationale makes sense. > > Reviewed-by: Uwe Kleine-König Thanks, Ahmad > > Best regards > Uwe > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |