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: <20251113160020.227664-1-yuntao.wang@linux.dev>
Date: Fri, 14 Nov 2025 00:00:20 +0800
From: Yuntao Wang <yuntao.wang@...ux.dev>
To: robh@...nel.org
Cc: akpm@...ux-foundation.org,
	bhe@...hat.com,
	catalin.marinas@....com,
	changyuanl@...gle.com,
	chenzhou10@...wei.com,
	devicetree@...r.kernel.org,
	geert+renesas@...der.be,
	graf@...zon.com,
	james.morse@....com,
	linux-kernel@...r.kernel.org,
	rppt@...nel.org,
	saravanak@...gle.com,
	takahiro.akashi@...aro.org,
	thunder.leizhen@...wei.com,
	yuntao.wang@...ux.dev
Subject: Re: [PATCH 00/10] of/fdt: Some bug fixes and cleanups

On Wed, 12 Nov 2025 14:55:51 -0600, Rob Herring <robh@...nel.org> wrote:

> 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

Hi Rob,

The link to the new patch series:

https://lore.kernel.org/linux-devicetree/20251113155104.226617-1-yuntao.wang@linux.dev/t/

Thanks,
Yuntao