* [PATCH] x86/e820: fix the function type for e820__mapped_all
@ 2020-11-13 18:26 Sami Tolvanen
2020-11-13 18:34 ` Randy Dunlap
2020-11-27 12:15 ` Borislav Petkov
0 siblings, 2 replies; 6+ messages in thread
From: Sami Tolvanen @ 2020-11-13 18:26 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov
Cc: Kees Cook, x86, linux-kernel, Sami Tolvanen, Sedat Dilek
e820__mapped_all is passed as a callback to is_mmconf_reserved, which
expects a function of type:
typedef bool (*check_reserved_t)(u64 start, u64 end, unsigned type);
This trips indirect call checking with Clang's Control-Flow Integrity
(CFI). Change the last argument from enum e820_type to unsigned to fix
the type mismatch.
Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
arch/x86/include/asm/e820/api.h | 2 +-
arch/x86/kernel/e820.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/e820/api.h b/arch/x86/include/asm/e820/api.h
index e8f58ddd06d9..e872a796619d 100644
--- a/arch/x86/include/asm/e820/api.h
+++ b/arch/x86/include/asm/e820/api.h
@@ -12,7 +12,7 @@ extern unsigned long pci_mem_start;
extern bool e820__mapped_raw_any(u64 start, u64 end, enum e820_type type);
extern bool e820__mapped_any(u64 start, u64 end, enum e820_type type);
-extern bool e820__mapped_all(u64 start, u64 end, enum e820_type type);
+extern bool e820__mapped_all(u64 start, u64 end, unsigned type);
extern void e820__range_add (u64 start, u64 size, enum e820_type type);
extern u64 e820__range_update(u64 start, u64 size, enum e820_type old_type, enum e820_type new_type);
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 22aad412f965..9f6a4e9bca4c 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -145,7 +145,7 @@ static struct e820_entry *__e820__mapped_all(u64 start, u64 end,
/*
* This function checks if the entire range <start,end> is mapped with type.
*/
-bool __init e820__mapped_all(u64 start, u64 end, enum e820_type type)
+bool __init e820__mapped_all(u64 start, u64 end, unsigned type)
{
return __e820__mapped_all(start, end, type);
}
base-commit: 585e5b17b92dead8a3aca4e3c9876fbca5f7e0ba
--
2.29.2.299.gdc1121823c-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] x86/e820: fix the function type for e820__mapped_all
2020-11-13 18:26 [PATCH] x86/e820: fix the function type for e820__mapped_all Sami Tolvanen
@ 2020-11-13 18:34 ` Randy Dunlap
2020-11-13 18:59 ` Sami Tolvanen
2020-11-27 12:15 ` Borislav Petkov
1 sibling, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2020-11-13 18:34 UTC (permalink / raw)
To: Sami Tolvanen, Thomas Gleixner, Ingo Molnar, Borislav Petkov
Cc: Kees Cook, x86, linux-kernel, Sedat Dilek
On 11/13/20 10:26 AM, Sami Tolvanen wrote:
> e820__mapped_all is passed as a callback to is_mmconf_reserved, which
> expects a function of type:
>
> typedef bool (*check_reserved_t)(u64 start, u64 end, unsigned type);
>
> This trips indirect call checking with Clang's Control-Flow Integrity
> (CFI). Change the last argument from enum e820_type to unsigned to fix
> the type mismatch.
Hi,
Kernel style is no raw unsigned -- use unsigned int or unsigned long, please.
checkpatch should or could have found that issue.
> Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> ---
> arch/x86/include/asm/e820/api.h | 2 +-
> arch/x86/kernel/e820.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/e820/api.h b/arch/x86/include/asm/e820/api.h
> index e8f58ddd06d9..e872a796619d 100644
> --- a/arch/x86/include/asm/e820/api.h
> +++ b/arch/x86/include/asm/e820/api.h
> @@ -12,7 +12,7 @@ extern unsigned long pci_mem_start;
>
> extern bool e820__mapped_raw_any(u64 start, u64 end, enum e820_type type);
> extern bool e820__mapped_any(u64 start, u64 end, enum e820_type type);
> -extern bool e820__mapped_all(u64 start, u64 end, enum e820_type type);
> +extern bool e820__mapped_all(u64 start, u64 end, unsigned type);
>
> extern void e820__range_add (u64 start, u64 size, enum e820_type type);
> extern u64 e820__range_update(u64 start, u64 size, enum e820_type old_type, enum e820_type new_type);
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index 22aad412f965..9f6a4e9bca4c 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -145,7 +145,7 @@ static struct e820_entry *__e820__mapped_all(u64 start, u64 end,
> /*
> * This function checks if the entire range <start,end> is mapped with type.
> */
> -bool __init e820__mapped_all(u64 start, u64 end, enum e820_type type)
> +bool __init e820__mapped_all(u64 start, u64 end, unsigned type)
> {
> return __e820__mapped_all(start, end, type);
> }
>
> base-commit: 585e5b17b92dead8a3aca4e3c9876fbca5f7e0ba
>
--
~Randy
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] x86/e820: fix the function type for e820__mapped_all
2020-11-13 18:34 ` Randy Dunlap
@ 2020-11-13 18:59 ` Sami Tolvanen
2020-11-13 19:01 ` Randy Dunlap
0 siblings, 1 reply; 6+ messages in thread
From: Sami Tolvanen @ 2020-11-13 18:59 UTC (permalink / raw)
To: Randy Dunlap
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Kees Cook, X86 ML,
LKML, Sedat Dilek
On Fri, Nov 13, 2020 at 10:35 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 11/13/20 10:26 AM, Sami Tolvanen wrote:
> > e820__mapped_all is passed as a callback to is_mmconf_reserved, which
> > expects a function of type:
> >
> > typedef bool (*check_reserved_t)(u64 start, u64 end, unsigned type);
> >
> > This trips indirect call checking with Clang's Control-Flow Integrity
> > (CFI). Change the last argument from enum e820_type to unsigned to fix
> > the type mismatch.
>
> Hi,
>
> Kernel style is no raw unsigned -- use unsigned int or unsigned long, please.
>
> checkpatch should or could have found that issue.
It did, but the existing type definition for the callback in
pci/mmconfig-shared.c uses raw unsigned. I can add a patch to change
that to unsigned int as well, if you prefer.
Sami
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/e820: fix the function type for e820__mapped_all
2020-11-13 18:59 ` Sami Tolvanen
@ 2020-11-13 19:01 ` Randy Dunlap
0 siblings, 0 replies; 6+ messages in thread
From: Randy Dunlap @ 2020-11-13 19:01 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Kees Cook, X86 ML,
LKML, Sedat Dilek
On 11/13/20 10:59 AM, Sami Tolvanen wrote:
> On Fri, Nov 13, 2020 at 10:35 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>> On 11/13/20 10:26 AM, Sami Tolvanen wrote:
>>> e820__mapped_all is passed as a callback to is_mmconf_reserved, which
>>> expects a function of type:
>>>
>>> typedef bool (*check_reserved_t)(u64 start, u64 end, unsigned type);
>>>
>>> This trips indirect call checking with Clang's Control-Flow Integrity
>>> (CFI). Change the last argument from enum e820_type to unsigned to fix
>>> the type mismatch.
>>
>> Hi,
>>
>> Kernel style is no raw unsigned -- use unsigned int or unsigned long, please.
>>
>> checkpatch should or could have found that issue.
>
> It did, but the existing type definition for the callback in
> pci/mmconfig-shared.c uses raw unsigned. I can add a patch to change
> that to unsigned int as well, if you prefer.
Hm. :)
Well, yes, I prefer, but I can't require it.
thanks for the explanation.
--
~Randy
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/e820: fix the function type for e820__mapped_all
2020-11-13 18:26 [PATCH] x86/e820: fix the function type for e820__mapped_all Sami Tolvanen
2020-11-13 18:34 ` Randy Dunlap
@ 2020-11-27 12:15 ` Borislav Petkov
2020-11-30 19:16 ` Sami Tolvanen
1 sibling, 1 reply; 6+ messages in thread
From: Borislav Petkov @ 2020-11-27 12:15 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Thomas Gleixner, Ingo Molnar, Kees Cook, x86, linux-kernel, Sedat Dilek
On Fri, Nov 13, 2020 at 10:26:54AM -0800, Sami Tolvanen wrote:
> e820__mapped_all is passed as a callback to is_mmconf_reserved, which
> expects a function of type:
>
> typedef bool (*check_reserved_t)(u64 start, u64 end, unsigned type);
>
> This trips indirect call checking with Clang's Control-Flow Integrity
> (CFI). Change the last argument from enum e820_type to unsigned to fix
> the type mismatch.
>
> Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> ---
> arch/x86/include/asm/e820/api.h | 2 +-
> arch/x86/kernel/e820.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/e820/api.h b/arch/x86/include/asm/e820/api.h
> index e8f58ddd06d9..e872a796619d 100644
> --- a/arch/x86/include/asm/e820/api.h
> +++ b/arch/x86/include/asm/e820/api.h
> @@ -12,7 +12,7 @@ extern unsigned long pci_mem_start;
>
> extern bool e820__mapped_raw_any(u64 start, u64 end, enum e820_type type);
> extern bool e820__mapped_any(u64 start, u64 end, enum e820_type type);
> -extern bool e820__mapped_all(u64 start, u64 end, enum e820_type type);
> +extern bool e820__mapped_all(u64 start, u64 end, unsigned type);
I think the proper fix is to fix the typedef to:
typedef bool (*check_reserved_t)(u64 start, u64 end, enum e820_type type);
because
* is_mmconf_reserved() is passing in E820_TYPE_RESERVED which is enum e820_type
* is_acpi_reserved() is the other check_reserved_t function ptr and the last arg
type there is unused so it can just as well be enum e820_type.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] x86/e820: fix the function type for e820__mapped_all
2020-11-27 12:15 ` Borislav Petkov
@ 2020-11-30 19:16 ` Sami Tolvanen
0 siblings, 0 replies; 6+ messages in thread
From: Sami Tolvanen @ 2020-11-30 19:16 UTC (permalink / raw)
To: Borislav Petkov
Cc: Thomas Gleixner, Ingo Molnar, Kees Cook, X86 ML, LKML, Sedat Dilek
On Fri, Nov 27, 2020 at 4:15 AM Borislav Petkov <bp@alien8.de> wrote:
> I think the proper fix is to fix the typedef to:
>
> typedef bool (*check_reserved_t)(u64 start, u64 end, enum e820_type type);
>
> because
>
> * is_mmconf_reserved() is passing in E820_TYPE_RESERVED which is enum e820_type
>
> * is_acpi_reserved() is the other check_reserved_t function ptr and the last arg
> type there is unused so it can just as well be enum e820_type.
Sure, sounds good. I'll send out v3 to change the type argument to
enum e820_type.
Sami
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-30 19:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-13 18:26 [PATCH] x86/e820: fix the function type for e820__mapped_all Sami Tolvanen
2020-11-13 18:34 ` Randy Dunlap
2020-11-13 18:59 ` Sami Tolvanen
2020-11-13 19:01 ` Randy Dunlap
2020-11-27 12:15 ` Borislav Petkov
2020-11-30 19:16 ` Sami Tolvanen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).