linux-kernel - Re: [RFC] module: Strict per-modname namespaces

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: <20241107115819.GA38786@noisy.programming.kicks-ass.net>
Date: Thu, 7 Nov 2024 12:58:19 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Masahiro Yamada <masahiroy@...nel.org>
Cc: mcgrof@...nel.org, x86@...nel.org, hpa@...or.com, petr.pavlu@...e.com,
	samitolvanen@...gle.com, da.gomez@...sung.com, nathan@...nel.org,
	nicolas@...sle.eu, linux-kernel@...r.kernel.org,
	linux-modules@...r.kernel.org, linux-kbuild@...r.kernel.org,
	hch@...radead.org, gregkh@...uxfoundation.org
Subject: Re: [RFC] module: Strict per-modname namespaces

On Thu, Nov 07, 2024 at 04:55:46PM +0900, Masahiro Yamada wrote:
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index 107393a8c48a..d1de3044ee03 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -1553,8 +1553,19 @@ static void mod_set_crcs(struct module *mod)
> >         free(buf);
> >  }
> >
> > +static const char *mod_basename(const char *modname)
> > +{
> > +       const char *basename = strrchr(modname, '/');
> > +       if (basename)
> > +               basename++;
> > +       else
> > +               basename = modname;
> > +       return basename;
> > +}
> > +
> >  static void read_symbols(const char *modname)
> >  {
> > +       char module_namespace[MODULE_NAME_LEN + 8];
> >         const char *symname;
> >         char *version;
> >         char *license;
> > @@ -1586,12 +1597,16 @@ static void read_symbols(const char *modname)
> >                         license = get_next_modinfo(&info, "license", license);
> >                 }
> >
> > -               namespace = get_modinfo(&info, "import_ns");
> > -               while (namespace) {
> > +               for (namespace = get_modinfo(&info, "import_ns"); namespace;
> > +                    namespace = get_next_modinfo(&info, "import_ns", namespace)) {
> 
> The conversion from while() to for() is an unrelated change.
> Split it to a separate patch if you want to change it.

Yeah, at some point I had a break or continue in there I think, it
didn't live. If you like I can keep it a split it out.

> > +                       if (strstarts(namespace, "MODULE_"))
> > +                               error("importing implicit module namespace: %s\n", namespace);
> > +
> >                         add_namespace(&mod->imported_namespaces, namespace);
> > -                       namespace = get_next_modinfo(&info, "import_ns",
> > -                                                    namespace);
> >                 }
> > +               snprintf(module_namespace, sizeof(module_namespace), "MODULE_%s",
> > +                        mod_basename(mod->name));

.. here ..

> > +               add_namespace(&mod->imported_namespaces, module_namespace);
> >
> >                 if (extra_warn && !get_modinfo(&info, "description"))
> >                         warn("missing MODULE_DESCRIPTION() in %s\n", modname);
> > @@ -1700,11 +1715,7 @@ static void check_exports(struct module *mod)
> >                 s->crc_valid = exp->crc_valid;
> >                 s->crc = exp->crc;
> >
> > -               basename = strrchr(mod->name, '/');
> > -               if (basename)
> > -                       basename++;
> > -               else
> > -                       basename = mod->name;
> > +               basename = mod_basename(mod->name);
> 
> This is an unrelated change.

Hardly unrelated, I added a second usage above. So you're saying I
should've just copy pasted the whole thing and then it would've been
fine?

> So, it should be split into a separate prerequisite patch,
> something like, "modpost: introduce mod_basename() helper"

Well, I can...

Anyway, let me see about doing that whole ',' parsing thing people seem
to want.