x86: Remove the arch_calc_vm_prot_bits() macro from the uapi

Message ID 20230706190217.371721-1-thuth@redhat.com
State New
Headers
Series x86: Remove the arch_calc_vm_prot_bits() macro from the uapi |

Commit Message

Thomas Huth July 6, 2023, 7:02 p.m. UTC
  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
  

Comments

Arnd Bergmann July 6, 2023, 8:22 p.m. UTC | #1
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
  
Dave Hansen July 6, 2023, 8:30 p.m. UTC | #2
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?
  
Arnd Bergmann July 6, 2023, 8:36 p.m. UTC | #3
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
  
Thomas Huth July 7, 2023, 6:48 a.m. UTC | #4
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
  
Nicolas Schier July 7, 2023, 9:29 a.m. UTC | #5
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
  

Patch

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