linux-kernel - Re: [PATCH] mips: export icache_flush

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: <CAGXu5jLUueSb2u_Ki+nEL1CaBzyZyT1+7rhCB-0ZGB0nD5cT4g@mail.gmail.com>
Date:	Sat, 22 Mar 2014 15:05:27 -0600
From:	Kees Cook <keescook@...omium.org>
To:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
Cc:	Paul Gortmaker <paul.gortmaker@...driver.com>,
	Ralf Baechle <ralf@...ux-mips.org>,
	Sanjay Lal <sanjayl@...asys.com>,
	John Crispin <blogic@...nwrt.org>, linux-mips@...ux-mips.org,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mips: export icache_flush_range

On Sat, Mar 22, 2014 at 3:54 PM, Sergei Shtylyov
<sergei.shtylyov@...entembedded.com> wrote:
> On 03/23/2014 12:48 AM, Sergei Shtylyov wrote:
>
>>>> The lkdtm module performs tests against executable memory ranges, so
>>>> it needs to flush the icache for proper behaviors. Other architectures
>>>> already export this, so do the same for MIPS.
>
>
>>>> Signed-off-by: Kees Cook <keescook@...omium.org>
>>>> ---
>>>> This is currently untested! I'm building a MIPS cross-compiler now...
>>>> If someone can validate this fixes the build when lkdtm is a module,
>>>> that would be appreciated. :)
>>>> ---
>>>>   arch/mips/mm/cache.c |    1 +
>>>>   1 file changed, 1 insertion(+)
>
>
>>>> diff --git a/arch/mips/mm/cache.c b/arch/mips/mm/cache.c
>>>> index fde7e56d13fe..b3f1df13d9f6 100644
>>>> --- a/arch/mips/mm/cache.c
>>>> +++ b/arch/mips/mm/cache.c
>>>> @@ -38,6 +38,7 @@ void (*__flush_kernel_vmap_range)(unsigned long vaddr,
>>>> int
>>>> size);
>>>>   void (*__invalidate_kernel_vmap_range)(unsigned long vaddr, int size);
>>>>
>>>>   EXPORT_SYMBOL_GPL(__flush_kernel_vmap_range);
>>>> +EXPORT_SYMBOL_GPL(flush_icache_range);
>
>
>>>     Have you run this thru scripts/checkpatch.pl? It would have told you
>>> that
>>> an export should immediately follow the corresponding function body,
>>> AFAIK.
>
>
>>     Hm, it doesn't now but definitely used to...
>
>
>    Decided to check Documentation/CodingStyle, and it still codifies this:
>
> In source files, separate functions with one blank line.  If the function is
> exported, the EXPORT* macro for it should follow immediately after the
> closing
> function brace line.  E.g.:
>
> int system_is_up(void)
> {
>         return system_state == SYSTEM_RUNNING;
> }
> EXPORT_SYMBOL(system_is_up);
>
> WBR, Sergei

Yup, thanks. I know the style, but it seemed from the source file that
the style was to declare the function pointers in bulk, and then
explicitly export them in the next section, so I continued that style.
For example:

void (*flush_cache_sigtramp)(unsigned long addr);
void (*local_flush_data_cache_page)(void * addr);
void (*flush_data_cache_page)(unsigned long addr);
void (*flush_icache_all)(void);

EXPORT_SYMBOL_GPL(local_flush_data_cache_page);
EXPORT_SYMBOL(flush_data_cache_page);
EXPORT_SYMBOL(flush_icache_all);

Regardless, I'm happy to stick it in the middle of the function
pointers if that's preferred.

-Kees

-- 
Kees Cook
Chrome OS Security
--
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/