linux-kernel - Re: [PATCH] of: address: Fix bug to get the highest cpu address of subtrees for dma

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: <20250804164329.98971-1-kjw1627@gmail.com>
Date: Tue,  5 Aug 2025 00:43:14 +0800
From: Joonwon Kang <kjw1627@...il.com>
To: robh@...nel.org
Cc: devicetree@...r.kernel.org,
	kjw1627@...il.com,
	linux-kernel@...r.kernel.org,
	nsaenzjulienne@...e.de,
	saravanak@...gle.com
Subject: Re: [PATCH] of: address: Fix bug to get the highest cpu address of subtrees for dma

On Sun, Jul 27, 2025 at 1:01 PM Joonwon Kang <kjw1627@...il.com> wrote:
> >
> > The function of_dma_get_max_cpu_address() for a device node should choose
> > the highest cpu address among the ones that nodes can access.
> > However, there was a bug of choosing the lowest cpu address and this
> > commit is to fix it.
>
> Please provide a test case in the DT unittests or at least details on
> the DT that is affected by the bug.

While working on the DT unittests, I got two questions to which I had failed to
have clear answers. Let's assume that the device tree looks as follows.

parent_bus@... {
	#address-cells = <1>;
	#size-cells = <1>;
	dma-ranges = <0x0 0x0 0x1000>;

	child_bus@... {
		#address-cells = <1>;
		#size-cells = <1>;
		/* Note that the size part exceeds the `parent_bus`' dma size. */
		dma-ranges = <0x0 0x0 0x2000>;

		child_device_1@... {
			/*
			 * Note that the size part exceeds the `child_bus`' dma size and
			 * also the `parent_bus`' dma size.
			 */
			reg = <0x0 0x3000>;
		};

		child_device_2@... {
			/*
			 * Note that the address part transitively exceeds the
			 *`parent_bus`' end address.
			 */
			reg = <0x1000 0x1000>
		};
	};

	another_child_bus@... {
		#address-cells = <1>;
		#size-cells = <1>;
		dma-ranges = <0x0 0x0 0x300>;
	};
};

Q1: What is the expected output of `of_dma_get_max_cpu_address(parent_bus)`?
I think it should be 0xfff since the `dma-ranges` in the `child_bus` should be
capped to the parent max cpu address instead of treating it as if the
`dma-ranges` in the `child_bus` does not exist. The current expectation is
0x2ff which is for `another_child_bus` based on the existing test case
in drivers/of/unittest.c and drivers/of/tests-address.dtsi.

Q2: `of_dma_get_max_cpu_address(child_device_1, reg_prop, &addr, &length)`
returns a success with `addr` set to 0x0 and `length` set to 0x3000. Similarly,
`of_translate_dma_address(child_device_1, reg_prop)` returns a success. On the
other hand, both functions for `child_device_2` return a failure since the
address is out of parent ranges. I think those functions should also fail
for `child_device_1` since the dma "end" address of the `child_device_1` node
is not valid in the first place. Are the current behaviors of both functions
intended?

> > Signed-off-by: Joonwon Kang <kjw1627@...il.com>
> > ---
> >  drivers/of/address.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index f0f8f0dd191c..5e984e0d372b 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -969,6 +969,7 @@ phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
> >  {
> >         phys_addr_t max_cpu_addr = PHYS_ADDR_MAX;
> >         struct of_range_parser parser;
> > +       phys_addr_t max_subtree_max_addr = PHYS_ADDR_MAX;
> >         phys_addr_t subtree_max_addr;
> >         struct device_node *child;
> >         struct of_range range;
> > @@ -992,10 +993,17 @@ phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
> >
> >         for_each_available_child_of_node(np, child) {
> >                 subtree_max_addr = of_dma_get_max_cpu_address(child);
> > -               if (max_cpu_addr > subtree_max_addr)
> > -                       max_cpu_addr = subtree_max_addr;
> > +               if (subtree_max_addr == PHYS_ADDR_MAX)
> > +                       continue;
> > +
> > +               if (max_subtree_max_addr == PHYS_ADDR_MAX)
> > +                       max_subtree_max_addr = subtree_max_addr;
> > +               else
> > +                       max_subtree_max_addr = max(max_subtree_max_addr, subtree_max_addr);
> >         }
> >
> > +       max_cpu_addr = min(max_cpu_addr, max_subtree_max_addr);
> > +
> >         return max_cpu_addr;
> >  }
> >
> > --
> > 2.46.0
> >