linux-kernel - Re: [RFC v1 1/3] luo: Live Update Orchestrator

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: <CA+CK2bBQwC16W2Qmw5vXdMi7EaadAGXfUs6ym1P1UZ90PzbZvQ@mail.gmail.com>
Date: Thu, 20 Mar 2025 14:30:25 -0400
From: Pasha Tatashin <pasha.tatashin@...een.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: changyuanl@...gle.com, graf@...zon.com, rppt@...nel.org, 
	rientjes@...gle.com, corbet@....net, rdunlap@...radead.org, 
	ilpo.jarvinen@...ux.intel.com, kanie@...ux.alibaba.com, ojeda@...nel.org, 
	aliceryhl@...gle.com, masahiroy@...nel.org, akpm@...ux-foundation.org, 
	tj@...nel.org, yoann.congal@...le.fr, mmaurer@...gle.com, 
	roman.gushchin@...ux.dev, chenridong@...wei.com, axboe@...nel.dk, 
	mark.rutland@....com, jannh@...gle.com, vincent.guittot@...aro.org, 
	hannes@...xchg.org, dan.j.williams@...el.com, david@...hat.com, 
	joel.granados@...nel.org, rostedt@...dmis.org, anna.schumaker@...cle.com, 
	song@...nel.org, zhangguopeng@...inos.cn, linux@...ssschuh.net, 
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org, linux-mm@...ck.org, 
	gregkh@...uxfoundation.org, tglx@...utronix.de, mingo@...hat.com, 
	bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, 
	rafael@...nel.org, dakr@...nel.org, bartosz.golaszewski@...aro.org, 
	cw00.choi@...sung.com, myungjoo.ham@...sung.com, yesanishhere@...il.com, 
	Jonathan.Cameron@...wei.com, quic_zijuhu@...cinc.com, 
	aleksander.lobakin@...el.com, ira.weiny@...el.com, leon@...nel.org, 
	lukas@...ner.de, bhelgaas@...gle.com, wagi@...nel.org, djeffery@...hat.com, 
	stuart.w.hayes@...il.com, jgowans@...zon.com, jgg@...dia.com
Subject: Re: [RFC v1 1/3] luo: Live Update Orchestrator

On Thu, Mar 20, 2025 at 1:50 PM Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
>
> On Thu, Mar 20, 2025 at 12:35:20PM -0400, Pasha Tatashin wrote:
> > On Thu, Mar 20, 2025 at 9:40 AM Andy Shevchenko
> > <andriy.shevchenko@...ux.intel.com> wrote:
> > > On Thu, Mar 20, 2025 at 02:40:09AM +0000, Pasha Tatashin wrote:
>
> ...
>
> > > > +#ifndef _LINUX_LIVEUPDATE_H
> > > > +#define _LINUX_LIVEUPDATE_H
> > >
> > > > +#include <linux/compiler.h>
> > > > +#include <linux/notifier.h>
> > >
> > > This is semi-random list of inclusions. Try to follow IWYU principle.
> > > See below.
> >
> > I will remove <linux/compiler.h>
>
> But you need to add something more...

...

>
> ...
>
> > > > +bool liveupdate_state_updated(void);
> > >
> > > Where bool is defined?
> >
> > in kernel/liveupdate.c
>
> Nope, I meant where the type is defined. It is IIRC in types.h which needs to
> be included.

Ah, I see what you mean, sure I will include types.h.

>
> ...
>
> > > > +     if (kstrtol(buf, 0, &val) < 0)
> > > > +             return -EINVAL;
> > >
> > > Shadower error code.
> >
> > In this case it is appropriate, we do not case why kstrtol() could not
> > be converted into an appropriate integer, all we care is that the
> > input was invalid, and that what we return back to user.
>
> The kstrtox() may give different error codes. User may want to know more about
> what's wrong with the input. Shadowed error codes are discouraged and should be
> explained.
>

...

> > > > +     if (val != 1 && val != 0)
> > > > +             return -EINVAL;
> > >
> > > What's wrong with using kstrtobool() from the beginning?
> >
> > It makes the input less defined, here we only allow '1' or '0',
> > kstrtobool() allows almost anything.
>
> But kstrtobool() is the interface for boolean input. You may document only
> 0 and 1 and don't tell people to use anything else. ABI documentation should
> be clear, that's it.

Sure, I will use kstrtobool().

>
> ...
>
> > > > +EXPORT_SYMBOL_GPL(liveupdate_state_normal);
> > >
> > > No namespace?
> >
> > Namespace is 'liveupdate_', all public interfaces have this prefix,
> > private functions are prefixed with luo_ where it makes sense.
>
> No, I'm talking about export namespace. Why does the entire kernel need these APIs?

These functions are intended for use by drivers and other subsystems
participating in the live update. They allow these components to
determine, during boot, whether to restore their state from the
serialized state, or, during runtime, whether a live update is in the
prepared state, causing different behavior compared to normal mode
(e.g., prohibiting DMA mappings modifications, binding/unbinding,
etc.).

Pasha

> --
> With Best Regards,
> Andy Shevchenko