linux-kernel - Re: [PATCH v3 3/4] block: add support for partition table defined in OF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <877catlcni.fsf@prevas.dk>
Date: Mon, 30 Sep 2024 11:21:53 +0200
From: Rasmus Villemoes <linux@...musvillemoes.dk>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: Jens Axboe <axboe@...nel.dk>,  Jonathan Corbet <corbet@....net>,  Ulf
 Hansson <ulf.hansson@...aro.org>,  Rob Herring <robh@...nel.org>,
  Krzysztof Kozlowski <krzk+dt@...nel.org>,  Conor Dooley
 <conor+dt@...nel.org>,  INAGAKI Hiroshi <musashino.open@...il.com>,
  Daniel Golle <daniel@...rotopia.org>,  Christian Brauner
 <brauner@...nel.org>,  Al Viro <viro@...iv.linux.org.uk>,  Jan Kara
 <jack@...e.cz>,  Li Lingfeng <lilingfeng3@...wei.com>,  Christian Heusel
 <christian@...sel.eu>,  linux-block@...r.kernel.org,
  linux-doc@...r.kernel.org,  linux-kernel@...r.kernel.org,
  linux-mmc@...r.kernel.org,  devicetree@...r.kernel.org,  Miquel Raynal
 <miquel.raynal@...tlin.com>,  Lorenzo Bianconi <lorenzo@...nel.org>,
  upstream@...oha.com
Subject: Re: [PATCH v3 3/4] block: add support for partition table defined
 in OF

Christian Marangi <ansuelsmth@...il.com> writes:

> diff --git a/block/partitions/of.c b/block/partitions/of.c
> new file mode 100644
> index 000000000000..bc6200eb86b3
> --- /dev/null
> +++ b/block/partitions/of.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/blkdev.h>
> +#include <linux/major.h>
> +#include <linux/of.h>
> +#include "check.h"
> +
> +#define BOOT0_STR	"boot0"
> +#define BOOT1_STR	"boot1"
> +
> +static struct device_node *get_partitions_node(struct device_node *disk_np,
> +					       struct gendisk *disk)
> +{
> +	const char *node_name = "partitions";
> +
> +	/*
> +	 * JEDEC specification 4.4 for eMMC introduced 3 additional partition
> +	 * present on every eMMC. These additional partition are always hardcoded
> +	 * from the eMMC driver as boot0, boot1 and rpmb. While rpmb is used to
> +	 * store keys and exposed as a char device, the other 2 are exposed as
> +	 * real separate disk with the boot0/1 appended to the disk name.
> +	 *
> +	 * Here we parse the disk_name in search for such suffix and select
> +	 * the correct partition node.
> +	 */
> +	if (disk->major == MMC_BLOCK_MAJOR) {
> +		const char *disk_name = disk->disk_name;
> +
> +		if (!memcmp(disk_name + strlen(disk_name) - strlen(BOOT0_STR),
> +			    BOOT0_STR, sizeof(BOOT0_STR)))
> +			node_name = "partitions-boot0";

If strlen(disk_name) is less than 5 (and I don't know if that's actually
possible), this well end up doing out-of-bounds access.

We have a strstarts() helper, could you also add a strends() helper that
handles this correctly? Something like

/**
 * strends - does @str end with @suffix?
 * @str: string to examine
 * @suffix: suffix to look for.
 */
static inline bool strends(const char *str, const char *suffix)
{
	size_t n = strlen(str);
        size_t m = strlen(suffix);
        return n >= m && !memcmp(str + n - m, suffix, m);
}

[or name it str_has_suffix() or str_ends_with(), "strends" is not
particularly readable, it's unfortunate that the existing strstarts is
spelled like that].

Rasmus