mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH master 1/2] FIT: factor out fit_fdt_is_compatible for readability
@ 2025-02-14 11:23 Ahmad Fatoum
  2025-02-14 11:23 ` [PATCH master 2/2] FIT: do not decompress device trees to find compatible Ahmad Fatoum
  2025-02-17  9:00 ` [PATCH master 1/2] FIT: factor out fit_fdt_is_compatible for readability Sascha Hauer
  0 siblings, 2 replies; 3+ messages in thread
From: Ahmad Fatoum @ 2025-02-14 11:23 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

This introduces no functional change, but makes the code inside
fit_find_compatible_unit() more readable.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/image-fit.c | 67 +++++++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 31 deletions(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index 58ce461623e6..e05161379d04 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -719,6 +719,40 @@ static int fit_config_verify_signature(struct fit_handle *handle, struct device_
 	return ret;
 }
 
+static int fit_fdt_is_compatible(struct fit_handle *handle,
+				 struct device_node *child,
+				 const char *machine)
+{
+	struct device_node *image;
+	const char *unit = "fdt";
+	int data_len;
+	const void *data;
+	int ret;
+
+	if (of_property_present(child, "compatible"))
+		return 0;
+	if (!of_property_present(child, "fdt"))
+		return 0;
+
+	ret = fit_get_image(handle, child, &unit, &image);
+	if (ret)
+		goto err;
+
+	data = of_get_property(image, "data", &data_len);
+	if (!data)
+		goto err;
+
+	ret = fit_handle_decompression(image, "fdt", &data, &data_len);
+	if (ret)
+		goto err;
+
+	return fdt_machine_is_compatible(data, data_len, machine);
+err:
+	pr_warn("skipping malformed configuration \"%pOF\"\n",
+		child);
+	return 0;
+}
+
 static int fit_find_compatible_unit(struct fit_handle *handle,
 				    struct device_node *conf_node,
 				    const char **unit)
@@ -740,37 +774,8 @@ static int fit_find_compatible_unit(struct fit_handle *handle,
 	for_each_child_of_node(conf_node, child) {
 		int score = of_device_is_compatible(child, machine);
 
-		if (!score && !of_property_present(child, "compatible") &&
-		    of_property_present(child, "fdt")) {
-			struct device_node *image;
-			const char *unit = "fdt";
-			int data_len;
-			const void *data;
-			int ret;
-
-			ret = fit_get_image(handle, child, &unit, &image);
-			if (ret)
-				goto next;
-
-			data = of_get_property(image, "data", &data_len);
-			if (!data) {
-				ret = -EINVAL;
-				goto next;
-			}
-
-			ret = fit_handle_decompression(image, "fdt", &data, &data_len);
-			if (ret) {
-				ret = -EILSEQ;
-				goto next;
-			}
-
-			score = fdt_machine_is_compatible(data, data_len, machine);
-
-next:
-			if (ret)
-				pr_warn("skipping malformed configuration: %pOF (%pe)\n",
-					child, ERR_PTR(ret));
-		}
+		if (!score)
+			score = fit_fdt_is_compatible(handle, child, machine);
 
 		if (score > best_score) {
 			best_score = score;
-- 
2.39.5




^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH master 2/2] FIT: do not decompress device trees to find compatible
  2025-02-14 11:23 [PATCH master 1/2] FIT: factor out fit_fdt_is_compatible for readability Ahmad Fatoum
@ 2025-02-14 11:23 ` Ahmad Fatoum
  2025-02-17  9:00 ` [PATCH master 1/2] FIT: factor out fit_fdt_is_compatible for readability Sascha Hauer
  1 sibling, 0 replies; 3+ messages in thread
From: Ahmad Fatoum @ 2025-02-14 11:23 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Uncompressing all FDTs just to look up the compatible is detrimental to
boot speed and negatively impacts security. Let's thus throw an error
and expect users to specify a compatible property in the configuration
or just refrain from compressing their DTs.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/image-fit.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index e05161379d04..959384abd275 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -570,18 +570,29 @@ static void fit_uncompress_error_fn(char *x)
 	pr_err("%s\n", x);
 }
 
+static const char *get_compression_type(struct device_node *image)
+{
+	const char *compression = NULL;
+
+	of_property_read_string(image, "compression", &compression);
+	if (!compression || !strcmp(compression, "none"))
+		return NULL;
+
+	return compression;
+}
+
 static int fit_handle_decompression(struct device_node *image,
 				    const char *type,
 				    const void **data,
 				    int *data_len)
 {
-	const char *compression = NULL;
+	const char *compression;
 	struct property *pp;
 	void *uc_data;
 	int ret;
 
-	of_property_read_string(image, "compression", &compression);
-	if (!compression || !strcmp(compression, "none"))
+	compression = get_compression_type(image);
+	if (!compression)
 		return 0;
 
 	if (!strcmp(type, "ramdisk")) {
@@ -723,6 +734,7 @@ static int fit_fdt_is_compatible(struct fit_handle *handle,
 				 struct device_node *child,
 				 const char *machine)
 {
+	const char *reason = "malformed";
 	struct device_node *image;
 	const char *unit = "fdt";
 	int data_len;
@@ -742,14 +754,25 @@ static int fit_fdt_is_compatible(struct fit_handle *handle,
 	if (!data)
 		goto err;
 
-	ret = fit_handle_decompression(image, "fdt", &data, &data_len);
-	if (ret)
+	/* We have three options here:
+	 *
+	 * 1) Increase our attack surface by all supported compression algos
+	 * 2) Verify all configurations in the image as we search for best
+	 *    OF match score
+	 * 3) Blame the user and expect them to supply a compatible property
+	 *    in the configuration node if they want to compress their FDTs
+	 *
+	 * We go for option 3.
+	 */
+	if (get_compression_type(image)) {
+		reason = "compressed";
 		goto err;
+	}
 
 	return fdt_machine_is_compatible(data, data_len, machine);
 err:
-	pr_warn("skipping malformed configuration \"%pOF\"\n",
-		child);
+	pr_warn("skipping %s configuration \"%pOF\"\n",
+		reason, child);
 	return 0;
 }
 
-- 
2.39.5




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH master 1/2] FIT: factor out fit_fdt_is_compatible for readability
  2025-02-14 11:23 [PATCH master 1/2] FIT: factor out fit_fdt_is_compatible for readability Ahmad Fatoum
  2025-02-14 11:23 ` [PATCH master 2/2] FIT: do not decompress device trees to find compatible Ahmad Fatoum
@ 2025-02-17  9:00 ` Sascha Hauer
  1 sibling, 0 replies; 3+ messages in thread
From: Sascha Hauer @ 2025-02-17  9:00 UTC (permalink / raw)
  To: barebox, Ahmad Fatoum


On Fri, 14 Feb 2025 12:23:42 +0100, Ahmad Fatoum wrote:
> This introduces no functional change, but makes the code inside
> fit_find_compatible_unit() more readable.
> 
> 

Applied, thanks!

[1/2] FIT: factor out fit_fdt_is_compatible for readability
      https://git.pengutronix.de/cgit/barebox/commit/?id=272c1900d073 (link may not be stable)
[2/2] FIT: do not decompress device trees to find compatible
      https://git.pengutronix.de/cgit/barebox/commit/?id=5d7d60c4961c (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-02-17 10:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-14 11:23 [PATCH master 1/2] FIT: factor out fit_fdt_is_compatible for readability Ahmad Fatoum
2025-02-14 11:23 ` [PATCH master 2/2] FIT: do not decompress device trees to find compatible Ahmad Fatoum
2025-02-17  9:00 ` [PATCH master 1/2] FIT: factor out fit_fdt_is_compatible for readability Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox