linux-kernel - Re: [PATCH v4 4/5] 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: <ZvqeSC3ZsRU0uEaB@smile.fi.intel.com>
Date: Mon, 30 Sep 2024 15:49:12 +0300
From: Andy Shevchenko <andy@...nel.org>
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>, Kees Cook <kees@...nel.org>,
	Daniel Golle <daniel@...rotopia.org>,
	INAGAKI Hiroshi <musashino.open@...il.com>,
	Christian Brauner <brauner@...nel.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	Li Lingfeng <lilingfeng3@...wei.com>,
	Ming Lei <ming.lei@...hat.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,
	linux-hardening@...r.kernel.org,
	Miquel Raynal <miquel.raynal@...tlin.com>,
	Lorenzo Bianconi <lorenzo@...nel.org>, upstream@...oha.com
Subject: Re: [PATCH v4 4/5] block: add support for partition table defined in
 OF

On Mon, Sep 30, 2024 at 01:30:11PM +0200, Christian Marangi wrote:
> Add support for partition table defined in Device Tree. Similar to how
> it's done with MTD, add support for defining a fixed partition table in
> device tree.
> 
> A common scenario for this is fixed block (eMMC) embedded devices that
> have no MBR or GPT partition table to save storage space. Bootloader
> access the block device with absolute address of data.
> 
> This is to complete the functionality with an equivalent implementation
> with providing partition table with bootargs, for case where the booargs
> can't be modified and tweaking the Device Tree is the only solution to
> have an usabe partition table.
> 
> The implementation follow the fixed-partitions parser used on MTD
> devices where a "partitions" node is expected to be declared with
> "fixed-partitions" compatible in the OF node of the disk device
> (mmc-card for eMMC for example) and each child node declare a label
> and a reg with offset and size. If label is not declared, the node name
> is used as fallback. Eventually is also possible to declare the read-only
> property to flag the partition as read-only.
> 
> For eMMC block, driver scan the disk name and check if it's suffixed with
> "boot0" or "boot1".
> This is to handle the additional disk provided by eMMC as supported in
> JEDEC 4.4+. If this suffix is detected, "partitions-boot0" or
> "partitions-boot1" are used instead of the generic "partitions" for the
> relevant disk.

...

> +	strscpy(info->volname, partname, sizeof(info->volname));

We have 2-arguments strscpy(), please use that.

> +	strlcat(state->pp_buf, tmp, PAGE_SIZE);

In new code we should not use strl*(). They are subject to remove.
And actually why? You have used strscpy() a few lines above...

...

> +	for_each_child_of_node(partitions_np, np) {

Use _scoped() variant.

> +		if (validate_of_partition(np, slot)) {
> +			of_node_put(np);
> +			ret = -1;
> +			goto exit;
> +		}
> +
> +		slot++;
> +	}

...

> +	for_each_child_of_node(partitions_np, np) {

Ditto.

> +		if (slot >= state->limit) {
> +			of_node_put(np);
> +			break;
> +		}
> +
> +		add_of_partition(state, slot, np);
> +
> +		slot++;
> +	}

...

> +	strlcat(state->pp_buf, "\n", PAGE_SIZE);

Why strl*()?

-- 
With Best Regards,
Andy Shevchenko