[v1] x86: Remove the arch_calc_vm_prot_bits() macro from the uapi

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

Posted by Thomas Huth 2 years, 7 months ago

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 <thuth@redhat.com>
---
 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

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

Posted by Arnd Bergmann 2 years, 7 months ago

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 <thuth@redhat.com>

Fixes: 8f62c883222c9 ("x86/mm/pkeys: Add arch-specific VMA protection bits")
Reviewed-by: Arnd Bergmann <arnd@arndb.de>

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

     Arnd

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

Posted by Dave Hansen 2 years, 7 months ago

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 <thuth@redhat.com>
> Fixes: 8f62c883222c9 ("x86/mm/pkeys: Add arch-specific VMA protection bits")
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> 
> 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?

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

Posted by Arnd Bergmann 2 years, 7 months ago

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 <thuth@redhat.com>
>> Fixes: 8f62c883222c9 ("x86/mm/pkeys: Add arch-specific VMA protection bits")
>> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>> 
>> 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

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

Posted by Thomas Huth 2 years, 7 months ago

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 <thuth@redhat.com>
>> Fixes: 8f62c883222c9 ("x86/mm/pkeys: Add arch-specific VMA protection bits")
>> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>>
>> 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

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

Posted by Nicolas Schier 2 years, 7 months ago

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 <thuth@redhat.com>
> ---

Thanks for fixing this config leakage.

Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>


>  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: nicolas@fjasle.eu          irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb  c82b 7d97 0932 55a0 ce7f
     -- frykten for herren er opphav til kunnskap --