linux-kernel - Re: [PATCH v8 0/3] ARM: Implement MODULE

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: <b26651f2-a5ca-ff5d-23e4-05b7eb7d9583@gmail.com>
Date:   Mon, 19 Apr 2021 14:54:30 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Qais Yousef <qais.yousef@....com>,
        Alexander Sverdlin <alexander.sverdlin@...ia.com>
Cc:     linux-arm-kernel@...ts.infradead.org,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>,
        Russell King <linux@...linux.org.uk>,
        linux-kernel@...r.kernel.org, Ard Biesheuvel <ardb@...nel.org>,
        Linus Walleij <linus.walleij@...aro.org>
Subject: Re: [PATCH v8 0/3] ARM: Implement MODULE_PLT support in FTRACE



On 4/12/2021 4:08 AM, Qais Yousef wrote:
> Hi Alexander
> 
> Fixing Ard's email as the Linaro one keeps bouncing back. Please fix that in
> your future postings.
> 
> On 04/12/21 08:28, Alexander Sverdlin wrote:
>> Hi!
>>
>> On 09/04/2021 17:33, Qais Yousef wrote:
>>> I still think the ifdefery in patch 3 is ugly. Any reason my suggestion didn't
>>> work out for you? I struggle to see how this is better and why it was hard to
>>> incorporate my suggestion.
>>>
>>> For example
>>>
>>> 	-       old = ftrace_call_replace(ip, adjust_address(rec, addr));
>>> 	+#ifdef CONFIG_ARM_MODULE_PLTS
>>> 	+       /* mod is only supplied during module loading */
>>> 	+       if (!mod)
>>> 	+               mod = rec->arch.mod;
>>> 	+       else
>>> 	+               rec->arch.mod = mod;
>>> 	+#endif
>>> 	+
>>> 	+       old = ftrace_call_replace(ip, aaddr,
>>> 	+                                 !IS_ENABLED(CONFIG_ARM_MODULE_PLTS) || !mod);
>>> 	+#ifdef CONFIG_ARM_MODULE_PLTS
>>> 	+       if (!old && mod) {
>>> 	+               aaddr = get_module_plt(mod, ip, aaddr);
>>> 	+               old = ftrace_call_replace(ip, aaddr, true);
>>> 	+       }
>>> 	+#endif
>>> 	+
>>>
>>> There's an ifdef, followed by a code that embeds
>>> !IS_ENABLED(CONFIG_ARM_MODULE_PLTS) followed by another ifdef :-/
>>
>> No, it's actually two small ifdefed blocks added before and after an original call,
>> which parameters have been modified as well. The issue with arch.mod was explained
>> by Steven Rostedt, maybe you've missed his email.
> 
> If you're referring to arch.mod having to be protected by the ifdef I did
> address that. Please look at my patch.
> 
> My comment here refers to the ugliness of this ifdefery. Introducing 2 simple
> wrapper functions would address that as I've demonstrated in my
> suggestion/patch.

What is the plan to move forward with this patch series, should v8 be
submitted into RMK's patch tracker and improved upon from there, or do
you feel like your suggestion needs to be addressed right away?
-- 
Florian