linux-kernel - Re: [00/11] pr_debug during module initialization

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: <87d388qiij.fsf@rustcorp.com.au>
Date:	Tue, 20 Mar 2012 09:57:00 +1030
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	Jim Cromie <jim.cromie@...il.com>
Cc:	jbaron@...hat.com, linux-kernel@...r.kernel.org,
	Thomas Renninger <trenn@...e.de>, pawel.moll@....com
Subject: Re: [00/11] pr_debug during module initialization

On Mon, 19 Mar 2012 00:17:29 -0600, Jim Cromie <jim.cromie@...il.com> wrote:
> On Sun, Mar 18, 2012 at 8:04 PM, Rusty Russell <rusty@...tcorp.com.au> wrote:
> > The module parts seem fine.  The re-parsing of the commandline seems
> > weird:  I'd really rather see something in unknown_bootoption(), like:
> >
> >        /* Unused module parameter. */
> >        if (strchr(param, '.') && (!val || strchr(param, '.') < val)) {
> > +               /* Check for <module>.dyndebug fake param */
> > +               dyndebug_parse(param, val);
> >                return 0;
> >        }
> 
> That is too early - the dyndbg rules cant be activated
> until dynamic_debug_init() runs, which is currently an
> arch_initcall(dynamic_debug_init);
> 
> Patch 11 makes it core-initcall, (which Im not sure is ok
> for all cases, but works for me), I also tried it with
> early_initcall (and that worked too)
> but I think thats still after the Booting kernel parse.

Indeed.  OK, that's fine then.

> I *could* capture the dyndbg options during Booting kernel
> parse, and activate them once the tables are loaded,
> but this seems convoluted.

Agreed.

> a "private" parse-args. After your earlier suggestion to use a
> callback, it occurred to me that just reusing parse_args would
> work, be fairly minimal (only 2 lines in dynamic_debug_init())
> and its already called a bunch of times; Booting kernel,
> module-load, and do-initcall-level (in Pawel's patch).
> Pawels patch doesnt do anything to avoid reparsing
> boot-time params.
> 
> Does this change your weirdness assessment ?

It's still weird to re-parse, but with a comment explaining that we
need to do it again after dynamic_debug_init().  Unless we can call
that explicitly before parse_args().
> Lastly, my earlier rev handled  foo.dyndbg params
> for loadable modules.  I took that out cuz
> Documentation/kernel-parameters.txt says thats
> for builtins (only, as I read it), and treating foo.dyndbg
> differently should be done w/o discussion.

Yes, when a module 'foo' declares it has a parameter 'bar', it becomes
'foo.bar' if the module is built-in, and just 'bar' if the module is
loaded.

Furthermore, modprobe reads /proc/cmdline when loading foo, looking for
"foo.*" to add to the modprobe arguments.

> > Otherwise, all looks good!
> >
> > Acked-by: Rusty Russell <rusty@...tcorp.com.au>
> 
> thanks.  I guess the Ack (vs SOB) means that Jason should
> forward it on to Greg as a single set ?
> (subject to his Ack of course)

I assumed it would go via Jason.  Otherwise, get his ack and resend to
me.

Thanks,
Rusty.
-- 
  How could I marry someone with more hair than me?  http://baldalex.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/