2023-07-06 19:17:48

by Thomas Huth

[permalink] [raw]
Subject: [PATCH] x86: Remove the arch_calc_vm_prot_bits() macro from the uapi

The arch_calc_vm_prot_bits() macro uses VM_PKEY_BIT0 etc. which are
not part of the uapi, so the macro is completely useless for userspace.
It is also hidden behind the CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
config switch which we shouldn't expose to userspace. Thus let's move
this macro into a new internal header instead.

Signed-off-by: Thomas Huth <[email protected]>
---
arch/x86/include/asm/mman.h | 15 +++++++++++++++
arch/x86/include/uapi/asm/mman.h | 8 --------
scripts/headers_install.sh | 1 -
3 files changed, 15 insertions(+), 9 deletions(-)
create mode 100644 arch/x86/include/asm/mman.h

diff --git a/arch/x86/include/asm/mman.h b/arch/x86/include/asm/mman.h
new file mode 100644
index 0000000000000..12b820259b9f3
--- /dev/null
+++ b/arch/x86/include/asm/mman.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_MMAN_H__
+#define __ASM_MMAN_H__
+
+#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
+#define arch_calc_vm_prot_bits(prot, key) ( \
+ ((key) & 0x1 ? VM_PKEY_BIT0 : 0) | \
+ ((key) & 0x2 ? VM_PKEY_BIT1 : 0) | \
+ ((key) & 0x4 ? VM_PKEY_BIT2 : 0) | \
+ ((key) & 0x8 ? VM_PKEY_BIT3 : 0))
+#endif
+
+#include <uapi/asm/mman.h>
+
+#endif /* __ASM_MMAN_H__ */
diff --git a/arch/x86/include/uapi/asm/mman.h b/arch/x86/include/uapi/asm/mman.h
index 775dbd3aff736..a72e4f3e13b17 100644
--- a/arch/x86/include/uapi/asm/mman.h
+++ b/arch/x86/include/uapi/asm/mman.h
@@ -4,14 +4,6 @@

#define MAP_32BIT 0x40 /* only give out 32bit addresses */

-#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
-#define arch_calc_vm_prot_bits(prot, key) ( \
- ((key) & 0x1 ? VM_PKEY_BIT0 : 0) | \
- ((key) & 0x2 ? VM_PKEY_BIT1 : 0) | \
- ((key) & 0x4 ? VM_PKEY_BIT2 : 0) | \
- ((key) & 0x8 ? VM_PKEY_BIT3 : 0))
-#endif
-
#include <asm-generic/mman.h>

#endif /* _ASM_X86_MMAN_H */
diff --git a/scripts/headers_install.sh b/scripts/headers_install.sh
index afdddc82f02b3..56d3c338d91d7 100755
--- a/scripts/headers_install.sh
+++ b/scripts/headers_install.sh
@@ -81,7 +81,6 @@ arch/nios2/include/uapi/asm/swab.h:CONFIG_NIOS2_CI_SWAB_NO
arch/nios2/include/uapi/asm/swab.h:CONFIG_NIOS2_CI_SWAB_SUPPORT
arch/x86/include/uapi/asm/auxvec.h:CONFIG_IA32_EMULATION
arch/x86/include/uapi/asm/auxvec.h:CONFIG_X86_64
-arch/x86/include/uapi/asm/mman.h:CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
"

for c in $configs
--
2.39.3



2023-07-06 20:35:15

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH] x86: Remove the arch_calc_vm_prot_bits() macro from the uapi

On 7/6/23 13:22, Arnd Bergmann wrote:
> On Thu, Jul 6, 2023, at 21:02, Thomas Huth wrote:
>> The arch_calc_vm_prot_bits() macro uses VM_PKEY_BIT0 etc. which are
>> not part of the uapi, so the macro is completely useless for userspace.
>> It is also hidden behind the CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
>> config switch which we shouldn't expose to userspace. Thus let's move
>> this macro into a new internal header instead.
>>
>> Signed-off-by: Thomas Huth <[email protected]>
> Fixes: 8f62c883222c9 ("x86/mm/pkeys: Add arch-specific VMA protection bits")
> Reviewed-by: Arnd Bergmann <[email protected]>
>
> It looks like this was introduced right after the uapi split,
> and probably is the result of an incorrect rebase.

Yeah, I bet I just glossed over the "uapi" in the path.

Is this causing any real problems? Or is it OK to just send it along
during the next merge window with other random cleanups?

2023-07-06 20:40:33

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] x86: Remove the arch_calc_vm_prot_bits() macro from the uapi

On Thu, Jul 6, 2023, at 21:02, Thomas Huth wrote:
> The arch_calc_vm_prot_bits() macro uses VM_PKEY_BIT0 etc. which are
> not part of the uapi, so the macro is completely useless for userspace.
> It is also hidden behind the CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
> config switch which we shouldn't expose to userspace. Thus let's move
> this macro into a new internal header instead.
>
> Signed-off-by: Thomas Huth <[email protected]>

Fixes: 8f62c883222c9 ("x86/mm/pkeys: Add arch-specific VMA protection bits")
Reviewed-by: Arnd Bergmann <[email protected]>

It looks like this was introduced right after the uapi split,
and probably is the result of an incorrect rebase.

Arnd

2023-07-06 20:54:03

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] x86: Remove the arch_calc_vm_prot_bits() macro from the uapi

On Thu, Jul 6, 2023, at 22:30, Dave Hansen wrote:
> On 7/6/23 13:22, Arnd Bergmann wrote:
>> On Thu, Jul 6, 2023, at 21:02, Thomas Huth wrote:
>>> The arch_calc_vm_prot_bits() macro uses VM_PKEY_BIT0 etc. which are
>>> not part of the uapi, so the macro is completely useless for userspace.
>>> It is also hidden behind the CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
>>> config switch which we shouldn't expose to userspace. Thus let's move
>>> this macro into a new internal header instead.
>>>
>>> Signed-off-by: Thomas Huth <[email protected]>
>> Fixes: 8f62c883222c9 ("x86/mm/pkeys: Add arch-specific VMA protection bits")
>> Reviewed-by: Arnd Bergmann <[email protected]>
>>
>> It looks like this was introduced right after the uapi split,
>> and probably is the result of an incorrect rebase.
>
> Yeah, I bet I just glossed over the "uapi" in the path.
>
> Is this causing any real problems? Or is it OK to just send it along
> during the next merge window with other random cleanups?

It's pretty harmless, there are currently 12 remaining CONFIG_*
#ifdef checks in uapi headers, which scripts/headers_install.sh
has an exception for, and unlike some of the others, this one has
no relevance for the actual uapi.

Ultimately, the goal is to remove the list of known instances
from the script and just warn about all of them when new ones
get added, but it only becomes urgent when we get to everything
else.

Arnd

2023-07-07 07:03:12

by Thomas Huth

[permalink] [raw]
Subject: Re: [PATCH] x86: Remove the arch_calc_vm_prot_bits() macro from the uapi

On 06/07/2023 22.30, Dave Hansen wrote:
> On 7/6/23 13:22, Arnd Bergmann wrote:
>> On Thu, Jul 6, 2023, at 21:02, Thomas Huth wrote:
>>> The arch_calc_vm_prot_bits() macro uses VM_PKEY_BIT0 etc. which are
>>> not part of the uapi, so the macro is completely useless for userspace.
>>> It is also hidden behind the CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
>>> config switch which we shouldn't expose to userspace. Thus let's move
>>> this macro into a new internal header instead.
>>>
>>> Signed-off-by: Thomas Huth <[email protected]>
>> Fixes: 8f62c883222c9 ("x86/mm/pkeys: Add arch-specific VMA protection bits")
>> Reviewed-by: Arnd Bergmann <[email protected]>
>>
>> It looks like this was introduced right after the uapi split,
>> and probably is the result of an incorrect rebase.
>
> Yeah, I bet I just glossed over the "uapi" in the path.
>
> Is this causing any real problems? Or is it OK to just send it along
> during the next merge window with other random cleanups?

As Arnd already said, it's not a real problem - I just came across this file
while looking at the list in scripts/headers_install.sh.

Thomas



2023-07-07 09:40:42

by Nicolas Schier

[permalink] [raw]
Subject: Re: [PATCH] x86: Remove the arch_calc_vm_prot_bits() macro from the uapi

On Thu, Jul 06, 2023 at 09:02:17PM +0200 Thomas Huth wrote:
> The arch_calc_vm_prot_bits() macro uses VM_PKEY_BIT0 etc. which are
> not part of the uapi, so the macro is completely useless for userspace.
> It is also hidden behind the CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
> config switch which we shouldn't expose to userspace. Thus let's move
> this macro into a new internal header instead.
>
> Signed-off-by: Thomas Huth <[email protected]>
> ---

Thanks for fixing this config leakage.

Reviewed-by: Nicolas Schier <[email protected]>


> arch/x86/include/asm/mman.h | 15 +++++++++++++++
> arch/x86/include/uapi/asm/mman.h | 8 --------
> scripts/headers_install.sh | 1 -
> 3 files changed, 15 insertions(+), 9 deletions(-)
> create mode 100644 arch/x86/include/asm/mman.h
>
> diff --git a/arch/x86/include/asm/mman.h b/arch/x86/include/asm/mman.h
> new file mode 100644
> index 0000000000000..12b820259b9f3
> --- /dev/null
> +++ b/arch/x86/include/asm/mman.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_MMAN_H__
> +#define __ASM_MMAN_H__
> +
> +#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
> +#define arch_calc_vm_prot_bits(prot, key) ( \
> + ((key) & 0x1 ? VM_PKEY_BIT0 : 0) | \
> + ((key) & 0x2 ? VM_PKEY_BIT1 : 0) | \
> + ((key) & 0x4 ? VM_PKEY_BIT2 : 0) | \
> + ((key) & 0x8 ? VM_PKEY_BIT3 : 0))
> +#endif
> +
> +#include <uapi/asm/mman.h>
> +
> +#endif /* __ASM_MMAN_H__ */
> diff --git a/arch/x86/include/uapi/asm/mman.h b/arch/x86/include/uapi/asm/mman.h
> index 775dbd3aff736..a72e4f3e13b17 100644
> --- a/arch/x86/include/uapi/asm/mman.h
> +++ b/arch/x86/include/uapi/asm/mman.h
> @@ -4,14 +4,6 @@
>
> #define MAP_32BIT 0x40 /* only give out 32bit addresses */
>
> -#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
> -#define arch_calc_vm_prot_bits(prot, key) ( \
> - ((key) & 0x1 ? VM_PKEY_BIT0 : 0) | \
> - ((key) & 0x2 ? VM_PKEY_BIT1 : 0) | \
> - ((key) & 0x4 ? VM_PKEY_BIT2 : 0) | \
> - ((key) & 0x8 ? VM_PKEY_BIT3 : 0))
> -#endif
> -
> #include <asm-generic/mman.h>
>
> #endif /* _ASM_X86_MMAN_H */
> diff --git a/scripts/headers_install.sh b/scripts/headers_install.sh
> index afdddc82f02b3..56d3c338d91d7 100755
> --- a/scripts/headers_install.sh
> +++ b/scripts/headers_install.sh
> @@ -81,7 +81,6 @@ arch/nios2/include/uapi/asm/swab.h:CONFIG_NIOS2_CI_SWAB_NO
> arch/nios2/include/uapi/asm/swab.h:CONFIG_NIOS2_CI_SWAB_SUPPORT
> arch/x86/include/uapi/asm/auxvec.h:CONFIG_IA32_EMULATION
> arch/x86/include/uapi/asm/auxvec.h:CONFIG_X86_64
> -arch/x86/include/uapi/asm/mman.h:CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
> "
>
> for c in $configs
> --
> 2.39.3

--
epost|xmpp: [email protected] irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f
-- frykten for herren er opphav til kunnskap --