linux-kernel - Re: [PATCH 00/10] of/fdt: Some bug fixes and cleanups

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: <20251112205551.GC2155854-robh@kernel.org>
Date: Wed, 12 Nov 2025 14:55:51 -0600
From: Rob Herring <robh@...nel.org>
To: Yuntao Wang <yuntao.wang@...ux.dev>
Cc: Saravana Kannan <saravanak@...gle.com>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	Catalin Marinas <catalin.marinas@....com>,
	AKASHI Takahiro <takahiro.akashi@...aro.org>,
	James Morse <james.morse@....com>,
	Chen Zhou <chenzhou10@...wei.com>, Baoquan He <bhe@...hat.com>,
	Zhen Lei <thunder.leizhen@...wei.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Changyuan Lyu <changyuanl@...gle.com>,
	Alexander Graf <graf@...zon.com>,
	"Mike Rapoport (Microsoft)" <rppt@...nel.org>,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 00/10] of/fdt: Some bug fixes and cleanups

On Wed, Nov 12, 2025 at 10:35:10PM +0800, Yuntao Wang wrote:
> This patch series fixes several bugs related to dt_root_addr_cells and
> dt_root_size_cells, and performs some cleanup.
> 
> Links to the previous related patches:
> 
> https://lore.kernel.org/lkml/CAL_JsqJxar7z+VcBXwPTw5-Et2oC9bQmH_CtMtKhoo_-=zN2XQ@mail.gmail.com/
> 
> Yuntao Wang (10):
>   of/fdt: Introduce dt_root_addr_size_cells() and
>     dt_root_addr_size_bytes()
>   of/reserved_mem: Use dt_root_addr_size_bytes() instead of open-coding
>     it
>   of/reserved_mem: Use dt_root_addr_size_bytes() instead of open-coding
>     it
>   of/reserved_mem: Use dt_root_addr_size_bytes() instead of open-coding
>     it

Your aim in writing subjects should be to write something that is unique 
for every commit in the past or future. Because you can never make the 
same change twice, right? (I'm excluding 'fix typos/spelling' type 
commits). Certainly the same subject in one series is never right.

>   of/fdt: Use dt_root_addr_size_bytes() instead of open-coding it
>   of/fdt: Fix the len check in early_init_dt_check_for_elfcorehdr()
>   of/fdt: Fix the len check in
>     early_init_dt_check_for_usable_mem_range()
>   of/fdt: Use dt_root_addr_size_bytes() instead of open-coding it

This is not what I meant. We have multiple copies of this where only 
the property name changes: 

	prop = of_get_flat_dt_prop(node, "linux,elfcorehdr", &len);
	if (!prop || (len < (dt_root_addr_cells + dt_root_size_cells)))
		return;

	elfcorehdr_addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
	elfcorehdr_size = dt_mem_next_cell(dt_root_size_cells, &prop);

Instead, add a function something like this:

static void early_init_dt_read_address(unsigned long node, const char 
*prop, u64 *addr, u64*size)
{
        prop = of_get_flat_dt_prop(node, prop, &len);
        if (!prop || (len < (dt_root_addr_cells + dt_root_size_cells)))
                return;

        *addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
        *size = dt_mem_next_cell(dt_root_size_cells, &prop);
}

Then we only have the length checks in one place.


That still leaves the cases with more than 1 entry open coded. So 
instead, to cover that case to something like this:

const __be32 *of_get_flat_dt_address_prop(unsigned long node, const char 
*propname, int *len)
{
	prop = of_get_flat_dt_prop(node, propname, &len);
	if (!prop || (*len % (dt_root_addr_cells + dt_root_size_cells))) {
		*len = 0;
		return NULL;
	}

	*len /= (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
	return prop;
}

And then a user would look something like this:

prop = of_get_flat_dt_address(node, "linux,usable-memory-range", &len);
for (i = 0; i < len; i++) {
	of_read_address_idx(prop, i, &addr, &size);
	...
}

Here 'len' is number of addr+size entries.

And the simple case of reading 1 entry could be just:

of_read_address_idx(of_get_flat_dt_address(node, "linux,elfcorehdr", NULL), 0, &addr, &size);

Rob