linux-kernel - Re: [PATCH v2 10/13] kho: Update FDT dynamically for subtree addition/removal

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aRllULpK1wxZyY23@kernel.org>
Date: Sun, 16 Nov 2025 07:46:56 +0200
From: Mike Rapoport <rppt@...nel.org>
To: Pasha Tatashin <pasha.tatashin@...een.com>
Cc: akpm@...ux-foundation.org, bhe@...hat.com, jasonmiu@...gle.com,
	arnd@...db.de, coxu@...hat.com, dave@...ilevsky.ca,
	ebiggers@...gle.com, graf@...zon.com, kees@...nel.org,
	linux-kernel@...r.kernel.org, kexec@...ts.infradead.org,
	linux-mm@...ck.org
Subject: Re: [PATCH v2 10/13] kho: Update FDT dynamically for subtree
 addition/removal

On Sat, Nov 15, 2025 at 09:51:07AM -0500, Pasha Tatashin wrote:
> On Sat, Nov 15, 2025 at 4:40 AM Mike Rapoport <rppt@...nel.org> wrote:
> >
> > On Fri, Nov 14, 2025 at 01:59:59PM -0500, Pasha Tatashin wrote:
> > > -     struct kho_sub_fdt *sub_fdt;
> > > +     phys_addr_t phys = virt_to_phys(fdt);
> > > +     void *root_fdt = kho_out.fdt;
> > > +     int err = -ENOMEM;
> > > +     int off, fdt_err;
> > >
> > > -     sub_fdt = kmalloc(sizeof(*sub_fdt), GFP_KERNEL);
> > > -     if (!sub_fdt)
> > > -             return -ENOMEM;
> > > +     guard(mutex)(&kho_out.lock);
> > > +
> > > +     fdt_err = fdt_open_into(root_fdt, root_fdt, PAGE_SIZE);
> > > +     if (fdt_err < 0)
> > > +             return err;
> > >
> > > -     INIT_LIST_HEAD(&sub_fdt->l);
> > > -     sub_fdt->name = name;
> > > -     sub_fdt->fdt = fdt;
> > > +     off = fdt_add_subnode(root_fdt, 0, name);
> >
> > Why not
> >         fdt_err = fdt_add_subnode()
> >
> > as I asked in v1 review?
> >
> 
> Oh, I missed that, there is a slight difference between the two:
> 'fdt_err' only contains FDT return value, i.e. error if negative. The
> 'off' on the other hand in the happy path contains subnode offset, and
> contains error only in the unhappy path. This is why I think it is a
> little cleaner to keep different name, however, if you still prefer
> re-using a single local variable for both, this is fix-up patch:
> 
> diff --git a/kernel/liveupdate/kexec_handover.c
> b/kernel/liveupdate/kexec_handover.c
> index 224bdf5becb6..81f60ccb2dc7 100644
> --- a/kernel/liveupdate/kexec_handover.c
> +++ b/kernel/liveupdate/kexec_handover.c
> @@ -713,7 +713,7 @@ int kho_add_subtree(const char *name, void *fdt)
>         phys_addr_t phys = virt_to_phys(fdt);
>         void *root_fdt = kho_out.fdt;
>         int err = -ENOMEM;
> -       int off, fdt_err;
> +       int fdt_err;
> 
>         guard(mutex)(&kho_out.lock);
> 
> @@ -721,14 +721,14 @@ int kho_add_subtree(const char *name, void *fdt)
>         if (fdt_err < 0)
>                 return err;
> 
> -       off = fdt_add_subnode(root_fdt, 0, name);
> -       if (off < 0) {
> -               if (off == -FDT_ERR_EXISTS)
> +       fdt_err = fdt_add_subnode(root_fdt, 0, name);
> +       if (fdt_err < 0) {
> +               if (fdt_err == -FDT_ERR_EXISTS)
>                         err = -EEXIST;
>                 goto out_pack;
>         }
> 
> -       err = fdt_setprop(root_fdt, off, PROP_SUB_FDT, &phys, sizeof(phys));
> +       err = fdt_setprop(root_fdt, fdt_err, PROP_SUB_FDT, &phys, sizeof(phys));

I missed 'off' here, never mind

>         if (err < 0)
>                 goto out_pack;
> 

-- 
Sincerely yours,
Mike.