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] [day] [month] [year] [list]
Message-ID: <CAL_JsqJjbtrn8KjLa=wSZf+g-j9GtmSt-LVuW42A+2eBvRwJtw@mail.gmail.com>
Date: Mon, 4 Aug 2025 17:44:16 -0500
From: Rob Herring <robh@...nel.org>
To: Joonwon Kang <kjw1627@...il.com>
Cc: devicetree@...r.kernel.org, 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 Mon, Aug 4, 2025 at 11:43 AM Joonwon Kang <kjw1627@...il.com> wrote:
>
> 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>;

dma-ranges is irrelevant for 'reg'. 'ranges' applies to 'reg'.

>                 };
>
>                 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.

0x2FF is correct. The max address returned is the minimum.

>
> 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?

Passing in child_device_1 is invalid. It doesn't contain dma-ranges,
so it will return PHYS_ADDR_MAX.

Passing 'reg' into the DMA translation functions is invalid. 'reg' has
0 to do with DMA addresses.

Rob

>
> > > 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
> > >