linux-kernel - Re: [PATCH v1] module: Fix prefix for module.sig

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: <01bf93d3-06c4-594e-e3c6-b6f56a1efa83@infradead.org>
Date:   Thu, 2 Jun 2022 21:30:53 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     Saravana Kannan <saravanak@...gle.com>,
        Luis Chamberlain <mcgrof@...nel.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Christophe Leroy <christophe.leroy@...roup.eu>,
        Aaron Tomlin <atomlin@...hat.com>,
        Christoph Lameter <cl@...ux.com>,
        Miroslav Benes <mbenes@...e.cz>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jessica Yu <jeyu@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-modules@...r.kernel.org, void@...ifault.com,
        atomlin@...mlin.com, Allen Pais <allen.lkml@...il.com>,
        Joe Perches <joe@...ches.com>,
        Michal Suchanek <msuchanek@...e.de>,
        Oleksandr Natalenko <oleksandr@...alenko.name>,
        Jason Wessel <jason.wessel@...driver.com>,
        Petr Mladek <pmladek@...e.com>,
        Daniel Thompson <daniel.thompson@...aro.org>,
        Christoph Hellwig <hch@...radead.org>,
        Android Kernel Team <kernel-team@...roid.com>
Subject: Re: [PATCH v1] module: Fix prefix for module.sig_enforce module param

Hi--

On 6/2/22 20:48, Saravana Kannan wrote:
> On Thu, Jun 2, 2022 at 3:59 PM Luis Chamberlain <mcgrof@...nel.org> wrote:
>>
>> On Thu, Jun 02, 2022 at 02:47:04PM -0700, Saravana Kannan wrote:
>>> On Thu, Jun 2, 2022 at 12:41 PM Linus Torvalds
>>> <torvalds@...ux-foundation.org> wrote:
>>>>
>>>> On Thu, Jun 2, 2022 at 12:16 PM Luis Chamberlain <mcgrof@...nel.org> wrote:
>>>>>
>>>>> Linus want to take this in or should I just queue these up?
>>>>
>>>> I'll take it, and remove the unnecessary #ifdef/#endif. The #undef
>>>> might as well be unconditional - simpler and doesn't hurt.
>>>
>>> Sounds good. I just copy-pasted how it was done elsewhere. Luis was
>>> mentioning adding a wrapper to go this cleanly and I needed it in
>>> another instance too. So I'll look into doing that in a future patch.
>>
>> Virtual hug, or something hippie like that.
> 
> Thanks :)
> 
> I gave this a shot.
> 
> + #define set_module_param_prefix(prefix) \
> + #undef MODULE_PARAM_PREFIX              \
> + #define MODULE_PARAM_PREFIX prefix
> 
> I even wrote up a nice chunk of "function doc" before I tried
> compiling it. And once I compiled it, I was smacked in the head by the
> compiler for trying to put a #define inside a #define (Duh, the
> preprocessing in a single pass).
> 
> Before I tried this, I looked up the current uses of the "hacky" snippet:
> 
> $ git grep -l "define.*MODULE_PARAM_PREFIX" -- :^include/
> block/disk-events.c
> drivers/misc/cxl/fault.c
> drivers/mmc/core/block.c
> drivers/pci/pcie/aspm.c
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> drivers/tty/serial/8250/8250_core.c
> drivers/xen/balloon.c
> drivers/xen/events/events_base.c
> kernel/debug/kdb/kdb_main.c
> kernel/kcsan/core.c
> kernel/rcu/tree.c
> kernel/rcu/update.c
> mm/damon/reclaim.c
> mm/kfence/core.c
> 
> 
> In a lot of those files, there are a lot of module params that follow
> this snippet. Going on a tangent, some of the uses of #define
> MODULE_PARAM_PREFIX don't seem to have any obvious use or param
> macros.
> 
> So adding a module_param_prefixed() doesn't make sense to me either,
> because I'll have to repeat the same prefix in every usage of
> module_param_prefixed() AND I'll have to create a _prefixed() variant
> for other param macros too.
> 
> So, something like:
> module_param_prefixed("module.", sig_enforce, bool, 644);
> module_param_prefixed("module.", another_param1, bool, 644);
> module_param_prefixed("module.", another_param2, bool, 644);
> 
> Or replace "module." with a MY_PREFIX so that it's easy to ensure the
> string is the same across each use:
> #define MY_PREFIX "module."
> module_param_prefixed(MY_PREFIX, sig_enforce, bool, 644);
> module_param_prefixed(MY_PREFIX, another_param1, bool, 644);
> module_param_prefixed(MY_PREFIX, another_param2, bool, 644);
> 
> But at that point, all I'm avoiding is one #undef MODULE_PARAM_PREFIX
> and a whole lot of code churn.
> 
> One other option is to do something like:
> #ifndef MOD_PREFIX
> #define MODULE_PARAM_PREFIX KBUILD_MODNAME "."
> #else
> #define MODULE_PARAM_PREFIX MOD_PREFIX "."
> #endif
> 
> But that will have the limitation that MOD_PREFIX has to be defined
> before any #includes is in a file (similar to pr_fmt())  and doesn't
> allow you to redefine the prefix half way through a file -- which is
> also a thing that happens in drivers/tty/serial/8250/8250_core.c.
> 
> So, long story short, none of these options sound especially appealing
> that it's worth all the code churn across multiple maintainer trees.
> Let me know if you have other ideas that might work or you think one
> of the options I dismissed is actually worth doing.

I agree with your assessment. Nothing new is needed.

-- 
~Randy