* [PATCH v18 01/15] arm64: untag user pointers in access_ok and __uaccess_mask_ptr
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
@ 2019-06-24 14:32 ` Andrey Konovalov
2019-06-24 14:32 ` [PATCH v18 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI Andrey Konovalov
` (14 subsequent siblings)
15 siblings, 0 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
copy_from_user (and a few other similar functions) are used to copy data
from user memory into the kernel memory or vice versa. Since a user can
provided a tagged pointer to one of the syscalls that use copy_from_user,
we need to correctly handle such pointers.
Do this by untagging user pointers in access_ok and in __uaccess_mask_ptr,
before performing access validity checks.
Note, that this patch only temporarily untags the pointers to perform the
checks, but then passes them as is into the kernel internals.
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
arch/arm64/include/asm/uaccess.h | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 5a1c32260c1f..a138e3b4f717 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -62,6 +62,8 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si
{
unsigned long ret, limit = current_thread_info()->addr_limit;
+ addr = untagged_addr(addr);
+
__chk_user_ptr(addr);
asm volatile(
// A + B <= C + 1 for all A,B,C, in four easy steps:
@@ -215,7 +217,8 @@ static inline void uaccess_enable_not_uao(void)
/*
* Sanitise a uaccess pointer such that it becomes NULL if above the
- * current addr_limit.
+ * current addr_limit. In case the pointer is tagged (has the top byte set),
+ * untag the pointer before checking.
*/
#define uaccess_mask_ptr(ptr) (__typeof__(ptr))__uaccess_mask_ptr(ptr)
static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
@@ -223,10 +226,11 @@ static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
void __user *safe_ptr;
asm volatile(
- " bics xzr, %1, %2\n"
+ " bics xzr, %3, %2\n"
" csel %0, %1, xzr, eq\n"
: "=&r" (safe_ptr)
- : "r" (ptr), "r" (current_thread_info()->addr_limit)
+ : "r" (ptr), "r" (current_thread_info()->addr_limit),
+ "r" (untagged_addr(ptr))
: "cc");
csdb();
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v18 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
2019-06-24 14:32 ` [PATCH v18 01/15] arm64: untag user pointers in access_ok and __uaccess_mask_ptr Andrey Konovalov
@ 2019-06-24 14:32 ` Andrey Konovalov
2019-06-24 15:04 ` Kees Cook
2019-06-24 14:32 ` [PATCH v18 03/15] lib: untag user pointers in strn*_user Andrey Konovalov
` (13 subsequent siblings)
15 siblings, 1 reply; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov
From: Catalin Marinas <catalin.marinas@arm.com>
It is not desirable to relax the ABI to allow tagged user addresses into
the kernel indiscriminately. This patch introduces a prctl() interface
for enabling or disabling the tagged ABI with a global sysctl control
for preventing applications from enabling the relaxed ABI (meant for
testing user-space prctl() return error checking without reconfiguring
the kernel). The ABI properties are inherited by threads of the same
application and fork()'ed children but cleared on execve(). A Kconfig
option allows the overall disabling of the relaxed ABI.
The PR_SET_TAGGED_ADDR_CTRL will be expanded in the future to handle
MTE-specific settings like imprecise vs precise exceptions.
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
arch/arm64/Kconfig | 9 ++++
arch/arm64/include/asm/processor.h | 8 ++++
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/include/asm/uaccess.h | 4 +-
arch/arm64/kernel/process.c | 72 ++++++++++++++++++++++++++++
include/uapi/linux/prctl.h | 5 ++
kernel/sys.c | 12 +++++
7 files changed, 110 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 697ea0510729..55fbaf20af2d 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1107,6 +1107,15 @@ config ARM64_SW_TTBR0_PAN
zeroed area and reserved ASID. The user access routines
restore the valid TTBR0_EL1 temporarily.
+config ARM64_TAGGED_ADDR_ABI
+ bool "Enable the tagged user addresses syscall ABI"
+ default y
+ help
+ When this option is enabled, user applications can opt in to a
+ relaxed ABI via prctl() allowing tagged addresses to be passed
+ to system calls as pointer arguments. For details, see
+ Documentation/arm64/tagged-address-abi.txt.
+
menuconfig COMPAT
bool "Kernel support for 32-bit EL0"
depends on ARM64_4K_PAGES || EXPERT
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index fd5b1a4efc70..ee86070a28d4 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -296,6 +296,14 @@ extern void __init minsigstksz_setup(void);
/* PR_PAC_RESET_KEYS prctl */
#define PAC_RESET_KEYS(tsk, arg) ptrauth_prctl_reset_keys(tsk, arg)
+#ifdef CONFIG_ARM64_TAGGED_ADDR_ABI
+/* PR_{SET,GET}_TAGGED_ADDR_CTRL prctl */
+long set_tagged_addr_ctrl(unsigned long arg);
+long get_tagged_addr_ctrl(void);
+#define SET_TAGGED_ADDR_CTRL(arg) set_tagged_addr_ctrl(arg)
+#define GET_TAGGED_ADDR_CTRL() get_tagged_addr_ctrl()
+#endif
+
/*
* For CONFIG_GCC_PLUGIN_STACKLEAK
*
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 2372e97db29c..4f81c4f15404 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -88,6 +88,7 @@ void arch_release_task_struct(struct task_struct *tsk);
#define TIF_SVE 23 /* Scalable Vector Extension in use */
#define TIF_SVE_VL_INHERIT 24 /* Inherit sve_vl_onexec across exec */
#define TIF_SSBD 25 /* Wants SSB mitigation */
+#define TIF_TAGGED_ADDR 26 /* Allow tagged user addresses */
#define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
#define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index a138e3b4f717..097d6bfac0b7 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -62,7 +62,9 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si
{
unsigned long ret, limit = current_thread_info()->addr_limit;
- addr = untagged_addr(addr);
+ if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) &&
+ test_thread_flag(TIF_TAGGED_ADDR))
+ addr = untagged_addr(addr);
__chk_user_ptr(addr);
asm volatile(
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 9856395ccdb7..60e70158a4a1 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -19,6 +19,7 @@
#include <linux/kernel.h>
#include <linux/mm.h>
#include <linux/stddef.h>
+#include <linux/sysctl.h>
#include <linux/unistd.h>
#include <linux/user.h>
#include <linux/delay.h>
@@ -307,11 +308,18 @@ static void tls_thread_flush(void)
}
}
+static void flush_tagged_addr_state(void)
+{
+ if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI))
+ clear_thread_flag(TIF_TAGGED_ADDR);
+}
+
void flush_thread(void)
{
fpsimd_flush_thread();
tls_thread_flush();
flush_ptrace_hw_breakpoint(current);
+ flush_tagged_addr_state();
}
void release_thread(struct task_struct *dead_task)
@@ -541,3 +549,67 @@ void arch_setup_new_exec(void)
ptrauth_thread_init_user(current);
}
+
+#ifdef CONFIG_ARM64_TAGGED_ADDR_ABI
+/*
+ * Control the relaxed ABI allowing tagged user addresses into the kernel.
+ */
+static unsigned int tagged_addr_prctl_allowed = 1;
+
+long set_tagged_addr_ctrl(unsigned long arg)
+{
+ if (!tagged_addr_prctl_allowed)
+ return -EINVAL;
+ if (is_compat_task())
+ return -EINVAL;
+ if (arg & ~PR_TAGGED_ADDR_ENABLE)
+ return -EINVAL;
+
+ update_thread_flag(TIF_TAGGED_ADDR, arg & PR_TAGGED_ADDR_ENABLE);
+
+ return 0;
+}
+
+long get_tagged_addr_ctrl(void)
+{
+ if (!tagged_addr_prctl_allowed)
+ return -EINVAL;
+ if (is_compat_task())
+ return -EINVAL;
+
+ if (test_thread_flag(TIF_TAGGED_ADDR))
+ return PR_TAGGED_ADDR_ENABLE;
+
+ return 0;
+}
+
+/*
+ * Global sysctl to disable the tagged user addresses support. This control
+ * only prevents the tagged address ABI enabling via prctl() and does not
+ * disable it for tasks that already opted in to the relaxed ABI.
+ */
+static int zero;
+static int one = 1;
+
+static struct ctl_table tagged_addr_sysctl_table[] = {
+ {
+ .procname = "tagged_addr",
+ .mode = 0644,
+ .data = &tagged_addr_prctl_allowed,
+ .maxlen = sizeof(int),
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &zero,
+ .extra2 = &one,
+ },
+ { }
+};
+
+static int __init tagged_addr_init(void)
+{
+ if (!register_sysctl("abi", tagged_addr_sysctl_table))
+ return -EINVAL;
+ return 0;
+}
+
+core_initcall(tagged_addr_init);
+#endif /* CONFIG_ARM64_TAGGED_ADDR_ABI */
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 094bb03b9cc2..2e927b3e9d6c 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -229,4 +229,9 @@ struct prctl_mm_map {
# define PR_PAC_APDBKEY (1UL << 3)
# define PR_PAC_APGAKEY (1UL << 4)
+/* Tagged user address controls for arm64 */
+#define PR_SET_TAGGED_ADDR_CTRL 55
+#define PR_GET_TAGGED_ADDR_CTRL 56
+# define PR_TAGGED_ADDR_ENABLE (1UL << 0)
+
#endif /* _LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index 2969304c29fe..c6c4d5358bd3 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -124,6 +124,12 @@
#ifndef PAC_RESET_KEYS
# define PAC_RESET_KEYS(a, b) (-EINVAL)
#endif
+#ifndef SET_TAGGED_ADDR_CTRL
+# define SET_TAGGED_ADDR_CTRL(a) (-EINVAL)
+#endif
+#ifndef GET_TAGGED_ADDR_CTRL
+# define GET_TAGGED_ADDR_CTRL() (-EINVAL)
+#endif
/*
* this is where the system-wide overflow UID and GID are defined, for
@@ -2492,6 +2498,12 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
return -EINVAL;
error = PAC_RESET_KEYS(me, arg2);
break;
+ case PR_SET_TAGGED_ADDR_CTRL:
+ error = SET_TAGGED_ADDR_CTRL(arg2);
+ break;
+ case PR_GET_TAGGED_ADDR_CTRL:
+ error = GET_TAGGED_ADDR_CTRL();
+ break;
default:
error = -EINVAL;
break;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v18 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI
2019-06-24 14:32 ` [PATCH v18 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI Andrey Konovalov
@ 2019-06-24 15:04 ` Kees Cook
0 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2019-06-24 15:04 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander, Alex Williamson, Leon Romanovsky,
Luc Van Oostenryck, Dave Martin, Khalid Aziz, enh,
Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy
On Mon, Jun 24, 2019 at 04:32:47PM +0200, Andrey Konovalov wrote:
> From: Catalin Marinas <catalin.marinas@arm.com>
>
> It is not desirable to relax the ABI to allow tagged user addresses into
> the kernel indiscriminately. This patch introduces a prctl() interface
> for enabling or disabling the tagged ABI with a global sysctl control
> for preventing applications from enabling the relaxed ABI (meant for
> testing user-space prctl() return error checking without reconfiguring
> the kernel). The ABI properties are inherited by threads of the same
> application and fork()'ed children but cleared on execve(). A Kconfig
> option allows the overall disabling of the relaxed ABI.
>
> The PR_SET_TAGGED_ADDR_CTRL will be expanded in the future to handle
> MTE-specific settings like imprecise vs precise exceptions.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
> arch/arm64/Kconfig | 9 ++++
> arch/arm64/include/asm/processor.h | 8 ++++
> arch/arm64/include/asm/thread_info.h | 1 +
> arch/arm64/include/asm/uaccess.h | 4 +-
> arch/arm64/kernel/process.c | 72 ++++++++++++++++++++++++++++
> include/uapi/linux/prctl.h | 5 ++
> kernel/sys.c | 12 +++++
> 7 files changed, 110 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 697ea0510729..55fbaf20af2d 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1107,6 +1107,15 @@ config ARM64_SW_TTBR0_PAN
> zeroed area and reserved ASID. The user access routines
> restore the valid TTBR0_EL1 temporarily.
>
> +config ARM64_TAGGED_ADDR_ABI
> + bool "Enable the tagged user addresses syscall ABI"
> + default y
> + help
> + When this option is enabled, user applications can opt in to a
> + relaxed ABI via prctl() allowing tagged addresses to be passed
> + to system calls as pointer arguments. For details, see
> + Documentation/arm64/tagged-address-abi.txt.
> +
> menuconfig COMPAT
> bool "Kernel support for 32-bit EL0"
> depends on ARM64_4K_PAGES || EXPERT
> diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> index fd5b1a4efc70..ee86070a28d4 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -296,6 +296,14 @@ extern void __init minsigstksz_setup(void);
> /* PR_PAC_RESET_KEYS prctl */
> #define PAC_RESET_KEYS(tsk, arg) ptrauth_prctl_reset_keys(tsk, arg)
>
> +#ifdef CONFIG_ARM64_TAGGED_ADDR_ABI
> +/* PR_{SET,GET}_TAGGED_ADDR_CTRL prctl */
> +long set_tagged_addr_ctrl(unsigned long arg);
> +long get_tagged_addr_ctrl(void);
> +#define SET_TAGGED_ADDR_CTRL(arg) set_tagged_addr_ctrl(arg)
> +#define GET_TAGGED_ADDR_CTRL() get_tagged_addr_ctrl()
> +#endif
> +
> /*
> * For CONFIG_GCC_PLUGIN_STACKLEAK
> *
> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 2372e97db29c..4f81c4f15404 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -88,6 +88,7 @@ void arch_release_task_struct(struct task_struct *tsk);
> #define TIF_SVE 23 /* Scalable Vector Extension in use */
> #define TIF_SVE_VL_INHERIT 24 /* Inherit sve_vl_onexec across exec */
> #define TIF_SSBD 25 /* Wants SSB mitigation */
> +#define TIF_TAGGED_ADDR 26 /* Allow tagged user addresses */
>
> #define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
> #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index a138e3b4f717..097d6bfac0b7 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -62,7 +62,9 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si
> {
> unsigned long ret, limit = current_thread_info()->addr_limit;
>
> - addr = untagged_addr(addr);
> + if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) &&
> + test_thread_flag(TIF_TAGGED_ADDR))
> + addr = untagged_addr(addr);
>
> __chk_user_ptr(addr);
> asm volatile(
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 9856395ccdb7..60e70158a4a1 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -19,6 +19,7 @@
> #include <linux/kernel.h>
> #include <linux/mm.h>
> #include <linux/stddef.h>
> +#include <linux/sysctl.h>
> #include <linux/unistd.h>
> #include <linux/user.h>
> #include <linux/delay.h>
> @@ -307,11 +308,18 @@ static void tls_thread_flush(void)
> }
> }
>
> +static void flush_tagged_addr_state(void)
> +{
> + if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI))
> + clear_thread_flag(TIF_TAGGED_ADDR);
> +}
> +
> void flush_thread(void)
> {
> fpsimd_flush_thread();
> tls_thread_flush();
> flush_ptrace_hw_breakpoint(current);
> + flush_tagged_addr_state();
> }
>
> void release_thread(struct task_struct *dead_task)
> @@ -541,3 +549,67 @@ void arch_setup_new_exec(void)
>
> ptrauth_thread_init_user(current);
> }
> +
> +#ifdef CONFIG_ARM64_TAGGED_ADDR_ABI
> +/*
> + * Control the relaxed ABI allowing tagged user addresses into the kernel.
> + */
> +static unsigned int tagged_addr_prctl_allowed = 1;
> +
> +long set_tagged_addr_ctrl(unsigned long arg)
> +{
> + if (!tagged_addr_prctl_allowed)
> + return -EINVAL;
> + if (is_compat_task())
> + return -EINVAL;
> + if (arg & ~PR_TAGGED_ADDR_ENABLE)
> + return -EINVAL;
> +
> + update_thread_flag(TIF_TAGGED_ADDR, arg & PR_TAGGED_ADDR_ENABLE);
> +
> + return 0;
> +}
> +
> +long get_tagged_addr_ctrl(void)
> +{
> + if (!tagged_addr_prctl_allowed)
> + return -EINVAL;
> + if (is_compat_task())
> + return -EINVAL;
> +
> + if (test_thread_flag(TIF_TAGGED_ADDR))
> + return PR_TAGGED_ADDR_ENABLE;
> +
> + return 0;
> +}
> +
> +/*
> + * Global sysctl to disable the tagged user addresses support. This control
> + * only prevents the tagged address ABI enabling via prctl() and does not
> + * disable it for tasks that already opted in to the relaxed ABI.
> + */
> +static int zero;
> +static int one = 1;
> +
> +static struct ctl_table tagged_addr_sysctl_table[] = {
> + {
> + .procname = "tagged_addr",
> + .mode = 0644,
> + .data = &tagged_addr_prctl_allowed,
> + .maxlen = sizeof(int),
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = &zero,
> + .extra2 = &one,
> + },
> + { }
> +};
> +
> +static int __init tagged_addr_init(void)
> +{
> + if (!register_sysctl("abi", tagged_addr_sysctl_table))
> + return -EINVAL;
> + return 0;
> +}
> +
> +core_initcall(tagged_addr_init);
> +#endif /* CONFIG_ARM64_TAGGED_ADDR_ABI */
> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index 094bb03b9cc2..2e927b3e9d6c 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -229,4 +229,9 @@ struct prctl_mm_map {
> # define PR_PAC_APDBKEY (1UL << 3)
> # define PR_PAC_APGAKEY (1UL << 4)
>
> +/* Tagged user address controls for arm64 */
> +#define PR_SET_TAGGED_ADDR_CTRL 55
> +#define PR_GET_TAGGED_ADDR_CTRL 56
> +# define PR_TAGGED_ADDR_ENABLE (1UL << 0)
> +
> #endif /* _LINUX_PRCTL_H */
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 2969304c29fe..c6c4d5358bd3 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -124,6 +124,12 @@
> #ifndef PAC_RESET_KEYS
> # define PAC_RESET_KEYS(a, b) (-EINVAL)
> #endif
> +#ifndef SET_TAGGED_ADDR_CTRL
> +# define SET_TAGGED_ADDR_CTRL(a) (-EINVAL)
> +#endif
> +#ifndef GET_TAGGED_ADDR_CTRL
> +# define GET_TAGGED_ADDR_CTRL() (-EINVAL)
> +#endif
>
> /*
> * this is where the system-wide overflow UID and GID are defined, for
> @@ -2492,6 +2498,12 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> return -EINVAL;
> error = PAC_RESET_KEYS(me, arg2);
> break;
> + case PR_SET_TAGGED_ADDR_CTRL:
> + error = SET_TAGGED_ADDR_CTRL(arg2);
> + break;
> + case PR_GET_TAGGED_ADDR_CTRL:
> + error = GET_TAGGED_ADDR_CTRL();
> + break;
> default:
> error = -EINVAL;
> break;
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v18 03/15] lib: untag user pointers in strn*_user
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
2019-06-24 14:32 ` [PATCH v18 01/15] arm64: untag user pointers in access_ok and __uaccess_mask_ptr Andrey Konovalov
2019-06-24 14:32 ` [PATCH v18 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI Andrey Konovalov
@ 2019-06-24 14:32 ` Andrey Konovalov
2019-06-24 14:32 ` [PATCH v18 04/15] mm: untag user pointers passed to memory syscalls Andrey Konovalov
` (12 subsequent siblings)
15 siblings, 0 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
strncpy_from_user and strnlen_user accept user addresses as arguments, and
do not go through the same path as copy_from_user and others, so here we
need to handle the case of tagged user addresses separately.
Untag user pointers passed to these functions.
Note, that this patch only temporarily untags the pointers to perform
validity checks, but then uses them as is to perform user memory accesses.
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
Acked-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
lib/strncpy_from_user.c | 3 ++-
lib/strnlen_user.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
index 023ba9f3b99f..dccb95af6003 100644
--- a/lib/strncpy_from_user.c
+++ b/lib/strncpy_from_user.c
@@ -6,6 +6,7 @@
#include <linux/uaccess.h>
#include <linux/kernel.h>
#include <linux/errno.h>
+#include <linux/mm.h>
#include <asm/byteorder.h>
#include <asm/word-at-a-time.h>
@@ -108,7 +109,7 @@ long strncpy_from_user(char *dst, const char __user *src, long count)
return 0;
max_addr = user_addr_max();
- src_addr = (unsigned long)src;
+ src_addr = (unsigned long)untagged_addr(src);
if (likely(src_addr < max_addr)) {
unsigned long max = max_addr - src_addr;
long retval;
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
index 7f2db3fe311f..28ff554a1be8 100644
--- a/lib/strnlen_user.c
+++ b/lib/strnlen_user.c
@@ -2,6 +2,7 @@
#include <linux/kernel.h>
#include <linux/export.h>
#include <linux/uaccess.h>
+#include <linux/mm.h>
#include <asm/word-at-a-time.h>
@@ -109,7 +110,7 @@ long strnlen_user(const char __user *str, long count)
return 0;
max_addr = user_addr_max();
- src_addr = (unsigned long)str;
+ src_addr = (unsigned long)untagged_addr(str);
if (likely(src_addr < max_addr)) {
unsigned long max = max_addr - src_addr;
long retval;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v18 04/15] mm: untag user pointers passed to memory syscalls
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
` (2 preceding siblings ...)
2019-06-24 14:32 ` [PATCH v18 03/15] lib: untag user pointers in strn*_user Andrey Konovalov
@ 2019-06-24 14:32 ` Andrey Konovalov
2019-06-24 14:32 ` [PATCH v18 05/15] mm: untag user pointers in mm/gup.c Andrey Konovalov
` (11 subsequent siblings)
15 siblings, 0 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
This patch allows tagged pointers to be passed to the following memory
syscalls: get_mempolicy, madvise, mbind, mincore, mlock, mlock2, mprotect,
mremap, msync, munlock, move_pages.
The mmap and mremap syscalls do not currently accept tagged addresses.
Architectures may interpret the tag as a background colour for the
corresponding vma.
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/madvise.c | 2 ++
mm/mempolicy.c | 3 +++
mm/migrate.c | 2 +-
mm/mincore.c | 2 ++
mm/mlock.c | 4 ++++
mm/mprotect.c | 2 ++
mm/mremap.c | 7 +++++++
mm/msync.c | 2 ++
8 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/mm/madvise.c b/mm/madvise.c
index 628022e674a7..39b82f8a698f 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -810,6 +810,8 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
size_t len;
struct blk_plug plug;
+ start = untagged_addr(start);
+
if (!madvise_behavior_valid(behavior))
return error;
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 01600d80ae01..78e0a88b2680 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1360,6 +1360,7 @@ static long kernel_mbind(unsigned long start, unsigned long len,
int err;
unsigned short mode_flags;
+ start = untagged_addr(start);
mode_flags = mode & MPOL_MODE_FLAGS;
mode &= ~MPOL_MODE_FLAGS;
if (mode >= MPOL_MAX)
@@ -1517,6 +1518,8 @@ static int kernel_get_mempolicy(int __user *policy,
int uninitialized_var(pval);
nodemask_t nodes;
+ addr = untagged_addr(addr);
+
if (nmask != NULL && maxnode < nr_node_ids)
return -EINVAL;
diff --git a/mm/migrate.c b/mm/migrate.c
index f2ecc2855a12..d22c45cf36b2 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1616,7 +1616,7 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
goto out_flush;
if (get_user(node, nodes + i))
goto out_flush;
- addr = (unsigned long)p;
+ addr = (unsigned long)untagged_addr(p);
err = -ENODEV;
if (node < 0 || node >= MAX_NUMNODES)
diff --git a/mm/mincore.c b/mm/mincore.c
index c3f058bd0faf..64c322ed845c 100644
--- a/mm/mincore.c
+++ b/mm/mincore.c
@@ -249,6 +249,8 @@ SYSCALL_DEFINE3(mincore, unsigned long, start, size_t, len,
unsigned long pages;
unsigned char *tmp;
+ start = untagged_addr(start);
+
/* Check the start address: needs to be page-aligned.. */
if (start & ~PAGE_MASK)
return -EINVAL;
diff --git a/mm/mlock.c b/mm/mlock.c
index a90099da4fb4..a72c1eeded77 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -674,6 +674,8 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla
unsigned long lock_limit;
int error = -ENOMEM;
+ start = untagged_addr(start);
+
if (!can_do_mlock())
return -EPERM;
@@ -735,6 +737,8 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)
{
int ret;
+ start = untagged_addr(start);
+
len = PAGE_ALIGN(len + (offset_in_page(start)));
start &= PAGE_MASK;
diff --git a/mm/mprotect.c b/mm/mprotect.c
index bf38dfbbb4b4..19f981b733bc 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -465,6 +465,8 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
const bool rier = (current->personality & READ_IMPLIES_EXEC) &&
(prot & PROT_READ);
+ start = untagged_addr(start);
+
prot &= ~(PROT_GROWSDOWN|PROT_GROWSUP);
if (grows == (PROT_GROWSDOWN|PROT_GROWSUP)) /* can't be both */
return -EINVAL;
diff --git a/mm/mremap.c b/mm/mremap.c
index fc241d23cd97..64c9a3b8be0a 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -606,6 +606,13 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
LIST_HEAD(uf_unmap_early);
LIST_HEAD(uf_unmap);
+ /*
+ * Architectures may interpret the tag passed to mmap as a background
+ * colour for the corresponding vma. For mremap we don't allow tagged
+ * new_addr to preserve similar behaviour to mmap.
+ */
+ addr = untagged_addr(addr);
+
if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE))
return ret;
diff --git a/mm/msync.c b/mm/msync.c
index ef30a429623a..c3bd3e75f687 100644
--- a/mm/msync.c
+++ b/mm/msync.c
@@ -37,6 +37,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
int unmapped_error = 0;
int error = -EINVAL;
+ start = untagged_addr(start);
+
if (flags & ~(MS_ASYNC | MS_INVALIDATE | MS_SYNC))
goto out;
if (offset_in_page(start))
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v18 05/15] mm: untag user pointers in mm/gup.c
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
` (3 preceding siblings ...)
2019-06-24 14:32 ` [PATCH v18 04/15] mm: untag user pointers passed to memory syscalls Andrey Konovalov
@ 2019-06-24 14:32 ` Andrey Konovalov
2019-06-24 14:32 ` [PATCH v18 06/15] mm: untag user pointers in get_vaddr_frames Andrey Konovalov
` (10 subsequent siblings)
15 siblings, 0 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
mm/gup.c provides a kernel interface that accepts user addresses and
manipulates user pages directly (for example get_user_pages, that is used
by the futex syscall). Since a user can provided tagged addresses, we need
to handle this case.
Add untagging to gup.c functions that use user addresses for vma lookups.
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/gup.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mm/gup.c b/mm/gup.c
index ddde097cf9e4..c37df3d455a2 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -802,6 +802,8 @@ static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
if (!nr_pages)
return 0;
+ start = untagged_addr(start);
+
VM_BUG_ON(!!pages != !!(gup_flags & FOLL_GET));
/*
@@ -964,6 +966,8 @@ int fixup_user_fault(struct task_struct *tsk, struct mm_struct *mm,
struct vm_area_struct *vma;
vm_fault_t ret, major = 0;
+ address = untagged_addr(address);
+
if (unlocked)
fault_flags |= FAULT_FLAG_ALLOW_RETRY;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v18 06/15] mm: untag user pointers in get_vaddr_frames
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
` (4 preceding siblings ...)
2019-06-24 14:32 ` [PATCH v18 05/15] mm: untag user pointers in mm/gup.c Andrey Konovalov
@ 2019-06-24 14:32 ` Andrey Konovalov
2019-06-24 14:32 ` [PATCH v18 07/15] fs/namespace: untag user pointers in copy_mount_options Andrey Konovalov
` (9 subsequent siblings)
15 siblings, 0 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
get_vaddr_frames uses provided user pointers for vma lookups, which can
only by done with untagged pointers. Instead of locating and changing
all callers of this function, perform untagging in it.
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/frame_vector.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index c64dca6e27c2..c431ca81dad5 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -46,6 +46,8 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
if (WARN_ON_ONCE(nr_frames > vec->nr_allocated))
nr_frames = vec->nr_allocated;
+ start = untagged_addr(start);
+
down_read(&mm->mmap_sem);
locked = 1;
vma = find_vma_intersection(mm, start, start + 1);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v18 07/15] fs/namespace: untag user pointers in copy_mount_options
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
` (5 preceding siblings ...)
2019-06-24 14:32 ` [PATCH v18 06/15] mm: untag user pointers in get_vaddr_frames Andrey Konovalov
@ 2019-06-24 14:32 ` Andrey Konovalov
2019-06-24 17:50 ` Catalin Marinas
2019-06-24 14:32 ` [PATCH v18 08/15] userfaultfd: untag user pointers Andrey Konovalov
` (8 subsequent siblings)
15 siblings, 1 reply; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
In copy_mount_options a user address is being subtracted from TASK_SIZE.
If the address is lower than TASK_SIZE, the size is calculated to not
allow the exact_copy_from_user() call to cross TASK_SIZE boundary.
However if the address is tagged, then the size will be calculated
incorrectly.
Untag the address before subtracting.
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
fs/namespace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index 7660c2749c96..ec78f7223917 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2994,7 +2994,7 @@ void *copy_mount_options(const void __user * data)
* the remainder of the page.
*/
/* copy_from_user cannot cross TASK_SIZE ! */
- size = TASK_SIZE - (unsigned long)data;
+ size = TASK_SIZE - (unsigned long)untagged_addr(data);
if (size > PAGE_SIZE)
size = PAGE_SIZE;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v18 07/15] fs/namespace: untag user pointers in copy_mount_options
2019-06-24 14:32 ` [PATCH v18 07/15] fs/namespace: untag user pointers in copy_mount_options Andrey Konovalov
@ 2019-06-24 17:50 ` Catalin Marinas
2019-07-15 16:00 ` Andrey Konovalov
0 siblings, 1 reply; 47+ messages in thread
From: Catalin Marinas @ 2019-06-24 17:50 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Jason Gunthorpe,
Christoph Hellwig, Dmitry Vyukov, Kostya Serebryany,
Evgeniy Stepanov, Lee Smith, Ramana Radhakrishnan, Jacob Bramley,
Ruben Ayrapetyan, Robin Murphy, Kevin Brodsky, Szabolcs Nagy,
Al Viro
On Mon, Jun 24, 2019 at 04:32:52PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> In copy_mount_options a user address is being subtracted from TASK_SIZE.
> If the address is lower than TASK_SIZE, the size is calculated to not
> allow the exact_copy_from_user() call to cross TASK_SIZE boundary.
> However if the address is tagged, then the size will be calculated
> incorrectly.
>
> Untag the address before subtracting.
>
> Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
> Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
> fs/namespace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 7660c2749c96..ec78f7223917 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2994,7 +2994,7 @@ void *copy_mount_options(const void __user * data)
> * the remainder of the page.
> */
> /* copy_from_user cannot cross TASK_SIZE ! */
> - size = TASK_SIZE - (unsigned long)data;
> + size = TASK_SIZE - (unsigned long)untagged_addr(data);
> if (size > PAGE_SIZE)
> size = PAGE_SIZE;
I think this patch needs an ack from Al Viro (cc'ed).
--
Catalin
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 07/15] fs/namespace: untag user pointers in copy_mount_options
2019-06-24 17:50 ` Catalin Marinas
@ 2019-07-15 16:00 ` Andrey Konovalov
2019-07-22 16:46 ` Kees Cook
0 siblings, 1 reply; 47+ messages in thread
From: Andrey Konovalov @ 2019-07-15 16:00 UTC (permalink / raw)
To: Al Viro
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Jason Gunthorpe,
Christoph Hellwig, Dmitry Vyukov, Kostya Serebryany,
Evgeniy Stepanov, Lee Smith, Ramana Radhakrishnan, Jacob Bramley,
Ruben Ayrapetyan, Robin Murphy, Kevin Brodsky, Szabolcs Nagy,
Catalin Marinas
On Mon, Jun 24, 2019 at 7:50 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Mon, Jun 24, 2019 at 04:32:52PM +0200, Andrey Konovalov wrote:
> > This patch is a part of a series that extends kernel ABI to allow to pass
> > tagged user pointers (with the top byte set to something else other than
> > 0x00) as syscall arguments.
> >
> > In copy_mount_options a user address is being subtracted from TASK_SIZE.
> > If the address is lower than TASK_SIZE, the size is calculated to not
> > allow the exact_copy_from_user() call to cross TASK_SIZE boundary.
> > However if the address is tagged, then the size will be calculated
> > incorrectly.
> >
> > Untag the address before subtracting.
> >
> > Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
> > Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> > Reviewed-by: Kees Cook <keescook@chromium.org>
> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > ---
> > fs/namespace.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/namespace.c b/fs/namespace.c
> > index 7660c2749c96..ec78f7223917 100644
> > --- a/fs/namespace.c
> > +++ b/fs/namespace.c
> > @@ -2994,7 +2994,7 @@ void *copy_mount_options(const void __user * data)
> > * the remainder of the page.
> > */
> > /* copy_from_user cannot cross TASK_SIZE ! */
> > - size = TASK_SIZE - (unsigned long)data;
> > + size = TASK_SIZE - (unsigned long)untagged_addr(data);
> > if (size > PAGE_SIZE)
> > size = PAGE_SIZE;
>
> I think this patch needs an ack from Al Viro (cc'ed).
>
> --
> Catalin
Hi Al,
Could you take a look and give your acked-by?
Thanks!
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 07/15] fs/namespace: untag user pointers in copy_mount_options
2019-07-15 16:00 ` Andrey Konovalov
@ 2019-07-22 16:46 ` Kees Cook
0 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2019-07-22 16:46 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Al Viro, Eric Biederman, Linux ARM, Linux Memory Management List,
LKML, amd-gfx, dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Jason Gunthorpe,
Christoph Hellwig, Dmitry Vyukov, Kostya Serebryany,
Evgeniy Stepanov, Lee Smith, Ramana Radhakrishnan, Jacob Bramley,
Ruben Ayrapetyan, Robin Murphy, Kevin Brodsky, Szabolcs Nagy,
Catalin Marinas
+Eric Biederman too, who might be able to Ack this...
On Mon, Jul 15, 2019 at 06:00:04PM +0200, Andrey Konovalov wrote:
> On Mon, Jun 24, 2019 at 7:50 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> >
> > On Mon, Jun 24, 2019 at 04:32:52PM +0200, Andrey Konovalov wrote:
> > > This patch is a part of a series that extends kernel ABI to allow to pass
> > > tagged user pointers (with the top byte set to something else other than
> > > 0x00) as syscall arguments.
> > >
> > > In copy_mount_options a user address is being subtracted from TASK_SIZE.
> > > If the address is lower than TASK_SIZE, the size is calculated to not
> > > allow the exact_copy_from_user() call to cross TASK_SIZE boundary.
> > > However if the address is tagged, then the size will be calculated
> > > incorrectly.
> > >
> > > Untag the address before subtracting.
> > >
> > > Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
> > > Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> > > Reviewed-by: Kees Cook <keescook@chromium.org>
> > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > > ---
> > > fs/namespace.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/fs/namespace.c b/fs/namespace.c
> > > index 7660c2749c96..ec78f7223917 100644
> > > --- a/fs/namespace.c
> > > +++ b/fs/namespace.c
> > > @@ -2994,7 +2994,7 @@ void *copy_mount_options(const void __user * data)
> > > * the remainder of the page.
> > > */
> > > /* copy_from_user cannot cross TASK_SIZE ! */
> > > - size = TASK_SIZE - (unsigned long)data;
> > > + size = TASK_SIZE - (unsigned long)untagged_addr(data);
> > > if (size > PAGE_SIZE)
> > > size = PAGE_SIZE;
> >
> > I think this patch needs an ack from Al Viro (cc'ed).
> >
> > --
> > Catalin
>
> Hi Al,
>
> Could you take a look and give your acked-by?
>
> Thanks!
--
Kees Cook
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v18 08/15] userfaultfd: untag user pointers
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
` (6 preceding siblings ...)
2019-06-24 14:32 ` [PATCH v18 07/15] fs/namespace: untag user pointers in copy_mount_options Andrey Konovalov
@ 2019-06-24 14:32 ` Andrey Konovalov
2019-06-24 17:51 ` Catalin Marinas
2019-06-24 14:32 ` [PATCH v18 09/15] drm/amdgpu: " Andrey Konovalov
` (7 subsequent siblings)
15 siblings, 1 reply; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
userfaultfd code use provided user pointers for vma lookups, which can
only by done with untagged pointers.
Untag user pointers in validate_range().
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
fs/userfaultfd.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index ae0b8b5f69e6..c2be36a168ca 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1261,21 +1261,23 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx,
}
static __always_inline int validate_range(struct mm_struct *mm,
- __u64 start, __u64 len)
+ __u64 *start, __u64 len)
{
__u64 task_size = mm->task_size;
- if (start & ~PAGE_MASK)
+ *start = untagged_addr(*start);
+
+ if (*start & ~PAGE_MASK)
return -EINVAL;
if (len & ~PAGE_MASK)
return -EINVAL;
if (!len)
return -EINVAL;
- if (start < mmap_min_addr)
+ if (*start < mmap_min_addr)
return -EINVAL;
- if (start >= task_size)
+ if (*start >= task_size)
return -EINVAL;
- if (len > task_size - start)
+ if (len > task_size - *start)
return -EINVAL;
return 0;
}
@@ -1325,7 +1327,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
goto out;
}
- ret = validate_range(mm, uffdio_register.range.start,
+ ret = validate_range(mm, &uffdio_register.range.start,
uffdio_register.range.len);
if (ret)
goto out;
@@ -1514,7 +1516,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister)))
goto out;
- ret = validate_range(mm, uffdio_unregister.start,
+ ret = validate_range(mm, &uffdio_unregister.start,
uffdio_unregister.len);
if (ret)
goto out;
@@ -1665,7 +1667,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake)))
goto out;
- ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len);
+ ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len);
if (ret)
goto out;
@@ -1705,7 +1707,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
sizeof(uffdio_copy)-sizeof(__s64)))
goto out;
- ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len);
+ ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len);
if (ret)
goto out;
/*
@@ -1761,7 +1763,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
sizeof(uffdio_zeropage)-sizeof(__s64)))
goto out;
- ret = validate_range(ctx->mm, uffdio_zeropage.range.start,
+ ret = validate_range(ctx->mm, &uffdio_zeropage.range.start,
uffdio_zeropage.range.len);
if (ret)
goto out;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v18 08/15] userfaultfd: untag user pointers
2019-06-24 14:32 ` [PATCH v18 08/15] userfaultfd: untag user pointers Andrey Konovalov
@ 2019-06-24 17:51 ` Catalin Marinas
2019-07-15 16:00 ` Andrey Konovalov
2019-07-17 11:09 ` Mike Rapoport
0 siblings, 2 replies; 47+ messages in thread
From: Catalin Marinas @ 2019-06-24 17:51 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Jason Gunthorpe,
Christoph Hellwig, Dmitry Vyukov, Kostya Serebryany,
Evgeniy Stepanov, Lee Smith, Ramana Radhakrishnan, Jacob Bramley,
Ruben Ayrapetyan, Robin Murphy, Kevin Brodsky, Szabolcs Nagy,
Al Viro
On Mon, Jun 24, 2019 at 04:32:53PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> userfaultfd code use provided user pointers for vma lookups, which can
> only by done with untagged pointers.
>
> Untag user pointers in validate_range().
>
> Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
> fs/userfaultfd.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
Same here, it needs an ack from Al Viro.
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index ae0b8b5f69e6..c2be36a168ca 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -1261,21 +1261,23 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx,
> }
>
> static __always_inline int validate_range(struct mm_struct *mm,
> - __u64 start, __u64 len)
> + __u64 *start, __u64 len)
> {
> __u64 task_size = mm->task_size;
>
> - if (start & ~PAGE_MASK)
> + *start = untagged_addr(*start);
> +
> + if (*start & ~PAGE_MASK)
> return -EINVAL;
> if (len & ~PAGE_MASK)
> return -EINVAL;
> if (!len)
> return -EINVAL;
> - if (start < mmap_min_addr)
> + if (*start < mmap_min_addr)
> return -EINVAL;
> - if (start >= task_size)
> + if (*start >= task_size)
> return -EINVAL;
> - if (len > task_size - start)
> + if (len > task_size - *start)
> return -EINVAL;
> return 0;
> }
> @@ -1325,7 +1327,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
> goto out;
> }
>
> - ret = validate_range(mm, uffdio_register.range.start,
> + ret = validate_range(mm, &uffdio_register.range.start,
> uffdio_register.range.len);
> if (ret)
> goto out;
> @@ -1514,7 +1516,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
> if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister)))
> goto out;
>
> - ret = validate_range(mm, uffdio_unregister.start,
> + ret = validate_range(mm, &uffdio_unregister.start,
> uffdio_unregister.len);
> if (ret)
> goto out;
> @@ -1665,7 +1667,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
> if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake)))
> goto out;
>
> - ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len);
> + ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len);
> if (ret)
> goto out;
>
> @@ -1705,7 +1707,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
> sizeof(uffdio_copy)-sizeof(__s64)))
> goto out;
>
> - ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len);
> + ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len);
> if (ret)
> goto out;
> /*
> @@ -1761,7 +1763,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
> sizeof(uffdio_zeropage)-sizeof(__s64)))
> goto out;
>
> - ret = validate_range(ctx->mm, uffdio_zeropage.range.start,
> + ret = validate_range(ctx->mm, &uffdio_zeropage.range.start,
> uffdio_zeropage.range.len);
> if (ret)
> goto out;
> --
> 2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 08/15] userfaultfd: untag user pointers
2019-06-24 17:51 ` Catalin Marinas
@ 2019-07-15 16:00 ` Andrey Konovalov
2019-07-17 11:09 ` Mike Rapoport
1 sibling, 0 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-07-15 16:00 UTC (permalink / raw)
To: Catalin Marinas
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Jason Gunthorpe,
Christoph Hellwig, Dmitry Vyukov, Kostya Serebryany,
Evgeniy Stepanov, Lee Smith, Ramana Radhakrishnan, Jacob Bramley,
Ruben Ayrapetyan, Robin Murphy, Kevin Brodsky, Szabolcs Nagy,
Al Viro
On Mon, Jun 24, 2019 at 7:51 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Mon, Jun 24, 2019 at 04:32:53PM +0200, Andrey Konovalov wrote:
> > This patch is a part of a series that extends kernel ABI to allow to pass
> > tagged user pointers (with the top byte set to something else other than
> > 0x00) as syscall arguments.
> >
> > userfaultfd code use provided user pointers for vma lookups, which can
> > only by done with untagged pointers.
> >
> > Untag user pointers in validate_range().
> >
> > Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> > Reviewed-by: Kees Cook <keescook@chromium.org>
> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > ---
> > fs/userfaultfd.c | 22 ++++++++++++----------
> > 1 file changed, 12 insertions(+), 10 deletions(-)
>
> Same here, it needs an ack from Al Viro.
Hi Al,
Could you take a look at this one as well and give your acked-by?
Thanks!
>
> > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> > index ae0b8b5f69e6..c2be36a168ca 100644
> > --- a/fs/userfaultfd.c
> > +++ b/fs/userfaultfd.c
> > @@ -1261,21 +1261,23 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx,
> > }
> >
> > static __always_inline int validate_range(struct mm_struct *mm,
> > - __u64 start, __u64 len)
> > + __u64 *start, __u64 len)
> > {
> > __u64 task_size = mm->task_size;
> >
> > - if (start & ~PAGE_MASK)
> > + *start = untagged_addr(*start);
> > +
> > + if (*start & ~PAGE_MASK)
> > return -EINVAL;
> > if (len & ~PAGE_MASK)
> > return -EINVAL;
> > if (!len)
> > return -EINVAL;
> > - if (start < mmap_min_addr)
> > + if (*start < mmap_min_addr)
> > return -EINVAL;
> > - if (start >= task_size)
> > + if (*start >= task_size)
> > return -EINVAL;
> > - if (len > task_size - start)
> > + if (len > task_size - *start)
> > return -EINVAL;
> > return 0;
> > }
> > @@ -1325,7 +1327,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
> > goto out;
> > }
> >
> > - ret = validate_range(mm, uffdio_register.range.start,
> > + ret = validate_range(mm, &uffdio_register.range.start,
> > uffdio_register.range.len);
> > if (ret)
> > goto out;
> > @@ -1514,7 +1516,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
> > if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister)))
> > goto out;
> >
> > - ret = validate_range(mm, uffdio_unregister.start,
> > + ret = validate_range(mm, &uffdio_unregister.start,
> > uffdio_unregister.len);
> > if (ret)
> > goto out;
> > @@ -1665,7 +1667,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
> > if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake)))
> > goto out;
> >
> > - ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len);
> > + ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len);
> > if (ret)
> > goto out;
> >
> > @@ -1705,7 +1707,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
> > sizeof(uffdio_copy)-sizeof(__s64)))
> > goto out;
> >
> > - ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len);
> > + ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len);
> > if (ret)
> > goto out;
> > /*
> > @@ -1761,7 +1763,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
> > sizeof(uffdio_zeropage)-sizeof(__s64)))
> > goto out;
> >
> > - ret = validate_range(ctx->mm, uffdio_zeropage.range.start,
> > + ret = validate_range(ctx->mm, &uffdio_zeropage.range.start,
> > uffdio_zeropage.range.len);
> > if (ret)
> > goto out;
> > --
> > 2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 08/15] userfaultfd: untag user pointers
2019-06-24 17:51 ` Catalin Marinas
2019-07-15 16:00 ` Andrey Konovalov
@ 2019-07-17 11:09 ` Mike Rapoport
2019-07-17 11:46 ` Andrey Konovalov
1 sibling, 1 reply; 47+ messages in thread
From: Mike Rapoport @ 2019-07-17 11:09 UTC (permalink / raw)
To: Catalin Marinas
Cc: Andrey Konovalov, linux-arm-kernel, linux-mm, linux-kernel,
amd-gfx, dri-devel, linux-rdma, linux-media, kvm,
linux-kselftest, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Al Viro
On Mon, Jun 24, 2019 at 06:51:21PM +0100, Catalin Marinas wrote:
> On Mon, Jun 24, 2019 at 04:32:53PM +0200, Andrey Konovalov wrote:
> > This patch is a part of a series that extends kernel ABI to allow to pass
> > tagged user pointers (with the top byte set to something else other than
> > 0x00) as syscall arguments.
> >
> > userfaultfd code use provided user pointers for vma lookups, which can
> > only by done with untagged pointers.
> >
> > Untag user pointers in validate_range().
> >
> > Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> > Reviewed-by: Kees Cook <keescook@chromium.org>
> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > ---
> > fs/userfaultfd.c | 22 ++++++++++++----------
> > 1 file changed, 12 insertions(+), 10 deletions(-)
>
> Same here, it needs an ack from Al Viro.
The userfault patches usually go via -mm tree, not sure if Al looks at them :)
FWIW, you can add
Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>
> > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> > index ae0b8b5f69e6..c2be36a168ca 100644
> > --- a/fs/userfaultfd.c
> > +++ b/fs/userfaultfd.c
> > @@ -1261,21 +1261,23 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx,
> > }
> >
> > static __always_inline int validate_range(struct mm_struct *mm,
> > - __u64 start, __u64 len)
> > + __u64 *start, __u64 len)
> > {
> > __u64 task_size = mm->task_size;
> >
> > - if (start & ~PAGE_MASK)
> > + *start = untagged_addr(*start);
> > +
> > + if (*start & ~PAGE_MASK)
> > return -EINVAL;
> > if (len & ~PAGE_MASK)
> > return -EINVAL;
> > if (!len)
> > return -EINVAL;
> > - if (start < mmap_min_addr)
> > + if (*start < mmap_min_addr)
> > return -EINVAL;
> > - if (start >= task_size)
> > + if (*start >= task_size)
> > return -EINVAL;
> > - if (len > task_size - start)
> > + if (len > task_size - *start)
> > return -EINVAL;
> > return 0;
> > }
> > @@ -1325,7 +1327,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
> > goto out;
> > }
> >
> > - ret = validate_range(mm, uffdio_register.range.start,
> > + ret = validate_range(mm, &uffdio_register.range.start,
> > uffdio_register.range.len);
> > if (ret)
> > goto out;
> > @@ -1514,7 +1516,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
> > if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister)))
> > goto out;
> >
> > - ret = validate_range(mm, uffdio_unregister.start,
> > + ret = validate_range(mm, &uffdio_unregister.start,
> > uffdio_unregister.len);
> > if (ret)
> > goto out;
> > @@ -1665,7 +1667,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
> > if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake)))
> > goto out;
> >
> > - ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len);
> > + ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len);
> > if (ret)
> > goto out;
> >
> > @@ -1705,7 +1707,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
> > sizeof(uffdio_copy)-sizeof(__s64)))
> > goto out;
> >
> > - ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len);
> > + ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len);
> > if (ret)
> > goto out;
> > /*
> > @@ -1761,7 +1763,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
> > sizeof(uffdio_zeropage)-sizeof(__s64)))
> > goto out;
> >
> > - ret = validate_range(ctx->mm, uffdio_zeropage.range.start,
> > + ret = validate_range(ctx->mm, &uffdio_zeropage.range.start,
> > uffdio_zeropage.range.len);
> > if (ret)
> > goto out;
> > --
> > 2.22.0.410.gd8fdbe21b5-goog
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 08/15] userfaultfd: untag user pointers
2019-07-17 11:09 ` Mike Rapoport
@ 2019-07-17 11:46 ` Andrey Konovalov
0 siblings, 0 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-07-17 11:46 UTC (permalink / raw)
To: Mike Rapoport
Cc: Catalin Marinas, Linux ARM, Linux Memory Management List, LKML,
amd-gfx, dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Jason Gunthorpe,
Christoph Hellwig, Dmitry Vyukov, Kostya Serebryany,
Evgeniy Stepanov, Lee Smith, Ramana Radhakrishnan, Jacob Bramley,
Ruben Ayrapetyan, Robin Murphy, Kevin Brodsky, Szabolcs Nagy,
Al Viro
On Wed, Jul 17, 2019 at 1:09 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> On Mon, Jun 24, 2019 at 06:51:21PM +0100, Catalin Marinas wrote:
> > On Mon, Jun 24, 2019 at 04:32:53PM +0200, Andrey Konovalov wrote:
> > > This patch is a part of a series that extends kernel ABI to allow to pass
> > > tagged user pointers (with the top byte set to something else other than
> > > 0x00) as syscall arguments.
> > >
> > > userfaultfd code use provided user pointers for vma lookups, which can
> > > only by done with untagged pointers.
> > >
> > > Untag user pointers in validate_range().
> > >
> > > Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> > > Reviewed-by: Kees Cook <keescook@chromium.org>
> > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > > ---
> > > fs/userfaultfd.c | 22 ++++++++++++----------
> > > 1 file changed, 12 insertions(+), 10 deletions(-)
> >
> > Same here, it needs an ack from Al Viro.
>
> The userfault patches usually go via -mm tree, not sure if Al looks at them :)
Ah, OK, I guess than Andrew will take a look at them when merging.
>
> FWIW, you can add
>
> Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>
I will, thanks!
>
> > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> > > index ae0b8b5f69e6..c2be36a168ca 100644
> > > --- a/fs/userfaultfd.c
> > > +++ b/fs/userfaultfd.c
> > > @@ -1261,21 +1261,23 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx,
> > > }
> > >
> > > static __always_inline int validate_range(struct mm_struct *mm,
> > > - __u64 start, __u64 len)
> > > + __u64 *start, __u64 len)
> > > {
> > > __u64 task_size = mm->task_size;
> > >
> > > - if (start & ~PAGE_MASK)
> > > + *start = untagged_addr(*start);
> > > +
> > > + if (*start & ~PAGE_MASK)
> > > return -EINVAL;
> > > if (len & ~PAGE_MASK)
> > > return -EINVAL;
> > > if (!len)
> > > return -EINVAL;
> > > - if (start < mmap_min_addr)
> > > + if (*start < mmap_min_addr)
> > > return -EINVAL;
> > > - if (start >= task_size)
> > > + if (*start >= task_size)
> > > return -EINVAL;
> > > - if (len > task_size - start)
> > > + if (len > task_size - *start)
> > > return -EINVAL;
> > > return 0;
> > > }
> > > @@ -1325,7 +1327,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
> > > goto out;
> > > }
> > >
> > > - ret = validate_range(mm, uffdio_register.range.start,
> > > + ret = validate_range(mm, &uffdio_register.range.start,
> > > uffdio_register.range.len);
> > > if (ret)
> > > goto out;
> > > @@ -1514,7 +1516,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
> > > if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister)))
> > > goto out;
> > >
> > > - ret = validate_range(mm, uffdio_unregister.start,
> > > + ret = validate_range(mm, &uffdio_unregister.start,
> > > uffdio_unregister.len);
> > > if (ret)
> > > goto out;
> > > @@ -1665,7 +1667,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
> > > if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake)))
> > > goto out;
> > >
> > > - ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len);
> > > + ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len);
> > > if (ret)
> > > goto out;
> > >
> > > @@ -1705,7 +1707,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
> > > sizeof(uffdio_copy)-sizeof(__s64)))
> > > goto out;
> > >
> > > - ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len);
> > > + ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len);
> > > if (ret)
> > > goto out;
> > > /*
> > > @@ -1761,7 +1763,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
> > > sizeof(uffdio_zeropage)-sizeof(__s64)))
> > > goto out;
> > >
> > > - ret = validate_range(ctx->mm, uffdio_zeropage.range.start,
> > > + ret = validate_range(ctx->mm, &uffdio_zeropage.range.start,
> > > uffdio_zeropage.range.len);
> > > if (ret)
> > > goto out;
> > > --
> > > 2.22.0.410.gd8fdbe21b5-goog
>
> --
> Sincerely yours,
> Mike.
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v18 09/15] drm/amdgpu: untag user pointers
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
` (7 preceding siblings ...)
2019-06-24 14:32 ` [PATCH v18 08/15] userfaultfd: untag user pointers Andrey Konovalov
@ 2019-06-24 14:32 ` Andrey Konovalov
2019-06-24 15:00 ` Kees Cook
2019-06-24 14:32 ` [PATCH v18 10/15] drm/radeon: untag user pointers in radeon_gem_userptr_ioctl Andrey Konovalov
` (6 subsequent siblings)
15 siblings, 1 reply; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
In amdgpu_gem_userptr_ioctl() and amdgpu_amdkfd_gpuvm.c/init_user_pages()
an MMU notifier is set up with a (tagged) userspace pointer. The untagged
address should be used so that MMU notifiers for the untagged address get
correctly matched up with the right BO. This patch untag user pointers in
amdgpu_gem_userptr_ioctl() for the GEM case and in amdgpu_amdkfd_gpuvm_
alloc_memory_of_gpu() for the KFD case. This also makes sure that an
untagged pointer is passed to amdgpu_ttm_tt_get_user_pages(), which uses
it for vma lookups.
Suggested-by: Felix Kuehling <Felix.Kuehling@amd.com>
Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index a6e5184d436c..5d476e9bbc43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1108,7 +1108,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
alloc_flags = 0;
if (!offset || !*offset)
return -EINVAL;
- user_addr = *offset;
+ user_addr = untagged_addr(*offset);
} else if (flags & ALLOC_MEM_FLAGS_DOORBELL) {
domain = AMDGPU_GEM_DOMAIN_GTT;
alloc_domain = AMDGPU_GEM_DOMAIN_CPU;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index d4fcf5475464..e91df1407618 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -287,6 +287,8 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
uint32_t handle;
int r;
+ args->addr = untagged_addr(args->addr);
+
if (offset_in_page(args->addr | args->size))
return -EINVAL;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v18 09/15] drm/amdgpu: untag user pointers
2019-06-24 14:32 ` [PATCH v18 09/15] drm/amdgpu: " Andrey Konovalov
@ 2019-06-24 15:00 ` Kees Cook
0 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2019-06-24 15:00 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander, Alex Williamson, Leon Romanovsky,
Luc Van Oostenryck, Dave Martin, Khalid Aziz, enh,
Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy
On Mon, Jun 24, 2019 at 04:32:54PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> In amdgpu_gem_userptr_ioctl() and amdgpu_amdkfd_gpuvm.c/init_user_pages()
> an MMU notifier is set up with a (tagged) userspace pointer. The untagged
> address should be used so that MMU notifiers for the untagged address get
> correctly matched up with the right BO. This patch untag user pointers in
> amdgpu_gem_userptr_ioctl() for the GEM case and in amdgpu_amdkfd_gpuvm_
> alloc_memory_of_gpu() for the KFD case. This also makes sure that an
> untagged pointer is passed to amdgpu_ttm_tt_get_user_pages(), which uses
> it for vma lookups.
>
> Suggested-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index a6e5184d436c..5d476e9bbc43 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1108,7 +1108,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> alloc_flags = 0;
> if (!offset || !*offset)
> return -EINVAL;
> - user_addr = *offset;
> + user_addr = untagged_addr(*offset);
> } else if (flags & ALLOC_MEM_FLAGS_DOORBELL) {
> domain = AMDGPU_GEM_DOMAIN_GTT;
> alloc_domain = AMDGPU_GEM_DOMAIN_CPU;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index d4fcf5475464..e91df1407618 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -287,6 +287,8 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
> uint32_t handle;
> int r;
>
> + args->addr = untagged_addr(args->addr);
> +
> if (offset_in_page(args->addr | args->size))
> return -EINVAL;
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v18 10/15] drm/radeon: untag user pointers in radeon_gem_userptr_ioctl
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
` (8 preceding siblings ...)
2019-06-24 14:32 ` [PATCH v18 09/15] drm/amdgpu: " Andrey Konovalov
@ 2019-06-24 14:32 ` Andrey Konovalov
2019-06-24 15:01 ` Kees Cook
` (2 more replies)
2019-06-24 14:32 ` [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr Andrey Konovalov
` (5 subsequent siblings)
15 siblings, 3 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
In radeon_gem_userptr_ioctl() an MMU notifier is set up with a (tagged)
userspace pointer. The untagged address should be used so that MMU
notifiers for the untagged address get correctly matched up with the right
BO. This funcation also calls radeon_ttm_tt_pin_userptr(), which uses
provided user pointers for vma lookups, which can only by done with
untagged pointers.
This patch untags user pointers in radeon_gem_userptr_ioctl().
Suggested-by: Felix Kuehling <Felix.Kuehling@amd.com>
Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
drivers/gpu/drm/radeon/radeon_gem.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index 44617dec8183..90eb78fb5eb2 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -291,6 +291,8 @@ int radeon_gem_userptr_ioctl(struct drm_device *dev, void *data,
uint32_t handle;
int r;
+ args->addr = untagged_addr(args->addr);
+
if (offset_in_page(args->addr | args->size))
return -EINVAL;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v18 10/15] drm/radeon: untag user pointers in radeon_gem_userptr_ioctl
2019-06-24 14:32 ` [PATCH v18 10/15] drm/radeon: untag user pointers in radeon_gem_userptr_ioctl Andrey Konovalov
@ 2019-06-24 15:01 ` Kees Cook
2019-06-24 15:02 ` Kees Cook
2019-06-26 17:50 ` Khalid Aziz
2 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2019-06-24 15:01 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander, Alex Williamson, Leon Romanovsky,
Luc Van Oostenryck, Dave Martin, Khalid Aziz, enh,
Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy
On Mon, Jun 24, 2019 at 04:32:55PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> In radeon_gem_userptr_ioctl() an MMU notifier is set up with a (tagged)
> userspace pointer. The untagged address should be used so that MMU
> notifiers for the untagged address get correctly matched up with the right
> BO. This funcation also calls radeon_ttm_tt_pin_userptr(), which uses
> provided user pointers for vma lookups, which can only by done with
> untagged pointers.
>
> This patch untags user pointers in radeon_gem_userptr_ioctl().
>
> Suggested-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> drivers/gpu/drm/radeon/radeon_gem.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> index 44617dec8183..90eb78fb5eb2 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -291,6 +291,8 @@ int radeon_gem_userptr_ioctl(struct drm_device *dev, void *data,
> uint32_t handle;
> int r;
>
> + args->addr = untagged_addr(args->addr);
> +
> if (offset_in_page(args->addr | args->size))
> return -EINVAL;
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 10/15] drm/radeon: untag user pointers in radeon_gem_userptr_ioctl
2019-06-24 14:32 ` [PATCH v18 10/15] drm/radeon: untag user pointers in radeon_gem_userptr_ioctl Andrey Konovalov
2019-06-24 15:01 ` Kees Cook
@ 2019-06-24 15:02 ` Kees Cook
2019-06-26 17:50 ` Khalid Aziz
2 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2019-06-24 15:02 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander, Alex Williamson, Leon Romanovsky,
Luc Van Oostenryck, Dave Martin, Khalid Aziz, enh,
Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy
On Mon, Jun 24, 2019 at 04:32:55PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> In radeon_gem_userptr_ioctl() an MMU notifier is set up with a (tagged)
> userspace pointer. The untagged address should be used so that MMU
> notifiers for the untagged address get correctly matched up with the right
> BO. This funcation also calls radeon_ttm_tt_pin_userptr(), which uses
> provided user pointers for vma lookups, which can only by done with
> untagged pointers.
>
> This patch untags user pointers in radeon_gem_userptr_ioctl().
>
> Suggested-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> drivers/gpu/drm/radeon/radeon_gem.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> index 44617dec8183..90eb78fb5eb2 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -291,6 +291,8 @@ int radeon_gem_userptr_ioctl(struct drm_device *dev, void *data,
> uint32_t handle;
> int r;
>
> + args->addr = untagged_addr(args->addr);
> +
> if (offset_in_page(args->addr | args->size))
> return -EINVAL;
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 10/15] drm/radeon: untag user pointers in radeon_gem_userptr_ioctl
2019-06-24 14:32 ` [PATCH v18 10/15] drm/radeon: untag user pointers in radeon_gem_userptr_ioctl Andrey Konovalov
2019-06-24 15:01 ` Kees Cook
2019-06-24 15:02 ` Kees Cook
@ 2019-06-26 17:50 ` Khalid Aziz
2 siblings, 0 replies; 47+ messages in thread
From: Khalid Aziz @ 2019-06-26 17:50 UTC (permalink / raw)
To: Andrey Konovalov, linux-arm-kernel, linux-mm, linux-kernel,
amd-gfx, dri-devel, linux-rdma, linux-media, kvm,
linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, enh,
Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy
On 6/24/19 8:32 AM, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> In radeon_gem_userptr_ioctl() an MMU notifier is set up with a (tagged)
> userspace pointer. The untagged address should be used so that MMU
> notifiers for the untagged address get correctly matched up with the right
> BO. This funcation also calls radeon_ttm_tt_pin_userptr(), which uses
> provided user pointers for vma lookups, which can only by done with
> untagged pointers.
>
> This patch untags user pointers in radeon_gem_userptr_ioctl().
>
> Suggested-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
> drivers/gpu/drm/radeon/radeon_gem.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> index 44617dec8183..90eb78fb5eb2 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -291,6 +291,8 @@ int radeon_gem_userptr_ioctl(struct drm_device *dev, void *data,
> uint32_t handle;
> int r;
>
> + args->addr = untagged_addr(args->addr);
> +
> if (offset_in_page(args->addr | args->size))
> return -EINVAL;
>
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
` (9 preceding siblings ...)
2019-06-24 14:32 ` [PATCH v18 10/15] drm/radeon: untag user pointers in radeon_gem_userptr_ioctl Andrey Konovalov
@ 2019-06-24 14:32 ` Andrey Konovalov
2019-06-24 15:01 ` Kees Cook
2019-06-24 17:40 ` Catalin Marinas
2019-06-24 14:32 ` [PATCH v18 12/15] media/v4l2-core: untag user pointers in videobuf_dma_contig_user_get Andrey Konovalov
` (4 subsequent siblings)
15 siblings, 2 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can
only by done with untagged pointers.
Untag user pointers in this function.
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/mr.c b/drivers/infiniband/hw/mlx4/mr.c
index 355205a28544..13d9f917f249 100644
--- a/drivers/infiniband/hw/mlx4/mr.c
+++ b/drivers/infiniband/hw/mlx4/mr.c
@@ -378,6 +378,7 @@ static struct ib_umem *mlx4_get_umem_mr(struct ib_udata *udata, u64 start,
* again
*/
if (!ib_access_writable(access_flags)) {
+ unsigned long untagged_start = untagged_addr(start);
struct vm_area_struct *vma;
down_read(¤t->mm->mmap_sem);
@@ -386,9 +387,9 @@ static struct ib_umem *mlx4_get_umem_mr(struct ib_udata *udata, u64 start,
* cover the memory, but for now it requires a single vma to
* entirely cover the MR to support RO mappings.
*/
- vma = find_vma(current->mm, start);
- if (vma && vma->vm_end >= start + length &&
- vma->vm_start <= start) {
+ vma = find_vma(current->mm, untagged_start);
+ if (vma && vma->vm_end >= untagged_start + length &&
+ vma->vm_start <= untagged_start) {
if (vma->vm_flags & VM_WRITE)
access_flags |= IB_ACCESS_LOCAL_WRITE;
} else {
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
2019-06-24 14:32 ` [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr Andrey Konovalov
@ 2019-06-24 15:01 ` Kees Cook
2019-06-24 17:40 ` Catalin Marinas
1 sibling, 0 replies; 47+ messages in thread
From: Kees Cook @ 2019-06-24 15:01 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander, Alex Williamson, Leon Romanovsky,
Luc Van Oostenryck, Dave Martin, Khalid Aziz, enh,
Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy
On Mon, Jun 24, 2019 at 04:32:56PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can
> only by done with untagged pointers.
>
> Untag user pointers in this function.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx4/mr.c b/drivers/infiniband/hw/mlx4/mr.c
> index 355205a28544..13d9f917f249 100644
> --- a/drivers/infiniband/hw/mlx4/mr.c
> +++ b/drivers/infiniband/hw/mlx4/mr.c
> @@ -378,6 +378,7 @@ static struct ib_umem *mlx4_get_umem_mr(struct ib_udata *udata, u64 start,
> * again
> */
> if (!ib_access_writable(access_flags)) {
> + unsigned long untagged_start = untagged_addr(start);
> struct vm_area_struct *vma;
>
> down_read(¤t->mm->mmap_sem);
> @@ -386,9 +387,9 @@ static struct ib_umem *mlx4_get_umem_mr(struct ib_udata *udata, u64 start,
> * cover the memory, but for now it requires a single vma to
> * entirely cover the MR to support RO mappings.
> */
> - vma = find_vma(current->mm, start);
> - if (vma && vma->vm_end >= start + length &&
> - vma->vm_start <= start) {
> + vma = find_vma(current->mm, untagged_start);
> + if (vma && vma->vm_end >= untagged_start + length &&
> + vma->vm_start <= untagged_start) {
> if (vma->vm_flags & VM_WRITE)
> access_flags |= IB_ACCESS_LOCAL_WRITE;
> } else {
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
2019-06-24 14:32 ` [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr Andrey Konovalov
2019-06-24 15:01 ` Kees Cook
@ 2019-06-24 17:40 ` Catalin Marinas
2019-07-15 16:01 ` Andrey Konovalov
1 sibling, 1 reply; 47+ messages in thread
From: Catalin Marinas @ 2019-06-24 17:40 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Jason Gunthorpe,
Christoph Hellwig, Dmitry Vyukov, Kostya Serebryany,
Evgeniy Stepanov, Lee Smith, Ramana Radhakrishnan, Jacob Bramley,
Ruben Ayrapetyan, Robin Murphy, Kevin Brodsky, Szabolcs Nagy
On Mon, Jun 24, 2019 at 04:32:56PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can
> only by done with untagged pointers.
>
> Untag user pointers in this function.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
> drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
This patch also needs an ack from the infiniband maintainers (Jason).
--
Catalin
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
2019-06-24 17:40 ` Catalin Marinas
@ 2019-07-15 16:01 ` Andrey Konovalov
2019-07-15 18:05 ` Jason Gunthorpe
0 siblings, 1 reply; 47+ messages in thread
From: Andrey Konovalov @ 2019-07-15 16:01 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Catalin Marinas
On Mon, Jun 24, 2019 at 7:40 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Mon, Jun 24, 2019 at 04:32:56PM +0200, Andrey Konovalov wrote:
> > This patch is a part of a series that extends kernel ABI to allow to pass
> > tagged user pointers (with the top byte set to something else other than
> > 0x00) as syscall arguments.
> >
> > mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can
> > only by done with untagged pointers.
> >
> > Untag user pointers in this function.
> >
> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > ---
> > drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>
> This patch also needs an ack from the infiniband maintainers (Jason).
Hi Jason,
Could you take a look and give your acked-by?
Thanks!
>
> --
> Catalin
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
2019-07-15 16:01 ` Andrey Konovalov
@ 2019-07-15 18:05 ` Jason Gunthorpe
2019-07-16 10:42 ` Andrey Konovalov
0 siblings, 1 reply; 47+ messages in thread
From: Jason Gunthorpe @ 2019-07-15 18:05 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Catalin Marinas
On Mon, Jul 15, 2019 at 06:01:29PM +0200, Andrey Konovalov wrote:
> On Mon, Jun 24, 2019 at 7:40 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> >
> > On Mon, Jun 24, 2019 at 04:32:56PM +0200, Andrey Konovalov wrote:
> > > This patch is a part of a series that extends kernel ABI to allow to pass
> > > tagged user pointers (with the top byte set to something else other than
> > > 0x00) as syscall arguments.
> > >
> > > mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can
> > > only by done with untagged pointers.
> > >
> > > Untag user pointers in this function.
> > >
> > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > > drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
> > > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> >
> > This patch also needs an ack from the infiniband maintainers (Jason).
>
> Hi Jason,
>
> Could you take a look and give your acked-by?
Oh, I think I did this a long time ago. Still looks OK. You will send
it?
Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>
Jason
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
2019-07-15 18:05 ` Jason Gunthorpe
@ 2019-07-16 10:42 ` Andrey Konovalov
2019-07-16 12:06 ` Jason Gunthorpe
0 siblings, 1 reply; 47+ messages in thread
From: Andrey Konovalov @ 2019-07-16 10:42 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Catalin Marinas
On Mon, Jul 15, 2019 at 8:05 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Mon, Jul 15, 2019 at 06:01:29PM +0200, Andrey Konovalov wrote:
> > On Mon, Jun 24, 2019 at 7:40 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > >
> > > On Mon, Jun 24, 2019 at 04:32:56PM +0200, Andrey Konovalov wrote:
> > > > This patch is a part of a series that extends kernel ABI to allow to pass
> > > > tagged user pointers (with the top byte set to something else other than
> > > > 0x00) as syscall arguments.
> > > >
> > > > mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can
> > > > only by done with untagged pointers.
> > > >
> > > > Untag user pointers in this function.
> > > >
> > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > > > drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
> > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > >
> > > This patch also needs an ack from the infiniband maintainers (Jason).
> >
> > Hi Jason,
> >
> > Could you take a look and give your acked-by?
>
> Oh, I think I did this a long time ago. Still looks OK.
Hm, maybe that was we who lost it. Thanks!
> You will send it?
I will resend the patchset once the merge window is closed, if that's
what you mean.
>
> Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>
>
> Jason
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
2019-07-16 10:42 ` Andrey Konovalov
@ 2019-07-16 12:06 ` Jason Gunthorpe
2019-07-17 11:42 ` Andrey Konovalov
2019-07-17 11:44 ` Andrey Konovalov
0 siblings, 2 replies; 47+ messages in thread
From: Jason Gunthorpe @ 2019-07-16 12:06 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Catalin Marinas
On Tue, Jul 16, 2019 at 12:42:07PM +0200, Andrey Konovalov wrote:
> On Mon, Jul 15, 2019 at 8:05 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > On Mon, Jul 15, 2019 at 06:01:29PM +0200, Andrey Konovalov wrote:
> > > On Mon, Jun 24, 2019 at 7:40 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > >
> > > > On Mon, Jun 24, 2019 at 04:32:56PM +0200, Andrey Konovalov wrote:
> > > > > This patch is a part of a series that extends kernel ABI to allow to pass
> > > > > tagged user pointers (with the top byte set to something else other than
> > > > > 0x00) as syscall arguments.
> > > > >
> > > > > mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can
> > > > > only by done with untagged pointers.
> > > > >
> > > > > Untag user pointers in this function.
> > > > >
> > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > > > > drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
> > > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > >
> > > > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > > >
> > > > This patch also needs an ack from the infiniband maintainers (Jason).
> > >
> > > Hi Jason,
> > >
> > > Could you take a look and give your acked-by?
> >
> > Oh, I think I did this a long time ago. Still looks OK.
>
> Hm, maybe that was we who lost it. Thanks!
>
> > You will send it?
>
> I will resend the patchset once the merge window is closed, if that's
> what you mean.
No.. I mean who send it to Linus's tree? ie do you want me to take
this patch into rdma?
Jason
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
2019-07-16 12:06 ` Jason Gunthorpe
@ 2019-07-17 11:42 ` Andrey Konovalov
2019-07-17 11:44 ` Andrey Konovalov
1 sibling, 0 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-07-17 11:42 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Catalin Marinas
On Tue, Jul 16, 2019 at 2:06 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Tue, Jul 16, 2019 at 12:42:07PM +0200, Andrey Konovalov wrote:
> > On Mon, Jul 15, 2019 at 8:05 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > >
> > > On Mon, Jul 15, 2019 at 06:01:29PM +0200, Andrey Konovalov wrote:
> > > > On Mon, Jun 24, 2019 at 7:40 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > > >
> > > > > On Mon, Jun 24, 2019 at 04:32:56PM +0200, Andrey Konovalov wrote:
> > > > > > This patch is a part of a series that extends kernel ABI to allow to pass
> > > > > > tagged user pointers (with the top byte set to something else other than
> > > > > > 0x00) as syscall arguments.
> > > > > >
> > > > > > mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can
> > > > > > only by done with untagged pointers.
> > > > > >
> > > > > > Untag user pointers in this function.
> > > > > >
> > > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > > > > > drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
> > > > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > > >
> > > > > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > > > >
> > > > > This patch also needs an ack from the infiniband maintainers (Jason).
> > > >
> > > > Hi Jason,
> > > >
> > > > Could you take a look and give your acked-by?
> > >
> > > Oh, I think I did this a long time ago. Still looks OK.
> >
> > Hm, maybe that was we who lost it. Thanks!
> >
> > > You will send it?
> >
> > I will resend the patchset once the merge window is closed, if that's
> > what you mean.
>
> No.. I mean who send it to Linus's tree? ie do you want me to take
> this patch into rdma?
>
> Jason
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
2019-07-16 12:06 ` Jason Gunthorpe
2019-07-17 11:42 ` Andrey Konovalov
@ 2019-07-17 11:44 ` Andrey Konovalov
2019-07-17 11:58 ` Jason Gunthorpe
1 sibling, 1 reply; 47+ messages in thread
From: Andrey Konovalov @ 2019-07-17 11:44 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Catalin Marinas
On Tue, Jul 16, 2019 at 2:06 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Tue, Jul 16, 2019 at 12:42:07PM +0200, Andrey Konovalov wrote:
> > On Mon, Jul 15, 2019 at 8:05 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > >
> > > On Mon, Jul 15, 2019 at 06:01:29PM +0200, Andrey Konovalov wrote:
> > > > On Mon, Jun 24, 2019 at 7:40 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > > >
> > > > > On Mon, Jun 24, 2019 at 04:32:56PM +0200, Andrey Konovalov wrote:
> > > > > > This patch is a part of a series that extends kernel ABI to allow to pass
> > > > > > tagged user pointers (with the top byte set to something else other than
> > > > > > 0x00) as syscall arguments.
> > > > > >
> > > > > > mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can
> > > > > > only by done with untagged pointers.
> > > > > >
> > > > > > Untag user pointers in this function.
> > > > > >
> > > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > > > > > drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
> > > > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > > >
> > > > > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > > > >
> > > > > This patch also needs an ack from the infiniband maintainers (Jason).
> > > >
> > > > Hi Jason,
> > > >
> > > > Could you take a look and give your acked-by?
> > >
> > > Oh, I think I did this a long time ago. Still looks OK.
> >
> > Hm, maybe that was we who lost it. Thanks!
> >
> > > You will send it?
> >
> > I will resend the patchset once the merge window is closed, if that's
> > what you mean.
>
> No.. I mean who send it to Linus's tree? ie do you want me to take
> this patch into rdma?
I think the plan was to merge the whole series through the mm tree.
But I don't mind if you want to take this patch into your tree. It's
just that this patch doesn't make much sense without the rest of the
series.
>
> Jason
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
2019-07-17 11:44 ` Andrey Konovalov
@ 2019-07-17 11:58 ` Jason Gunthorpe
2019-07-17 13:36 ` Andrey Konovalov
0 siblings, 1 reply; 47+ messages in thread
From: Jason Gunthorpe @ 2019-07-17 11:58 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Catalin Marinas
On Wed, Jul 17, 2019 at 01:44:07PM +0200, Andrey Konovalov wrote:
> On Tue, Jul 16, 2019 at 2:06 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > On Tue, Jul 16, 2019 at 12:42:07PM +0200, Andrey Konovalov wrote:
> > > On Mon, Jul 15, 2019 at 8:05 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > >
> > > > On Mon, Jul 15, 2019 at 06:01:29PM +0200, Andrey Konovalov wrote:
> > > > > On Mon, Jun 24, 2019 at 7:40 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > > > >
> > > > > > On Mon, Jun 24, 2019 at 04:32:56PM +0200, Andrey Konovalov wrote:
> > > > > > > This patch is a part of a series that extends kernel ABI to allow to pass
> > > > > > > tagged user pointers (with the top byte set to something else other than
> > > > > > > 0x00) as syscall arguments.
> > > > > > >
> > > > > > > mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can
> > > > > > > only by done with untagged pointers.
> > > > > > >
> > > > > > > Untag user pointers in this function.
> > > > > > >
> > > > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > > > > > > drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
> > > > > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > > > > >
> > > > > > This patch also needs an ack from the infiniband maintainers (Jason).
> > > > >
> > > > > Hi Jason,
> > > > >
> > > > > Could you take a look and give your acked-by?
> > > >
> > > > Oh, I think I did this a long time ago. Still looks OK.
> > >
> > > Hm, maybe that was we who lost it. Thanks!
> > >
> > > > You will send it?
> > >
> > > I will resend the patchset once the merge window is closed, if that's
> > > what you mean.
> >
> > No.. I mean who send it to Linus's tree? ie do you want me to take
> > this patch into rdma?
>
> I think the plan was to merge the whole series through the mm tree.
> But I don't mind if you want to take this patch into your tree. It's
> just that this patch doesn't make much sense without the rest of the
> series.
Generally I prefer if subsystem changes stay in subsystem trees. If
the patch is good standalone, and the untag API has already been
merged, this is a better strategy.
Jason
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
2019-07-17 11:58 ` Jason Gunthorpe
@ 2019-07-17 13:36 ` Andrey Konovalov
0 siblings, 0 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-07-17 13:36 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Catalin Marinas
On Wed, Jul 17, 2019 at 1:58 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Wed, Jul 17, 2019 at 01:44:07PM +0200, Andrey Konovalov wrote:
> > On Tue, Jul 16, 2019 at 2:06 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > >
> > > On Tue, Jul 16, 2019 at 12:42:07PM +0200, Andrey Konovalov wrote:
> > > > On Mon, Jul 15, 2019 at 8:05 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > > >
> > > > > On Mon, Jul 15, 2019 at 06:01:29PM +0200, Andrey Konovalov wrote:
> > > > > > On Mon, Jun 24, 2019 at 7:40 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > > > > >
> > > > > > > On Mon, Jun 24, 2019 at 04:32:56PM +0200, Andrey Konovalov wrote:
> > > > > > > > This patch is a part of a series that extends kernel ABI to allow to pass
> > > > > > > > tagged user pointers (with the top byte set to something else other than
> > > > > > > > 0x00) as syscall arguments.
> > > > > > > >
> > > > > > > > mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can
> > > > > > > > only by done with untagged pointers.
> > > > > > > >
> > > > > > > > Untag user pointers in this function.
> > > > > > > >
> > > > > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > > > > > > > drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
> > > > > > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > > > > >
> > > > > > > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > > > > > >
> > > > > > > This patch also needs an ack from the infiniband maintainers (Jason).
> > > > > >
> > > > > > Hi Jason,
> > > > > >
> > > > > > Could you take a look and give your acked-by?
> > > > >
> > > > > Oh, I think I did this a long time ago. Still looks OK.
> > > >
> > > > Hm, maybe that was we who lost it. Thanks!
> > > >
> > > > > You will send it?
> > > >
> > > > I will resend the patchset once the merge window is closed, if that's
> > > > what you mean.
> > >
> > > No.. I mean who send it to Linus's tree? ie do you want me to take
> > > this patch into rdma?
> >
> > I think the plan was to merge the whole series through the mm tree.
> > But I don't mind if you want to take this patch into your tree. It's
> > just that this patch doesn't make much sense without the rest of the
> > series.
>
> Generally I prefer if subsystem changes stay in subsystem trees. If
> the patch is good standalone, and the untag API has already been
> merged, this is a better strategy.
OK, feel free to take this into your tree, this works for me.
>
> Jason
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v18 12/15] media/v4l2-core: untag user pointers in videobuf_dma_contig_user_get
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
` (10 preceding siblings ...)
2019-06-24 14:32 ` [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr Andrey Konovalov
@ 2019-06-24 14:32 ` Andrey Konovalov
2019-06-24 14:32 ` [PATCH v18 13/15] tee/shm: untag user pointers in tee_shm_register Andrey Konovalov
` (3 subsequent siblings)
15 siblings, 0 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov,
Mauro Carvalho Chehab
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
videobuf_dma_contig_user_get() uses provided user pointers for vma
lookups, which can only by done with untagged pointers.
Untag the pointers in this function.
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
drivers/media/v4l2-core/videobuf-dma-contig.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf-dma-contig.c b/drivers/media/v4l2-core/videobuf-dma-contig.c
index 0491122b03c4..ec554eff29b9 100644
--- a/drivers/media/v4l2-core/videobuf-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf-dma-contig.c
@@ -157,6 +157,7 @@ static void videobuf_dma_contig_user_put(struct videobuf_dma_contig_memory *mem)
static int videobuf_dma_contig_user_get(struct videobuf_dma_contig_memory *mem,
struct videobuf_buffer *vb)
{
+ unsigned long untagged_baddr = untagged_addr(vb->baddr);
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
unsigned long prev_pfn, this_pfn;
@@ -164,22 +165,22 @@ static int videobuf_dma_contig_user_get(struct videobuf_dma_contig_memory *mem,
unsigned int offset;
int ret;
- offset = vb->baddr & ~PAGE_MASK;
+ offset = untagged_baddr & ~PAGE_MASK;
mem->size = PAGE_ALIGN(vb->size + offset);
ret = -EINVAL;
down_read(&mm->mmap_sem);
- vma = find_vma(mm, vb->baddr);
+ vma = find_vma(mm, untagged_baddr);
if (!vma)
goto out_up;
- if ((vb->baddr + mem->size) > vma->vm_end)
+ if ((untagged_baddr + mem->size) > vma->vm_end)
goto out_up;
pages_done = 0;
prev_pfn = 0; /* kill warning */
- user_address = vb->baddr;
+ user_address = untagged_baddr;
while (pages_done < (mem->size >> PAGE_SHIFT)) {
ret = follow_pfn(vma, user_address, &this_pfn);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v18 13/15] tee/shm: untag user pointers in tee_shm_register
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
` (11 preceding siblings ...)
2019-06-24 14:32 ` [PATCH v18 12/15] media/v4l2-core: untag user pointers in videobuf_dma_contig_user_get Andrey Konovalov
@ 2019-06-24 14:32 ` Andrey Konovalov
2019-06-24 14:32 ` [PATCH v18 14/15] vfio/type1: untag user pointers in vaddr_get_pfn Andrey Konovalov
` (2 subsequent siblings)
15 siblings, 0 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
tee_shm_register()->optee_shm_unregister()->check_mem_type() uses provided
user pointers for vma lookups (via __check_mem_type()), which can only by
done with untagged pointers.
Untag user pointers in this function.
Reviewed-by: Kees Cook <keescook@chromium.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
drivers/tee/tee_shm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 2da026fd12c9..09ddcd06c715 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -254,6 +254,7 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr,
shm->teedev = teedev;
shm->ctx = ctx;
shm->id = -1;
+ addr = untagged_addr(addr);
start = rounddown(addr, PAGE_SIZE);
shm->offset = addr - start;
shm->size = length;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v18 14/15] vfio/type1: untag user pointers in vaddr_get_pfn
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
` (12 preceding siblings ...)
2019-06-24 14:32 ` [PATCH v18 13/15] tee/shm: untag user pointers in tee_shm_register Andrey Konovalov
@ 2019-06-24 14:32 ` Andrey Konovalov
2019-06-24 14:33 ` [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel Andrey Konovalov
2019-06-26 17:18 ` [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Catalin Marinas
15 siblings, 0 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov,
Eric Auger
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
vaddr_get_pfn() uses provided user pointers for vma lookups, which can
only by done with untagged pointers.
Untag user pointers in this function.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
drivers/vfio/vfio_iommu_type1.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index add34adfadc7..7b8283e33d10 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -381,6 +381,8 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned long vaddr,
down_read(&mm->mmap_sem);
+ vaddr = untagged_addr(vaddr);
+
vma = find_vma_intersection(mm, vaddr, vaddr + 1);
if (vma && vma->vm_flags & VM_PFNMAP) {
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
` (13 preceding siblings ...)
2019-06-24 14:32 ` [PATCH v18 14/15] vfio/type1: untag user pointers in vaddr_get_pfn Andrey Konovalov
@ 2019-06-24 14:33 ` Andrey Konovalov
2019-06-24 15:02 ` Kees Cook
` (2 more replies)
2019-06-26 17:18 ` [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Catalin Marinas
15 siblings, 3 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-06-24 14:33 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy, Andrey Konovalov
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
This patch adds a simple test, that calls the uname syscall with a
tagged user pointer as an argument. Without the kernel accepting tagged
user pointers the test fails with EFAULT.
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
tools/testing/selftests/arm64/.gitignore | 1 +
tools/testing/selftests/arm64/Makefile | 11 +++++++
.../testing/selftests/arm64/run_tags_test.sh | 12 ++++++++
tools/testing/selftests/arm64/tags_test.c | 29 +++++++++++++++++++
4 files changed, 53 insertions(+)
create mode 100644 tools/testing/selftests/arm64/.gitignore
create mode 100644 tools/testing/selftests/arm64/Makefile
create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh
create mode 100644 tools/testing/selftests/arm64/tags_test.c
diff --git a/tools/testing/selftests/arm64/.gitignore b/tools/testing/selftests/arm64/.gitignore
new file mode 100644
index 000000000000..e8fae8d61ed6
--- /dev/null
+++ b/tools/testing/selftests/arm64/.gitignore
@@ -0,0 +1 @@
+tags_test
diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
new file mode 100644
index 000000000000..a61b2e743e99
--- /dev/null
+++ b/tools/testing/selftests/arm64/Makefile
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0
+
+# ARCH can be overridden by the user for cross compiling
+ARCH ?= $(shell uname -m 2>/dev/null || echo not)
+
+ifneq (,$(filter $(ARCH),aarch64 arm64))
+TEST_GEN_PROGS := tags_test
+TEST_PROGS := run_tags_test.sh
+endif
+
+include ../lib.mk
diff --git a/tools/testing/selftests/arm64/run_tags_test.sh b/tools/testing/selftests/arm64/run_tags_test.sh
new file mode 100755
index 000000000000..745f11379930
--- /dev/null
+++ b/tools/testing/selftests/arm64/run_tags_test.sh
@@ -0,0 +1,12 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+echo "--------------------"
+echo "running tags test"
+echo "--------------------"
+./tags_test
+if [ $? -ne 0 ]; then
+ echo "[FAIL]"
+else
+ echo "[PASS]"
+fi
diff --git a/tools/testing/selftests/arm64/tags_test.c b/tools/testing/selftests/arm64/tags_test.c
new file mode 100644
index 000000000000..22a1b266e373
--- /dev/null
+++ b/tools/testing/selftests/arm64/tags_test.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdint.h>
+#include <sys/prctl.h>
+#include <sys/utsname.h>
+
+#define SHIFT_TAG(tag) ((uint64_t)(tag) << 56)
+#define SET_TAG(ptr, tag) (((uint64_t)(ptr) & ~SHIFT_TAG(0xff)) | \
+ SHIFT_TAG(tag))
+
+int main(void)
+{
+ static int tbi_enabled = 0;
+ struct utsname *ptr, *tagged_ptr;
+ int err;
+
+ if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
+ tbi_enabled = 1;
+ ptr = (struct utsname *)malloc(sizeof(*ptr));
+ if (tbi_enabled)
+ tagged_ptr = (struct utsname *)SET_TAG(ptr, 0x42);
+ err = uname(tagged_ptr);
+ free(ptr);
+
+ return err;
+}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
2019-06-24 14:33 ` [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel Andrey Konovalov
@ 2019-06-24 15:02 ` Kees Cook
2019-06-24 17:38 ` Catalin Marinas
2019-08-23 13:56 ` Cristian Marussi
2 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2019-06-24 15:02 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander, Alex Williamson, Leon Romanovsky,
Luc Van Oostenryck, Dave Martin, Khalid Aziz, enh,
Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy
On Mon, Jun 24, 2019 at 04:33:00PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> This patch adds a simple test, that calls the uname syscall with a
> tagged user pointer as an argument. Without the kernel accepting tagged
> user pointers the test fails with EFAULT.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Acked-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> tools/testing/selftests/arm64/.gitignore | 1 +
> tools/testing/selftests/arm64/Makefile | 11 +++++++
> .../testing/selftests/arm64/run_tags_test.sh | 12 ++++++++
> tools/testing/selftests/arm64/tags_test.c | 29 +++++++++++++++++++
> 4 files changed, 53 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/.gitignore
> create mode 100644 tools/testing/selftests/arm64/Makefile
> create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh
> create mode 100644 tools/testing/selftests/arm64/tags_test.c
>
> diff --git a/tools/testing/selftests/arm64/.gitignore b/tools/testing/selftests/arm64/.gitignore
> new file mode 100644
> index 000000000000..e8fae8d61ed6
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/.gitignore
> @@ -0,0 +1 @@
> +tags_test
> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> new file mode 100644
> index 000000000000..a61b2e743e99
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/Makefile
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# ARCH can be overridden by the user for cross compiling
> +ARCH ?= $(shell uname -m 2>/dev/null || echo not)
> +
> +ifneq (,$(filter $(ARCH),aarch64 arm64))
> +TEST_GEN_PROGS := tags_test
> +TEST_PROGS := run_tags_test.sh
> +endif
> +
> +include ../lib.mk
> diff --git a/tools/testing/selftests/arm64/run_tags_test.sh b/tools/testing/selftests/arm64/run_tags_test.sh
> new file mode 100755
> index 000000000000..745f11379930
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/run_tags_test.sh
> @@ -0,0 +1,12 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +
> +echo "--------------------"
> +echo "running tags test"
> +echo "--------------------"
> +./tags_test
> +if [ $? -ne 0 ]; then
> + echo "[FAIL]"
> +else
> + echo "[PASS]"
> +fi
> diff --git a/tools/testing/selftests/arm64/tags_test.c b/tools/testing/selftests/arm64/tags_test.c
> new file mode 100644
> index 000000000000..22a1b266e373
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/tags_test.c
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <stdint.h>
> +#include <sys/prctl.h>
> +#include <sys/utsname.h>
> +
> +#define SHIFT_TAG(tag) ((uint64_t)(tag) << 56)
> +#define SET_TAG(ptr, tag) (((uint64_t)(ptr) & ~SHIFT_TAG(0xff)) | \
> + SHIFT_TAG(tag))
> +
> +int main(void)
> +{
> + static int tbi_enabled = 0;
> + struct utsname *ptr, *tagged_ptr;
> + int err;
> +
> + if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> + tbi_enabled = 1;
> + ptr = (struct utsname *)malloc(sizeof(*ptr));
> + if (tbi_enabled)
> + tagged_ptr = (struct utsname *)SET_TAG(ptr, 0x42);
> + err = uname(tagged_ptr);
> + free(ptr);
> +
> + return err;
> +}
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
2019-06-24 14:33 ` [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel Andrey Konovalov
2019-06-24 15:02 ` Kees Cook
@ 2019-06-24 17:38 ` Catalin Marinas
2019-08-23 13:56 ` Cristian Marussi
2 siblings, 0 replies; 47+ messages in thread
From: Catalin Marinas @ 2019-06-24 17:38 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Vincenzo Frascino,
Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
Christian Koenig, Mauro Carvalho Chehab, Jens Wiklander,
Alex Williamson, Leon Romanovsky, Luc Van Oostenryck,
Dave Martin, Khalid Aziz, enh, Jason Gunthorpe,
Christoph Hellwig, Dmitry Vyukov, Kostya Serebryany,
Evgeniy Stepanov, Lee Smith, Ramana Radhakrishnan, Jacob Bramley,
Ruben Ayrapetyan, Robin Murphy, Kevin Brodsky, Szabolcs Nagy
On Mon, Jun 24, 2019 at 04:33:00PM +0200, Andrey Konovalov wrote:
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/tags_test.c
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <stdint.h>
> +#include <sys/prctl.h>
> +#include <sys/utsname.h>
> +
> +#define SHIFT_TAG(tag) ((uint64_t)(tag) << 56)
> +#define SET_TAG(ptr, tag) (((uint64_t)(ptr) & ~SHIFT_TAG(0xff)) | \
> + SHIFT_TAG(tag))
> +
> +int main(void)
> +{
> + static int tbi_enabled = 0;
> + struct utsname *ptr, *tagged_ptr;
> + int err;
> +
> + if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> + tbi_enabled = 1;
Nitpick: with the latest prctl() patch, you can skip the last three
arguments as they are ignored.
Either way:
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
2019-06-24 14:33 ` [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel Andrey Konovalov
2019-06-24 15:02 ` Kees Cook
2019-06-24 17:38 ` Catalin Marinas
@ 2019-08-23 13:56 ` Cristian Marussi
2019-08-23 17:16 ` Andrey Konovalov
2 siblings, 1 reply; 47+ messages in thread
From: Cristian Marussi @ 2019-08-23 13:56 UTC (permalink / raw)
To: Andrey Konovalov, linux-arm-kernel, linux-mm, linux-kernel,
amd-gfx, dri-devel, linux-rdma, linux-media, kvm,
linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh, Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy
Hi Andrey
On 24/06/2019 15:33, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> This patch adds a simple test, that calls the uname syscall with a
> tagged user pointer as an argument. Without the kernel accepting tagged
> user pointers the test fails with EFAULT.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
> tools/testing/selftests/arm64/.gitignore | 1 +
> tools/testing/selftests/arm64/Makefile | 11 +++++++
> .../testing/selftests/arm64/run_tags_test.sh | 12 ++++++++
> tools/testing/selftests/arm64/tags_test.c | 29 +++++++++++++++++++
> 4 files changed, 53 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/.gitignore
> create mode 100644 tools/testing/selftests/arm64/Makefile
> create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh
> create mode 100644 tools/testing/selftests/arm64/tags_test.c
After building a fresh Kernel from arm64/for-next-core from scratch at:
commit 239ab658bea3b387424501e7c416640d6752dc0c
Merge: 6bfa3134bd3a 42d038c4fb00 1243cb6a676f d55c5f28afaf d06fa5a118f1 34b5560db40d
Author: Will Deacon <will@kernel.org>
Date: Thu Aug 22 18:23:53 2019 +0100
Merge branches 'for-next/error-injection', 'for-next/tbi', 'for-next/psci-cpuidle', 'for-next/cpu-topology' and 'for-next/52-bit-kva' into for-next/core
KSFT arm64 tests build is broken for me, both setting or not KBUILD_OUTPUT=
13:30 $ make TARGETS=arm64 kselftest-clean
make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
rm -f -r /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test
make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
✔ ~/ARM/dev/src/pdsw/linux [arm64_for_next_core|…8⚑ 23]
13:30 $ make TARGETS=arm64 kselftest
make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built
make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \
ARCH=arm64 -C ../../.. headers_install
HOSTCC scripts/basic/fixdep
HOSTCC scripts/unifdef
...
...
HDRINST usr/include/asm/msgbuf.h
HDRINST usr/include/asm/shmbuf.h
INSTALL /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/usr/include
/opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc tags_test.c -o /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test
tags_test.c: In function ‘main’:
tags_test.c:21:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first use in this function); did you mean ‘PR_GET_TID_ADDRESS’?
if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
^~~~~~~~~~~~~~~~~~~~~~~
PR_GET_TID_ADDRESS
tags_test.c:21:12: note: each undeclared identifier is reported only once for each function it appears in
tags_test.c:21:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use in this function); did you mean ‘PR_GET_DUMPABLE’?
if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
^~~~~~~~~~~~~~~~~~~~~
PR_GET_DUMPABLE
../lib.mk:138: recipe for target '/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test' failed
make[3]: *** [/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test] Error 1
Makefile:136: recipe for target 'all' failed
make[2]: *** [all] Error 2
/home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1237: recipe for target 'kselftest' failed
make[1]: *** [kselftest] Error 2
make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
Makefile:179: recipe for target 'sub-make' failed
make: *** [sub-make] Error 2
Despite seeing KSFT installing Kernel Headers, they cannot be found.
Fixing this patch like this make it work for me:
diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
index a61b2e743e99..f9f79fb272f0 100644
--- a/tools/testing/selftests/arm64/Makefile
+++ b/tools/testing/selftests/arm64/Makefile
@@ -4,6 +4,7 @@
ARCH ?= $(shell uname -m 2>/dev/null || echo not)
ifneq (,$(filter $(ARCH),aarch64 arm64))
+CFLAGS += -I../../../../usr/include/
TEST_GEN_PROGS := tags_test
TEST_PROGS := run_tags_test.sh
endif
but is not really a proper fix since it does NOT account for case in which you have
installed the Kernel Headers in a non standard location like when you use KBUILD_OUTPUT.
Am I missing something ?
Thanks
Cristian
>
> diff --git a/tools/testing/selftests/arm64/.gitignore b/tools/testing/selftests/arm64/.gitignore
> new file mode 100644
> index 000000000000..e8fae8d61ed6
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/.gitignore
> @@ -0,0 +1 @@
> +tags_test
> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> new file mode 100644
> index 000000000000..a61b2e743e99
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/Makefile
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# ARCH can be overridden by the user for cross compiling
> +ARCH ?= $(shell uname -m 2>/dev/null || echo not)
> +
> +ifneq (,$(filter $(ARCH),aarch64 arm64))
> +TEST_GEN_PROGS := tags_test
> +TEST_PROGS := run_tags_test.sh
> +endif
> +
> +include ../lib.mk
> diff --git a/tools/testing/selftests/arm64/run_tags_test.sh b/tools/testing/selftests/arm64/run_tags_test.sh
> new file mode 100755
> index 000000000000..745f11379930
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/run_tags_test.sh
> @@ -0,0 +1,12 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +
> +echo "--------------------"
> +echo "running tags test"
> +echo "--------------------"
> +./tags_test
> +if [ $? -ne 0 ]; then
> + echo "[FAIL]"
> +else
> + echo "[PASS]"
> +fi
> diff --git a/tools/testing/selftests/arm64/tags_test.c b/tools/testing/selftests/arm64/tags_test.c
> new file mode 100644
> index 000000000000..22a1b266e373
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/tags_test.c
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <stdint.h>
> +#include <sys/prctl.h>
> +#include <sys/utsname.h>
> +
> +#define SHIFT_TAG(tag) ((uint64_t)(tag) << 56)
> +#define SET_TAG(ptr, tag) (((uint64_t)(ptr) & ~SHIFT_TAG(0xff)) | \
> + SHIFT_TAG(tag))
> +
> +int main(void)
> +{
> + static int tbi_enabled = 0;
> + struct utsname *ptr, *tagged_ptr;
> + int err;
> +
> + if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> + tbi_enabled = 1;
> + ptr = (struct utsname *)malloc(sizeof(*ptr));
> + if (tbi_enabled)
> + tagged_ptr = (struct utsname *)SET_TAG(ptr, 0x42);
> + err = uname(tagged_ptr);
> + free(ptr);
> +
> + return err;
> +}
>
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
2019-08-23 13:56 ` Cristian Marussi
@ 2019-08-23 17:16 ` Andrey Konovalov
2019-08-23 17:49 ` Cristian Marussi
0 siblings, 1 reply; 47+ messages in thread
From: Andrey Konovalov @ 2019-08-23 17:16 UTC (permalink / raw)
To: Cristian Marussi
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Kees Cook, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander, Alex Williamson, Leon Romanovsky,
Luc Van Oostenryck, Dave Martin, Khalid Aziz, enh,
Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy
On Fri, Aug 23, 2019 at 3:56 PM Cristian Marussi
<cristian.marussi@arm.com> wrote:
>
> Hi Andrey
>
> On 24/06/2019 15:33, Andrey Konovalov wrote:
> > This patch is a part of a series that extends kernel ABI to allow to pass
> > tagged user pointers (with the top byte set to something else other than
> > 0x00) as syscall arguments.
> >
> > This patch adds a simple test, that calls the uname syscall with a
> > tagged user pointer as an argument. Without the kernel accepting tagged
> > user pointers the test fails with EFAULT.
> >
> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > ---
> > tools/testing/selftests/arm64/.gitignore | 1 +
> > tools/testing/selftests/arm64/Makefile | 11 +++++++
> > .../testing/selftests/arm64/run_tags_test.sh | 12 ++++++++
> > tools/testing/selftests/arm64/tags_test.c | 29 +++++++++++++++++++
> > 4 files changed, 53 insertions(+)
> > create mode 100644 tools/testing/selftests/arm64/.gitignore
> > create mode 100644 tools/testing/selftests/arm64/Makefile
> > create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh
> > create mode 100644 tools/testing/selftests/arm64/tags_test.c
>
> After building a fresh Kernel from arm64/for-next-core from scratch at:
>
> commit 239ab658bea3b387424501e7c416640d6752dc0c
> Merge: 6bfa3134bd3a 42d038c4fb00 1243cb6a676f d55c5f28afaf d06fa5a118f1 34b5560db40d
> Author: Will Deacon <will@kernel.org>
> Date: Thu Aug 22 18:23:53 2019 +0100
>
> Merge branches 'for-next/error-injection', 'for-next/tbi', 'for-next/psci-cpuidle', 'for-next/cpu-topology' and 'for-next/52-bit-kva' into for-next/core
>
>
> KSFT arm64 tests build is broken for me, both setting or not KBUILD_OUTPUT=
>
> 13:30 $ make TARGETS=arm64 kselftest-clean
> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> rm -f -r /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test
> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
>
> ✔ ~/ARM/dev/src/pdsw/linux [arm64_for_next_core|…8⚑ 23]
>
> 13:30 $ make TARGETS=arm64 kselftest
> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built
> make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \
> ARCH=arm64 -C ../../.. headers_install
> HOSTCC scripts/basic/fixdep
> HOSTCC scripts/unifdef
> ...
> ...
> HDRINST usr/include/asm/msgbuf.h
> HDRINST usr/include/asm/shmbuf.h
> INSTALL /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/usr/include
> /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc tags_test.c -o /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test
> tags_test.c: In function ‘main’:
> tags_test.c:21:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first use in this function); did you mean ‘PR_GET_TID_ADDRESS’?
> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> ^~~~~~~~~~~~~~~~~~~~~~~
> PR_GET_TID_ADDRESS
> tags_test.c:21:12: note: each undeclared identifier is reported only once for each function it appears in
> tags_test.c:21:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use in this function); did you mean ‘PR_GET_DUMPABLE’?
> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> ^~~~~~~~~~~~~~~~~~~~~
> PR_GET_DUMPABLE
> ../lib.mk:138: recipe for target '/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test' failed
> make[3]: *** [/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test] Error 1
> Makefile:136: recipe for target 'all' failed
> make[2]: *** [all] Error 2
> /home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1237: recipe for target 'kselftest' failed
> make[1]: *** [kselftest] Error 2
> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> Makefile:179: recipe for target 'sub-make' failed
> make: *** [sub-make] Error 2
>
> Despite seeing KSFT installing Kernel Headers, they cannot be found.
>
> Fixing this patch like this make it work for me:
>
> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> index a61b2e743e99..f9f79fb272f0 100644
> --- a/tools/testing/selftests/arm64/Makefile
> +++ b/tools/testing/selftests/arm64/Makefile
> @@ -4,6 +4,7 @@
> ARCH ?= $(shell uname -m 2>/dev/null || echo not)
>
> ifneq (,$(filter $(ARCH),aarch64 arm64))
> +CFLAGS += -I../../../../usr/include/
> TEST_GEN_PROGS := tags_test
> TEST_PROGS := run_tags_test.sh
> endif
>
> but is not really a proper fix since it does NOT account for case in which you have
> installed the Kernel Headers in a non standard location like when you use KBUILD_OUTPUT.
>
> Am I missing something ?
Hm, PR_SET_TAGGED_ADDR_CTRL is defined in include/uapi/linux/prctl.h,
and the test has #include <sys/prctl.h> so as long as you've updated
your kernel headers this should work.
(I'm OOO next week, I'll see if I can reproduce this once I'm back).
>
> Thanks
>
> Cristian
>
> >
> > diff --git a/tools/testing/selftests/arm64/.gitignore b/tools/testing/selftests/arm64/.gitignore
> > new file mode 100644
> > index 000000000000..e8fae8d61ed6
> > --- /dev/null
> > +++ b/tools/testing/selftests/arm64/.gitignore
> > @@ -0,0 +1 @@
> > +tags_test
> > diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> > new file mode 100644
> > index 000000000000..a61b2e743e99
> > --- /dev/null
> > +++ b/tools/testing/selftests/arm64/Makefile
> > @@ -0,0 +1,11 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +# ARCH can be overridden by the user for cross compiling
> > +ARCH ?= $(shell uname -m 2>/dev/null || echo not)
> > +
> > +ifneq (,$(filter $(ARCH),aarch64 arm64))
> > +TEST_GEN_PROGS := tags_test
> > +TEST_PROGS := run_tags_test.sh
> > +endif
> > +
> > +include ../lib.mk
> > diff --git a/tools/testing/selftests/arm64/run_tags_test.sh b/tools/testing/selftests/arm64/run_tags_test.sh
> > new file mode 100755
> > index 000000000000..745f11379930
> > --- /dev/null
> > +++ b/tools/testing/selftests/arm64/run_tags_test.sh
> > @@ -0,0 +1,12 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +echo "--------------------"
> > +echo "running tags test"
> > +echo "--------------------"
> > +./tags_test
> > +if [ $? -ne 0 ]; then
> > + echo "[FAIL]"
> > +else
> > + echo "[PASS]"
> > +fi
> > diff --git a/tools/testing/selftests/arm64/tags_test.c b/tools/testing/selftests/arm64/tags_test.c
> > new file mode 100644
> > index 000000000000..22a1b266e373
> > --- /dev/null
> > +++ b/tools/testing/selftests/arm64/tags_test.c
> > @@ -0,0 +1,29 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <stdint.h>
> > +#include <sys/prctl.h>
> > +#include <sys/utsname.h>
> > +
> > +#define SHIFT_TAG(tag) ((uint64_t)(tag) << 56)
> > +#define SET_TAG(ptr, tag) (((uint64_t)(ptr) & ~SHIFT_TAG(0xff)) | \
> > + SHIFT_TAG(tag))
> > +
> > +int main(void)
> > +{
> > + static int tbi_enabled = 0;
> > + struct utsname *ptr, *tagged_ptr;
> > + int err;
> > +
> > + if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> > + tbi_enabled = 1;
> > + ptr = (struct utsname *)malloc(sizeof(*ptr));
> > + if (tbi_enabled)
> > + tagged_ptr = (struct utsname *)SET_TAG(ptr, 0x42);
> > + err = uname(tagged_ptr);
> > + free(ptr);
> > +
> > + return err;
> > +}
> >
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
2019-08-23 17:16 ` Andrey Konovalov
@ 2019-08-23 17:49 ` Cristian Marussi
2019-09-04 14:52 ` Andrey Konovalov
0 siblings, 1 reply; 47+ messages in thread
From: Cristian Marussi @ 2019-08-23 17:49 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Kees Cook, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander, Alex Williamson, Leon Romanovsky,
Luc Van Oostenryck, Dave Martin, Khalid Aziz, enh,
Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy
Hi
On 23/08/2019 18:16, Andrey Konovalov wrote:
> On Fri, Aug 23, 2019 at 3:56 PM Cristian Marussi
> <cristian.marussi@arm.com> wrote:
>>
>> Hi Andrey
>>
>> On 24/06/2019 15:33, Andrey Konovalov wrote:
>>> This patch is a part of a series that extends kernel ABI to allow to pass
>>> tagged user pointers (with the top byte set to something else other than
>>> 0x00) as syscall arguments.
>>>
>>> This patch adds a simple test, that calls the uname syscall with a
>>> tagged user pointer as an argument. Without the kernel accepting tagged
>>> user pointers the test fails with EFAULT.
>>>
>>> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
>>> ---
>>> tools/testing/selftests/arm64/.gitignore | 1 +
>>> tools/testing/selftests/arm64/Makefile | 11 +++++++
>>> .../testing/selftests/arm64/run_tags_test.sh | 12 ++++++++
>>> tools/testing/selftests/arm64/tags_test.c | 29 +++++++++++++++++++
>>> 4 files changed, 53 insertions(+)
>>> create mode 100644 tools/testing/selftests/arm64/.gitignore
>>> create mode 100644 tools/testing/selftests/arm64/Makefile
>>> create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh
>>> create mode 100644 tools/testing/selftests/arm64/tags_test.c
>>
>> After building a fresh Kernel from arm64/for-next-core from scratch at:
>>
>> commit 239ab658bea3b387424501e7c416640d6752dc0c
>> Merge: 6bfa3134bd3a 42d038c4fb00 1243cb6a676f d55c5f28afaf d06fa5a118f1 34b5560db40d
>> Author: Will Deacon <will@kernel.org>
>> Date: Thu Aug 22 18:23:53 2019 +0100
>>
>> Merge branches 'for-next/error-injection', 'for-next/tbi', 'for-next/psci-cpuidle', 'for-next/cpu-topology' and 'for-next/52-bit-kva' into for-next/core
>>
>>
>> KSFT arm64 tests build is broken for me, both setting or not KBUILD_OUTPUT=
>>
>> 13:30 $ make TARGETS=arm64 kselftest-clean
>> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
>> rm -f -r /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test
>> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
>>
>> ✔ ~/ARM/dev/src/pdsw/linux [arm64_for_next_core|…8⚑ 23]
>>
>> 13:30 $ make TARGETS=arm64 kselftest
>> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
>> arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built
>> make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \
>> ARCH=arm64 -C ../../.. headers_install
>> HOSTCC scripts/basic/fixdep
>> HOSTCC scripts/unifdef
>> ...
>> ...
>> HDRINST usr/include/asm/msgbuf.h
>> HDRINST usr/include/asm/shmbuf.h
>> INSTALL /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/usr/include
>> /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc tags_test.c -o /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test
>> tags_test.c: In function ‘main’:
>> tags_test.c:21:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first use in this function); did you mean ‘PR_GET_TID_ADDRESS’?
>> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
>> ^~~~~~~~~~~~~~~~~~~~~~~
>> PR_GET_TID_ADDRESS
>> tags_test.c:21:12: note: each undeclared identifier is reported only once for each function it appears in
>> tags_test.c:21:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use in this function); did you mean ‘PR_GET_DUMPABLE’?
>> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
>> ^~~~~~~~~~~~~~~~~~~~~
>> PR_GET_DUMPABLE
>> ../lib.mk:138: recipe for target '/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test' failed
>> make[3]: *** [/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test] Error 1
>> Makefile:136: recipe for target 'all' failed
>> make[2]: *** [all] Error 2
>> /home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1237: recipe for target 'kselftest' failed
>> make[1]: *** [kselftest] Error 2
>> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
>> Makefile:179: recipe for target 'sub-make' failed
>> make: *** [sub-make] Error 2
>>
>> Despite seeing KSFT installing Kernel Headers, they cannot be found.
>>
>> Fixing this patch like this make it work for me:
>>
>> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
>> index a61b2e743e99..f9f79fb272f0 100644
>> --- a/tools/testing/selftests/arm64/Makefile
>> +++ b/tools/testing/selftests/arm64/Makefile
>> @@ -4,6 +4,7 @@
>> ARCH ?= $(shell uname -m 2>/dev/null || echo not)
>>
>> ifneq (,$(filter $(ARCH),aarch64 arm64))
>> +CFLAGS += -I../../../../usr/include/
>> TEST_GEN_PROGS := tags_test
>> TEST_PROGS := run_tags_test.sh
>> endif
>>
>> but is not really a proper fix since it does NOT account for case in which you have
>> installed the Kernel Headers in a non standard location like when you use KBUILD_OUTPUT.
>>
>> Am I missing something ?
>
> Hm, PR_SET_TAGGED_ADDR_CTRL is defined in include/uapi/linux/prctl.h,
> and the test has #include <sys/prctl.h> so as long as you've updated
> your kernel headers this should work.
>
> (I'm OOO next week, I'll see if I can reproduce this once I'm back).
Ok. Thanks for the reply.
I think I've got it in my local tree having cloned arm64/for-next-core:
18:32 $ egrep -A 10 PR_SET_TAG ./include/uapi/linux/prctl.h
#define PR_SET_TAGGED_ADDR_CTRL 55
#define PR_GET_TAGGED_ADDR_CTRL 56
# define PR_TAGGED_ADDR_ENABLE (1UL << 0)
#endif /* _LINUX_PRCTL_H */
and Kernel header are locally installed in my kernel src dir (by KSFT indeed)
18:34 $ egrep -RA 10 PR_SET_TAG usr/include/
usr/include/linux/prctl.h:#define PR_SET_TAGGED_ADDR_CTRL 55
usr/include/linux/prctl.h-#define PR_GET_TAGGED_ADDR_CTRL 56
usr/include/linux/prctl.h-# define PR_TAGGED_ADDR_ENABLE (1UL << 0)
usr/include/linux/prctl.h-
usr/include/linux/prctl.h-#endif /* _LINUX_PRCTL_H */
but how are they supposed to be found if nor the test Makefile
neither the KSFT Makefile who installs them pass any -I options to the
compiler ?
I suppose <sys/prctl.h> tries to include arch specific headers from the regular system path,
but when you are cross-compiling ?
18:34 $ make TARGETS=arm64 kselftest
make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built
make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \
ARCH=arm64 -C ../../.. headers_install
INSTALL /home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/usr/include
/opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc -Wall -O2 -g tags_test.c -o /home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test
tags_test.c: In function ‘main’:
tags_test.c:20:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first use in this function); did you mean ‘PR_GET_TID_ADDRESS’?
if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
^~~~~~~~~~~~~~~~~~~~~~~
PR_GET_TID_ADDRESS
tags_test.c:20:12: note: each undeclared identifier is reported only once for each function it appears in
tags_test.c:20:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use in this function); did you mean ‘PR_GET_DUMPABLE’?
if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
^~~~~~~~~~~~~~~~~~~~~
PR_GET_DUMPABLE
../../lib.mk:138: recipe for target '/home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test' failed
make[4]: *** [/home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test] Error 1
Makefile:19: recipe for target 'all' failed
make[3]: *** [all] Error 2
Makefile:137: recipe for target 'all' failed
make[2]: *** [all] Error 2
/home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1236: recipe for target 'kselftest' failed
make[1]: *** [kselftest] Error 2
make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
Makefile:179: recipe for target 'sub-make' failed
make: *** [sub-make] Error 2
In fact many KSFT testcases seems to brutally add default headers path:
tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../include/uapi/
tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../include/
tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../usr/include/
tools/testing/selftests/net/Makefile:CFLAGS += -I../../../../usr/include/
tools/testing/selftests/membarrier/Makefile:CFLAGS += -g -I../../../../usr/include/
...
Cheers
Cristian
>
>
>
>>
>> Thanks
>>
>> Cristian
>>
>>>
>>> diff --git a/tools/testing/selftests/arm64/.gitignore b/tools/testing/selftests/arm64/.gitignore
>>> new file mode 100644
>>> index 000000000000..e8fae8d61ed6
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/arm64/.gitignore
>>> @@ -0,0 +1 @@
>>> +tags_test
>>> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
>>> new file mode 100644
>>> index 000000000000..a61b2e743e99
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/arm64/Makefile
>>> @@ -0,0 +1,11 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +
>>> +# ARCH can be overridden by the user for cross compiling
>>> +ARCH ?= $(shell uname -m 2>/dev/null || echo not)
>>> +
>>> +ifneq (,$(filter $(ARCH),aarch64 arm64))
>>> +TEST_GEN_PROGS := tags_test
>>> +TEST_PROGS := run_tags_test.sh
>>> +endif
>>> +
>>> +include ../lib.mk
>>> diff --git a/tools/testing/selftests/arm64/run_tags_test.sh b/tools/testing/selftests/arm64/run_tags_test.sh
>>> new file mode 100755
>>> index 000000000000..745f11379930
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/arm64/run_tags_test.sh
>>> @@ -0,0 +1,12 @@
>>> +#!/bin/sh
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +
>>> +echo "--------------------"
>>> +echo "running tags test"
>>> +echo "--------------------"
>>> +./tags_test
>>> +if [ $? -ne 0 ]; then
>>> + echo "[FAIL]"
>>> +else
>>> + echo "[PASS]"
>>> +fi
>>> diff --git a/tools/testing/selftests/arm64/tags_test.c b/tools/testing/selftests/arm64/tags_test.c
>>> new file mode 100644
>>> index 000000000000..22a1b266e373
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/arm64/tags_test.c
>>> @@ -0,0 +1,29 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +#include <unistd.h>
>>> +#include <stdint.h>
>>> +#include <sys/prctl.h>
>>> +#include <sys/utsname.h>
>>> +
>>> +#define SHIFT_TAG(tag) ((uint64_t)(tag) << 56)
>>> +#define SET_TAG(ptr, tag) (((uint64_t)(ptr) & ~SHIFT_TAG(0xff)) | \
>>> + SHIFT_TAG(tag))
>>> +
>>> +int main(void)
>>> +{
>>> + static int tbi_enabled = 0;
>>> + struct utsname *ptr, *tagged_ptr;
>>> + int err;
>>> +
>>> + if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
>>> + tbi_enabled = 1;
>>> + ptr = (struct utsname *)malloc(sizeof(*ptr));
>>> + if (tbi_enabled)
>>> + tagged_ptr = (struct utsname *)SET_TAG(ptr, 0x42);
>>> + err = uname(tagged_ptr);
>>> + free(ptr);
>>> +
>>> + return err;
>>> +}
>>>
>>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
2019-08-23 17:49 ` Cristian Marussi
@ 2019-09-04 14:52 ` Andrey Konovalov
2019-09-04 16:22 ` Cristian Marussi
0 siblings, 1 reply; 47+ messages in thread
From: Andrey Konovalov @ 2019-09-04 14:52 UTC (permalink / raw)
To: Cristian Marussi
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Kees Cook, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander, Alex Williamson, Leon Romanovsky,
Luc Van Oostenryck, Dave Martin, Khalid Aziz, enh,
Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy
On Fri, Aug 23, 2019 at 7:49 PM Cristian Marussi
<cristian.marussi@arm.com> wrote:
>
>
> Hi
>
> On 23/08/2019 18:16, Andrey Konovalov wrote:
> > On Fri, Aug 23, 2019 at 3:56 PM Cristian Marussi
> > <cristian.marussi@arm.com> wrote:
> >>
> >> Hi Andrey
> >>
> >> On 24/06/2019 15:33, Andrey Konovalov wrote:
> >>> This patch is a part of a series that extends kernel ABI to allow to pass
> >>> tagged user pointers (with the top byte set to something else other than
> >>> 0x00) as syscall arguments.
> >>>
> >>> This patch adds a simple test, that calls the uname syscall with a
> >>> tagged user pointer as an argument. Without the kernel accepting tagged
> >>> user pointers the test fails with EFAULT.
> >>>
> >>> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> >>> ---
> >>> tools/testing/selftests/arm64/.gitignore | 1 +
> >>> tools/testing/selftests/arm64/Makefile | 11 +++++++
> >>> .../testing/selftests/arm64/run_tags_test.sh | 12 ++++++++
> >>> tools/testing/selftests/arm64/tags_test.c | 29 +++++++++++++++++++
> >>> 4 files changed, 53 insertions(+)
> >>> create mode 100644 tools/testing/selftests/arm64/.gitignore
> >>> create mode 100644 tools/testing/selftests/arm64/Makefile
> >>> create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh
> >>> create mode 100644 tools/testing/selftests/arm64/tags_test.c
> >>
> >> After building a fresh Kernel from arm64/for-next-core from scratch at:
> >>
> >> commit 239ab658bea3b387424501e7c416640d6752dc0c
> >> Merge: 6bfa3134bd3a 42d038c4fb00 1243cb6a676f d55c5f28afaf d06fa5a118f1 34b5560db40d
> >> Author: Will Deacon <will@kernel.org>
> >> Date: Thu Aug 22 18:23:53 2019 +0100
> >>
> >> Merge branches 'for-next/error-injection', 'for-next/tbi', 'for-next/psci-cpuidle', 'for-next/cpu-topology' and 'for-next/52-bit-kva' into for-next/core
> >>
> >>
> >> KSFT arm64 tests build is broken for me, both setting or not KBUILD_OUTPUT=
> >>
> >> 13:30 $ make TARGETS=arm64 kselftest-clean
> >> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> >> rm -f -r /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test
> >> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> >>
> >> ✔ ~/ARM/dev/src/pdsw/linux [arm64_for_next_core|…8⚑ 23]
> >>
> >> 13:30 $ make TARGETS=arm64 kselftest
> >> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> >> arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built
> >> make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \
> >> ARCH=arm64 -C ../../.. headers_install
> >> HOSTCC scripts/basic/fixdep
> >> HOSTCC scripts/unifdef
> >> ...
> >> ...
> >> HDRINST usr/include/asm/msgbuf.h
> >> HDRINST usr/include/asm/shmbuf.h
> >> INSTALL /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/usr/include
> >> /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc tags_test.c -o /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test
> >> tags_test.c: In function ‘main’:
> >> tags_test.c:21:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first use in this function); did you mean ‘PR_GET_TID_ADDRESS’?
> >> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> >> ^~~~~~~~~~~~~~~~~~~~~~~
> >> PR_GET_TID_ADDRESS
> >> tags_test.c:21:12: note: each undeclared identifier is reported only once for each function it appears in
> >> tags_test.c:21:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use in this function); did you mean ‘PR_GET_DUMPABLE’?
> >> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> >> ^~~~~~~~~~~~~~~~~~~~~
> >> PR_GET_DUMPABLE
> >> ../lib.mk:138: recipe for target '/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test' failed
> >> make[3]: *** [/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test] Error 1
> >> Makefile:136: recipe for target 'all' failed
> >> make[2]: *** [all] Error 2
> >> /home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1237: recipe for target 'kselftest' failed
> >> make[1]: *** [kselftest] Error 2
> >> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> >> Makefile:179: recipe for target 'sub-make' failed
> >> make: *** [sub-make] Error 2
> >>
> >> Despite seeing KSFT installing Kernel Headers, they cannot be found.
> >>
> >> Fixing this patch like this make it work for me:
> >>
> >> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> >> index a61b2e743e99..f9f79fb272f0 100644
> >> --- a/tools/testing/selftests/arm64/Makefile
> >> +++ b/tools/testing/selftests/arm64/Makefile
> >> @@ -4,6 +4,7 @@
> >> ARCH ?= $(shell uname -m 2>/dev/null || echo not)
> >>
> >> ifneq (,$(filter $(ARCH),aarch64 arm64))
> >> +CFLAGS += -I../../../../usr/include/
> >> TEST_GEN_PROGS := tags_test
> >> TEST_PROGS := run_tags_test.sh
> >> endif
> >>
> >> but is not really a proper fix since it does NOT account for case in which you have
> >> installed the Kernel Headers in a non standard location like when you use KBUILD_OUTPUT.
> >>
> >> Am I missing something ?
> >
> > Hm, PR_SET_TAGGED_ADDR_CTRL is defined in include/uapi/linux/prctl.h,
> > and the test has #include <sys/prctl.h> so as long as you've updated
> > your kernel headers this should work.
> >
> > (I'm OOO next week, I'll see if I can reproduce this once I'm back).
>
> Ok. Thanks for the reply.
>
> I think I've got it in my local tree having cloned arm64/for-next-core:
>
> 18:32 $ egrep -A 10 PR_SET_TAG ./include/uapi/linux/prctl.h
> #define PR_SET_TAGGED_ADDR_CTRL 55
> #define PR_GET_TAGGED_ADDR_CTRL 56
> # define PR_TAGGED_ADDR_ENABLE (1UL << 0)
>
> #endif /* _LINUX_PRCTL_H */
>
> and Kernel header are locally installed in my kernel src dir (by KSFT indeed)
>
> 18:34 $ egrep -RA 10 PR_SET_TAG usr/include/
> usr/include/linux/prctl.h:#define PR_SET_TAGGED_ADDR_CTRL 55
> usr/include/linux/prctl.h-#define PR_GET_TAGGED_ADDR_CTRL 56
> usr/include/linux/prctl.h-# define PR_TAGGED_ADDR_ENABLE (1UL << 0)
> usr/include/linux/prctl.h-
> usr/include/linux/prctl.h-#endif /* _LINUX_PRCTL_H */
>
> but how are they supposed to be found if nor the test Makefile
> neither the KSFT Makefile who installs them pass any -I options to the
> compiler ?
> I suppose <sys/prctl.h> tries to include arch specific headers from the regular system path,
> but when you are cross-compiling ?
>
> 18:34 $ make TARGETS=arm64 kselftest
> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built
> make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \
> ARCH=arm64 -C ../../.. headers_install
> INSTALL /home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/usr/include
> /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc -Wall -O2 -g tags_test.c -o /home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test
> tags_test.c: In function ‘main’:
> tags_test.c:20:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first use in this function); did you mean ‘PR_GET_TID_ADDRESS’?
> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> ^~~~~~~~~~~~~~~~~~~~~~~
> PR_GET_TID_ADDRESS
> tags_test.c:20:12: note: each undeclared identifier is reported only once for each function it appears in
> tags_test.c:20:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use in this function); did you mean ‘PR_GET_DUMPABLE’?
> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> ^~~~~~~~~~~~~~~~~~~~~
> PR_GET_DUMPABLE
> ../../lib.mk:138: recipe for target '/home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test' failed
> make[4]: *** [/home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test] Error 1
> Makefile:19: recipe for target 'all' failed
> make[3]: *** [all] Error 2
> Makefile:137: recipe for target 'all' failed
> make[2]: *** [all] Error 2
> /home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1236: recipe for target 'kselftest' failed
> make[1]: *** [kselftest] Error 2
> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> Makefile:179: recipe for target 'sub-make' failed
> make: *** [sub-make] Error 2
>
>
> In fact many KSFT testcases seems to brutally add default headers path:
>
> tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../include/uapi/
> tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../include/
> tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../usr/include/
> tools/testing/selftests/net/Makefile:CFLAGS += -I../../../../usr/include/
> tools/testing/selftests/membarrier/Makefile:CFLAGS += -g -I../../../../usr/include/
> ...
Hi Cristian!
Indeed, I can reproduce the issue. I don't know what's the proper way
to resolve this. Adding "CFLAGS += -I../../../../usr/include/" looks
good to me. AFAICS your series resolves this issue in a similar way,
but I think we should fix this before the current rc is released. Do
you want to submit a patch that adds this simple fix or should I do
that?
Thanks!
>
> Cheers
>
> Cristian
> >
> >
> >
> >>
> >> Thanks
> >>
> >> Cristian
> >>
> >>>
> >>> diff --git a/tools/testing/selftests/arm64/.gitignore b/tools/testing/selftests/arm64/.gitignore
> >>> new file mode 100644
> >>> index 000000000000..e8fae8d61ed6
> >>> --- /dev/null
> >>> +++ b/tools/testing/selftests/arm64/.gitignore
> >>> @@ -0,0 +1 @@
> >>> +tags_test
> >>> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> >>> new file mode 100644
> >>> index 000000000000..a61b2e743e99
> >>> --- /dev/null
> >>> +++ b/tools/testing/selftests/arm64/Makefile
> >>> @@ -0,0 +1,11 @@
> >>> +# SPDX-License-Identifier: GPL-2.0
> >>> +
> >>> +# ARCH can be overridden by the user for cross compiling
> >>> +ARCH ?= $(shell uname -m 2>/dev/null || echo not)
> >>> +
> >>> +ifneq (,$(filter $(ARCH),aarch64 arm64))
> >>> +TEST_GEN_PROGS := tags_test
> >>> +TEST_PROGS := run_tags_test.sh
> >>> +endif
> >>> +
> >>> +include ../lib.mk
> >>> diff --git a/tools/testing/selftests/arm64/run_tags_test.sh b/tools/testing/selftests/arm64/run_tags_test.sh
> >>> new file mode 100755
> >>> index 000000000000..745f11379930
> >>> --- /dev/null
> >>> +++ b/tools/testing/selftests/arm64/run_tags_test.sh
> >>> @@ -0,0 +1,12 @@
> >>> +#!/bin/sh
> >>> +# SPDX-License-Identifier: GPL-2.0
> >>> +
> >>> +echo "--------------------"
> >>> +echo "running tags test"
> >>> +echo "--------------------"
> >>> +./tags_test
> >>> +if [ $? -ne 0 ]; then
> >>> + echo "[FAIL]"
> >>> +else
> >>> + echo "[PASS]"
> >>> +fi
> >>> diff --git a/tools/testing/selftests/arm64/tags_test.c b/tools/testing/selftests/arm64/tags_test.c
> >>> new file mode 100644
> >>> index 000000000000..22a1b266e373
> >>> --- /dev/null
> >>> +++ b/tools/testing/selftests/arm64/tags_test.c
> >>> @@ -0,0 +1,29 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +
> >>> +#include <stdio.h>
> >>> +#include <stdlib.h>
> >>> +#include <unistd.h>
> >>> +#include <stdint.h>
> >>> +#include <sys/prctl.h>
> >>> +#include <sys/utsname.h>
> >>> +
> >>> +#define SHIFT_TAG(tag) ((uint64_t)(tag) << 56)
> >>> +#define SET_TAG(ptr, tag) (((uint64_t)(ptr) & ~SHIFT_TAG(0xff)) | \
> >>> + SHIFT_TAG(tag))
> >>> +
> >>> +int main(void)
> >>> +{
> >>> + static int tbi_enabled = 0;
> >>> + struct utsname *ptr, *tagged_ptr;
> >>> + int err;
> >>> +
> >>> + if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> >>> + tbi_enabled = 1;
> >>> + ptr = (struct utsname *)malloc(sizeof(*ptr));
> >>> + if (tbi_enabled)
> >>> + tagged_ptr = (struct utsname *)SET_TAG(ptr, 0x42);
> >>> + err = uname(tagged_ptr);
> >>> + free(ptr);
> >>> +
> >>> + return err;
> >>> +}
> >>>
> >>
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
2019-09-04 14:52 ` Andrey Konovalov
@ 2019-09-04 16:22 ` Cristian Marussi
2019-09-04 16:42 ` Andrey Konovalov
0 siblings, 1 reply; 47+ messages in thread
From: Cristian Marussi @ 2019-09-04 16:22 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Kees Cook, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander, Alex Williamson, Leon Romanovsky,
Luc Van Oostenryck, Dave Martin, Khalid Aziz, enh,
Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy
Hi Andrey !
On 04/09/2019 15:52, Andrey Konovalov wrote:
> On Fri, Aug 23, 2019 at 7:49 PM Cristian Marussi
> <cristian.marussi@arm.com> wrote:
>>
>>
>> Hi
>>
>> On 23/08/2019 18:16, Andrey Konovalov wrote:
>>> On Fri, Aug 23, 2019 at 3:56 PM Cristian Marussi
>>> <cristian.marussi@arm.com> wrote:
>>>>
>>>> Hi Andrey
>>>>
>>>> On 24/06/2019 15:33, Andrey Konovalov wrote:
>>>>> This patch is a part of a series that extends kernel ABI to allow to pass
>>>>> tagged user pointers (with the top byte set to something else other than
>>>>> 0x00) as syscall arguments.
>>>>>
>>>>> This patch adds a simple test, that calls the uname syscall with a
>>>>> tagged user pointer as an argument. Without the kernel accepting tagged
>>>>> user pointers the test fails with EFAULT.
>>>>>
>>>>> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
>>>>> ---
>>>>> tools/testing/selftests/arm64/.gitignore | 1 +
>>>>> tools/testing/selftests/arm64/Makefile | 11 +++++++
>>>>> .../testing/selftests/arm64/run_tags_test.sh | 12 ++++++++
>>>>> tools/testing/selftests/arm64/tags_test.c | 29 +++++++++++++++++++
>>>>> 4 files changed, 53 insertions(+)
>>>>> create mode 100644 tools/testing/selftests/arm64/.gitignore
>>>>> create mode 100644 tools/testing/selftests/arm64/Makefile
>>>>> create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh
>>>>> create mode 100644 tools/testing/selftests/arm64/tags_test.c
>>>>
>>>> After building a fresh Kernel from arm64/for-next-core from scratch at:
>>>>
>>>> commit 239ab658bea3b387424501e7c416640d6752dc0c
>>>> Merge: 6bfa3134bd3a 42d038c4fb00 1243cb6a676f d55c5f28afaf d06fa5a118f1 34b5560db40d
>>>> Author: Will Deacon <will@kernel.org>
>>>> Date: Thu Aug 22 18:23:53 2019 +0100
>>>>
>>>> Merge branches 'for-next/error-injection', 'for-next/tbi', 'for-next/psci-cpuidle', 'for-next/cpu-topology' and 'for-next/52-bit-kva' into for-next/core
>>>>
>>>>
>>>> KSFT arm64 tests build is broken for me, both setting or not KBUILD_OUTPUT=
>>>>
>>>> 13:30 $ make TARGETS=arm64 kselftest-clean
>>>> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
>>>> rm -f -r /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test
>>>> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
>>>>
>>>> ✔ ~/ARM/dev/src/pdsw/linux [arm64_for_next_core|…8⚑ 23]
>>>>
>>>> 13:30 $ make TARGETS=arm64 kselftest
>>>> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
>>>> arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built
>>>> make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \
>>>> ARCH=arm64 -C ../../.. headers_install
>>>> HOSTCC scripts/basic/fixdep
>>>> HOSTCC scripts/unifdef
>>>> ...
>>>> ...
>>>> HDRINST usr/include/asm/msgbuf.h
>>>> HDRINST usr/include/asm/shmbuf.h
>>>> INSTALL /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/usr/include
>>>> /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc tags_test.c -o /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test
>>>> tags_test.c: In function ‘main’:
>>>> tags_test.c:21:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first use in this function); did you mean ‘PR_GET_TID_ADDRESS’?
>>>> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
>>>> ^~~~~~~~~~~~~~~~~~~~~~~
>>>> PR_GET_TID_ADDRESS
>>>> tags_test.c:21:12: note: each undeclared identifier is reported only once for each function it appears in
>>>> tags_test.c:21:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use in this function); did you mean ‘PR_GET_DUMPABLE’?
>>>> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
>>>> ^~~~~~~~~~~~~~~~~~~~~
>>>> PR_GET_DUMPABLE
>>>> ../lib.mk:138: recipe for target '/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test' failed
>>>> make[3]: *** [/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test] Error 1
>>>> Makefile:136: recipe for target 'all' failed
>>>> make[2]: *** [all] Error 2
>>>> /home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1237: recipe for target 'kselftest' failed
>>>> make[1]: *** [kselftest] Error 2
>>>> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
>>>> Makefile:179: recipe for target 'sub-make' failed
>>>> make: *** [sub-make] Error 2
>>>>
>>>> Despite seeing KSFT installing Kernel Headers, they cannot be found.
>>>>
>>>> Fixing this patch like this make it work for me:
>>>>
>>>> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
>>>> index a61b2e743e99..f9f79fb272f0 100644
>>>> --- a/tools/testing/selftests/arm64/Makefile
>>>> +++ b/tools/testing/selftests/arm64/Makefile
>>>> @@ -4,6 +4,7 @@
>>>> ARCH ?= $(shell uname -m 2>/dev/null || echo not)
>>>>
>>>> ifneq (,$(filter $(ARCH),aarch64 arm64))
>>>> +CFLAGS += -I../../../../usr/include/
>>>> TEST_GEN_PROGS := tags_test
>>>> TEST_PROGS := run_tags_test.sh
>>>> endif
>>>>
>>>> but is not really a proper fix since it does NOT account for case in which you have
>>>> installed the Kernel Headers in a non standard location like when you use KBUILD_OUTPUT.
>>>>
>>>> Am I missing something ?
>>>
>>> Hm, PR_SET_TAGGED_ADDR_CTRL is defined in include/uapi/linux/prctl.h,
>>> and the test has #include <sys/prctl.h> so as long as you've updated
>>> your kernel headers this should work.
>>>
>>> (I'm OOO next week, I'll see if I can reproduce this once I'm back).
>>
>> Ok. Thanks for the reply.
>>
>> I think I've got it in my local tree having cloned arm64/for-next-core:
>>
>> 18:32 $ egrep -A 10 PR_SET_TAG ./include/uapi/linux/prctl.h
>> #define PR_SET_TAGGED_ADDR_CTRL 55
>> #define PR_GET_TAGGED_ADDR_CTRL 56
>> # define PR_TAGGED_ADDR_ENABLE (1UL << 0)
>>
>> #endif /* _LINUX_PRCTL_H */
>>
>> and Kernel header are locally installed in my kernel src dir (by KSFT indeed)
>>
>> 18:34 $ egrep -RA 10 PR_SET_TAG usr/include/
>> usr/include/linux/prctl.h:#define PR_SET_TAGGED_ADDR_CTRL 55
>> usr/include/linux/prctl.h-#define PR_GET_TAGGED_ADDR_CTRL 56
>> usr/include/linux/prctl.h-# define PR_TAGGED_ADDR_ENABLE (1UL << 0)
>> usr/include/linux/prctl.h-
>> usr/include/linux/prctl.h-#endif /* _LINUX_PRCTL_H */
>>
>> but how are they supposed to be found if nor the test Makefile
>> neither the KSFT Makefile who installs them pass any -I options to the
>> compiler ?
>> I suppose <sys/prctl.h> tries to include arch specific headers from the regular system path,
>> but when you are cross-compiling ?
>>
>> 18:34 $ make TARGETS=arm64 kselftest
>> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
>> arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built
>> make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \
>> ARCH=arm64 -C ../../.. headers_install
>> INSTALL /home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/usr/include
>> /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc -Wall -O2 -g tags_test.c -o /home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test
>> tags_test.c: In function ‘main’:
>> tags_test.c:20:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first use in this function); did you mean ‘PR_GET_TID_ADDRESS’?
>> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
>> ^~~~~~~~~~~~~~~~~~~~~~~
>> PR_GET_TID_ADDRESS
>> tags_test.c:20:12: note: each undeclared identifier is reported only once for each function it appears in
>> tags_test.c:20:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use in this function); did you mean ‘PR_GET_DUMPABLE’?
>> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
>> ^~~~~~~~~~~~~~~~~~~~~
>> PR_GET_DUMPABLE
>> ../../lib.mk:138: recipe for target '/home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test' failed
>> make[4]: *** [/home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test] Error 1
>> Makefile:19: recipe for target 'all' failed
>> make[3]: *** [all] Error 2
>> Makefile:137: recipe for target 'all' failed
>> make[2]: *** [all] Error 2
>> /home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1236: recipe for target 'kselftest' failed
>> make[1]: *** [kselftest] Error 2
>> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
>> Makefile:179: recipe for target 'sub-make' failed
>> make: *** [sub-make] Error 2
>>
>>
>> In fact many KSFT testcases seems to brutally add default headers path:
>>
>> tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../include/uapi/
>> tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../include/
>> tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../usr/include/
>> tools/testing/selftests/net/Makefile:CFLAGS += -I../../../../usr/include/
>> tools/testing/selftests/membarrier/Makefile:CFLAGS += -g -I../../../../usr/include/
>> ...
>
> Hi Cristian!
>
> Indeed, I can reproduce the issue. I don't know what's the proper way
> to resolve this. Adding "CFLAGS += -I../../../../usr/include/" looks
> good to me. AFAICS your series resolves this issue in a similar way,
> but I think we should fix this before the current rc is released. Do
> you want to submit a patch that adds this simple fix or should I do
> that?
>
Please feel free to post the single line patch above to quickly fix this before
release, so we don't have a broken build straight away. (our CI is already beating me...:D)
On my side (01/11) in the meantime I'll fix the top level KSFT arm64 makefile so as to calculate
and propagate once for all the headers search path down to all KSFT arm64/ in one go,
trying to guess where they are; this is needed because the above fix works fine as long
as you don't have KBUILD_OUTPUT set, once you set it, KSFT installs kheaders in a different
place and the above -I fix is fooled again....but this is a general problem also in other
KSFT tests as I can see now so I think this fix is good enough for now
(and the fix on my side, even if trivial, is not going to go into this release for sure)
Thanks !
Cheers
Cristian
> Thanks!
>
>>
>> Cheers
>>
>> Cristian
>>>
>>>
>>>
>>>>
>>>> Thanks
>>>>
>>>> Cristian
>>>>
>>>>>
>>>>> diff --git a/tools/testing/selftests/arm64/.gitignore b/tools/testing/selftests/arm64/.gitignore
>>>>> new file mode 100644
>>>>> index 000000000000..e8fae8d61ed6
>>>>> --- /dev/null
>>>>> +++ b/tools/testing/selftests/arm64/.gitignore
>>>>> @@ -0,0 +1 @@
>>>>> +tags_test
>>>>> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
>>>>> new file mode 100644
>>>>> index 000000000000..a61b2e743e99
>>>>> --- /dev/null
>>>>> +++ b/tools/testing/selftests/arm64/Makefile
>>>>> @@ -0,0 +1,11 @@
>>>>> +# SPDX-License-Identifier: GPL-2.0
>>>>> +
>>>>> +# ARCH can be overridden by the user for cross compiling
>>>>> +ARCH ?= $(shell uname -m 2>/dev/null || echo not)
>>>>> +
>>>>> +ifneq (,$(filter $(ARCH),aarch64 arm64))
>>>>> +TEST_GEN_PROGS := tags_test
>>>>> +TEST_PROGS := run_tags_test.sh
>>>>> +endif
>>>>> +
>>>>> +include ../lib.mk
>>>>> diff --git a/tools/testing/selftests/arm64/run_tags_test.sh b/tools/testing/selftests/arm64/run_tags_test.sh
>>>>> new file mode 100755
>>>>> index 000000000000..745f11379930
>>>>> --- /dev/null
>>>>> +++ b/tools/testing/selftests/arm64/run_tags_test.sh
>>>>> @@ -0,0 +1,12 @@
>>>>> +#!/bin/sh
>>>>> +# SPDX-License-Identifier: GPL-2.0
>>>>> +
>>>>> +echo "--------------------"
>>>>> +echo "running tags test"
>>>>> +echo "--------------------"
>>>>> +./tags_test
>>>>> +if [ $? -ne 0 ]; then
>>>>> + echo "[FAIL]"
>>>>> +else
>>>>> + echo "[PASS]"
>>>>> +fi
>>>>> diff --git a/tools/testing/selftests/arm64/tags_test.c b/tools/testing/selftests/arm64/tags_test.c
>>>>> new file mode 100644
>>>>> index 000000000000..22a1b266e373
>>>>> --- /dev/null
>>>>> +++ b/tools/testing/selftests/arm64/tags_test.c
>>>>> @@ -0,0 +1,29 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>> +
>>>>> +#include <stdio.h>
>>>>> +#include <stdlib.h>
>>>>> +#include <unistd.h>
>>>>> +#include <stdint.h>
>>>>> +#include <sys/prctl.h>
>>>>> +#include <sys/utsname.h>
>>>>> +
>>>>> +#define SHIFT_TAG(tag) ((uint64_t)(tag) << 56)
>>>>> +#define SET_TAG(ptr, tag) (((uint64_t)(ptr) & ~SHIFT_TAG(0xff)) | \
>>>>> + SHIFT_TAG(tag))
>>>>> +
>>>>> +int main(void)
>>>>> +{
>>>>> + static int tbi_enabled = 0;
>>>>> + struct utsname *ptr, *tagged_ptr;
>>>>> + int err;
>>>>> +
>>>>> + if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
>>>>> + tbi_enabled = 1;
>>>>> + ptr = (struct utsname *)malloc(sizeof(*ptr));
>>>>> + if (tbi_enabled)
>>>>> + tagged_ptr = (struct utsname *)SET_TAG(ptr, 0x42);
>>>>> + err = uname(tagged_ptr);
>>>>> + free(ptr);
>>>>> +
>>>>> + return err;
>>>>> +}
>>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
2019-09-04 16:22 ` Cristian Marussi
@ 2019-09-04 16:42 ` Andrey Konovalov
0 siblings, 0 replies; 47+ messages in thread
From: Andrey Konovalov @ 2019-09-04 16:42 UTC (permalink / raw)
To: Cristian Marussi
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx,
dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Kees Cook, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander, Alex Williamson, Leon Romanovsky,
Luc Van Oostenryck, Dave Martin, Khalid Aziz, enh,
Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy
On Wed, Sep 4, 2019 at 6:22 PM Cristian Marussi
<cristian.marussi@arm.com> wrote:
>
> Hi Andrey !
>
> On 04/09/2019 15:52, Andrey Konovalov wrote:
> > On Fri, Aug 23, 2019 at 7:49 PM Cristian Marussi
> > <cristian.marussi@arm.com> wrote:
> >>
> >>
> >> Hi
> >>
> >> On 23/08/2019 18:16, Andrey Konovalov wrote:
> >>> On Fri, Aug 23, 2019 at 3:56 PM Cristian Marussi
> >>> <cristian.marussi@arm.com> wrote:
> >>>>
> >>>> Hi Andrey
> >>>>
> >>>> On 24/06/2019 15:33, Andrey Konovalov wrote:
> >>>>> This patch is a part of a series that extends kernel ABI to allow to pass
> >>>>> tagged user pointers (with the top byte set to something else other than
> >>>>> 0x00) as syscall arguments.
> >>>>>
> >>>>> This patch adds a simple test, that calls the uname syscall with a
> >>>>> tagged user pointer as an argument. Without the kernel accepting tagged
> >>>>> user pointers the test fails with EFAULT.
> >>>>>
> >>>>> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> >>>>> ---
> >>>>> tools/testing/selftests/arm64/.gitignore | 1 +
> >>>>> tools/testing/selftests/arm64/Makefile | 11 +++++++
> >>>>> .../testing/selftests/arm64/run_tags_test.sh | 12 ++++++++
> >>>>> tools/testing/selftests/arm64/tags_test.c | 29 +++++++++++++++++++
> >>>>> 4 files changed, 53 insertions(+)
> >>>>> create mode 100644 tools/testing/selftests/arm64/.gitignore
> >>>>> create mode 100644 tools/testing/selftests/arm64/Makefile
> >>>>> create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh
> >>>>> create mode 100644 tools/testing/selftests/arm64/tags_test.c
> >>>>
> >>>> After building a fresh Kernel from arm64/for-next-core from scratch at:
> >>>>
> >>>> commit 239ab658bea3b387424501e7c416640d6752dc0c
> >>>> Merge: 6bfa3134bd3a 42d038c4fb00 1243cb6a676f d55c5f28afaf d06fa5a118f1 34b5560db40d
> >>>> Author: Will Deacon <will@kernel.org>
> >>>> Date: Thu Aug 22 18:23:53 2019 +0100
> >>>>
> >>>> Merge branches 'for-next/error-injection', 'for-next/tbi', 'for-next/psci-cpuidle', 'for-next/cpu-topology' and 'for-next/52-bit-kva' into for-next/core
> >>>>
> >>>>
> >>>> KSFT arm64 tests build is broken for me, both setting or not KBUILD_OUTPUT=
> >>>>
> >>>> 13:30 $ make TARGETS=arm64 kselftest-clean
> >>>> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> >>>> rm -f -r /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test
> >>>> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> >>>>
> >>>> ✔ ~/ARM/dev/src/pdsw/linux [arm64_for_next_core|…8⚑ 23]
> >>>>
> >>>> 13:30 $ make TARGETS=arm64 kselftest
> >>>> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> >>>> arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built
> >>>> make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \
> >>>> ARCH=arm64 -C ../../.. headers_install
> >>>> HOSTCC scripts/basic/fixdep
> >>>> HOSTCC scripts/unifdef
> >>>> ...
> >>>> ...
> >>>> HDRINST usr/include/asm/msgbuf.h
> >>>> HDRINST usr/include/asm/shmbuf.h
> >>>> INSTALL /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/usr/include
> >>>> /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc tags_test.c -o /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test
> >>>> tags_test.c: In function ‘main’:
> >>>> tags_test.c:21:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first use in this function); did you mean ‘PR_GET_TID_ADDRESS’?
> >>>> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> >>>> ^~~~~~~~~~~~~~~~~~~~~~~
> >>>> PR_GET_TID_ADDRESS
> >>>> tags_test.c:21:12: note: each undeclared identifier is reported only once for each function it appears in
> >>>> tags_test.c:21:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use in this function); did you mean ‘PR_GET_DUMPABLE’?
> >>>> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> >>>> ^~~~~~~~~~~~~~~~~~~~~
> >>>> PR_GET_DUMPABLE
> >>>> ../lib.mk:138: recipe for target '/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test' failed
> >>>> make[3]: *** [/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test] Error 1
> >>>> Makefile:136: recipe for target 'all' failed
> >>>> make[2]: *** [all] Error 2
> >>>> /home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1237: recipe for target 'kselftest' failed
> >>>> make[1]: *** [kselftest] Error 2
> >>>> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> >>>> Makefile:179: recipe for target 'sub-make' failed
> >>>> make: *** [sub-make] Error 2
> >>>>
> >>>> Despite seeing KSFT installing Kernel Headers, they cannot be found.
> >>>>
> >>>> Fixing this patch like this make it work for me:
> >>>>
> >>>> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> >>>> index a61b2e743e99..f9f79fb272f0 100644
> >>>> --- a/tools/testing/selftests/arm64/Makefile
> >>>> +++ b/tools/testing/selftests/arm64/Makefile
> >>>> @@ -4,6 +4,7 @@
> >>>> ARCH ?= $(shell uname -m 2>/dev/null || echo not)
> >>>>
> >>>> ifneq (,$(filter $(ARCH),aarch64 arm64))
> >>>> +CFLAGS += -I../../../../usr/include/
> >>>> TEST_GEN_PROGS := tags_test
> >>>> TEST_PROGS := run_tags_test.sh
> >>>> endif
> >>>>
> >>>> but is not really a proper fix since it does NOT account for case in which you have
> >>>> installed the Kernel Headers in a non standard location like when you use KBUILD_OUTPUT.
> >>>>
> >>>> Am I missing something ?
> >>>
> >>> Hm, PR_SET_TAGGED_ADDR_CTRL is defined in include/uapi/linux/prctl.h,
> >>> and the test has #include <sys/prctl.h> so as long as you've updated
> >>> your kernel headers this should work.
> >>>
> >>> (I'm OOO next week, I'll see if I can reproduce this once I'm back).
> >>
> >> Ok. Thanks for the reply.
> >>
> >> I think I've got it in my local tree having cloned arm64/for-next-core:
> >>
> >> 18:32 $ egrep -A 10 PR_SET_TAG ./include/uapi/linux/prctl.h
> >> #define PR_SET_TAGGED_ADDR_CTRL 55
> >> #define PR_GET_TAGGED_ADDR_CTRL 56
> >> # define PR_TAGGED_ADDR_ENABLE (1UL << 0)
> >>
> >> #endif /* _LINUX_PRCTL_H */
> >>
> >> and Kernel header are locally installed in my kernel src dir (by KSFT indeed)
> >>
> >> 18:34 $ egrep -RA 10 PR_SET_TAG usr/include/
> >> usr/include/linux/prctl.h:#define PR_SET_TAGGED_ADDR_CTRL 55
> >> usr/include/linux/prctl.h-#define PR_GET_TAGGED_ADDR_CTRL 56
> >> usr/include/linux/prctl.h-# define PR_TAGGED_ADDR_ENABLE (1UL << 0)
> >> usr/include/linux/prctl.h-
> >> usr/include/linux/prctl.h-#endif /* _LINUX_PRCTL_H */
> >>
> >> but how are they supposed to be found if nor the test Makefile
> >> neither the KSFT Makefile who installs them pass any -I options to the
> >> compiler ?
> >> I suppose <sys/prctl.h> tries to include arch specific headers from the regular system path,
> >> but when you are cross-compiling ?
> >>
> >> 18:34 $ make TARGETS=arm64 kselftest
> >> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> >> arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built
> >> make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \
> >> ARCH=arm64 -C ../../.. headers_install
> >> INSTALL /home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/usr/include
> >> /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc -Wall -O2 -g tags_test.c -o /home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test
> >> tags_test.c: In function ‘main’:
> >> tags_test.c:20:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first use in this function); did you mean ‘PR_GET_TID_ADDRESS’?
> >> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> >> ^~~~~~~~~~~~~~~~~~~~~~~
> >> PR_GET_TID_ADDRESS
> >> tags_test.c:20:12: note: each undeclared identifier is reported only once for each function it appears in
> >> tags_test.c:20:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use in this function); did you mean ‘PR_GET_DUMPABLE’?
> >> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> >> ^~~~~~~~~~~~~~~~~~~~~
> >> PR_GET_DUMPABLE
> >> ../../lib.mk:138: recipe for target '/home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test' failed
> >> make[4]: *** [/home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test] Error 1
> >> Makefile:19: recipe for target 'all' failed
> >> make[3]: *** [all] Error 2
> >> Makefile:137: recipe for target 'all' failed
> >> make[2]: *** [all] Error 2
> >> /home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1236: recipe for target 'kselftest' failed
> >> make[1]: *** [kselftest] Error 2
> >> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux'
> >> Makefile:179: recipe for target 'sub-make' failed
> >> make: *** [sub-make] Error 2
> >>
> >>
> >> In fact many KSFT testcases seems to brutally add default headers path:
> >>
> >> tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../include/uapi/
> >> tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../include/
> >> tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../usr/include/
> >> tools/testing/selftests/net/Makefile:CFLAGS += -I../../../../usr/include/
> >> tools/testing/selftests/membarrier/Makefile:CFLAGS += -g -I../../../../usr/include/
> >> ...
> >
> > Hi Cristian!
> >
> > Indeed, I can reproduce the issue. I don't know what's the proper way
> > to resolve this. Adding "CFLAGS += -I../../../../usr/include/" looks
> > good to me. AFAICS your series resolves this issue in a similar way,
> > but I think we should fix this before the current rc is released. Do
> > you want to submit a patch that adds this simple fix or should I do
> > that?
> >
>
> Please feel free to post the single line patch above to quickly fix this before
> release, so we don't have a broken build straight away. (our CI is already beating me...:D)
Done.
>
> On my side (01/11) in the meantime I'll fix the top level KSFT arm64 makefile so as to calculate
> and propagate once for all the headers search path down to all KSFT arm64/ in one go,
> trying to guess where they are; this is needed because the above fix works fine as long
> as you don't have KBUILD_OUTPUT set, once you set it, KSFT installs kheaders in a different
> place and the above -I fix is fooled again....but this is a general problem also in other
> KSFT tests as I can see now so I think this fix is good enough for now
> (and the fix on my side, even if trivial, is not going to go into this release for sure)
Ah, I see.
Thanks for the report!
>
> Thanks !
>
> Cheers
>
> Cristian
>
> > Thanks!
> >
> >>
> >> Cheers
> >>
> >> Cristian
> >>>
> >>>
> >>>
> >>>>
> >>>> Thanks
> >>>>
> >>>> Cristian
> >>>>
> >>>>>
> >>>>> diff --git a/tools/testing/selftests/arm64/.gitignore b/tools/testing/selftests/arm64/.gitignore
> >>>>> new file mode 100644
> >>>>> index 000000000000..e8fae8d61ed6
> >>>>> --- /dev/null
> >>>>> +++ b/tools/testing/selftests/arm64/.gitignore
> >>>>> @@ -0,0 +1 @@
> >>>>> +tags_test
> >>>>> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> >>>>> new file mode 100644
> >>>>> index 000000000000..a61b2e743e99
> >>>>> --- /dev/null
> >>>>> +++ b/tools/testing/selftests/arm64/Makefile
> >>>>> @@ -0,0 +1,11 @@
> >>>>> +# SPDX-License-Identifier: GPL-2.0
> >>>>> +
> >>>>> +# ARCH can be overridden by the user for cross compiling
> >>>>> +ARCH ?= $(shell uname -m 2>/dev/null || echo not)
> >>>>> +
> >>>>> +ifneq (,$(filter $(ARCH),aarch64 arm64))
> >>>>> +TEST_GEN_PROGS := tags_test
> >>>>> +TEST_PROGS := run_tags_test.sh
> >>>>> +endif
> >>>>> +
> >>>>> +include ../lib.mk
> >>>>> diff --git a/tools/testing/selftests/arm64/run_tags_test.sh b/tools/testing/selftests/arm64/run_tags_test.sh
> >>>>> new file mode 100755
> >>>>> index 000000000000..745f11379930
> >>>>> --- /dev/null
> >>>>> +++ b/tools/testing/selftests/arm64/run_tags_test.sh
> >>>>> @@ -0,0 +1,12 @@
> >>>>> +#!/bin/sh
> >>>>> +# SPDX-License-Identifier: GPL-2.0
> >>>>> +
> >>>>> +echo "--------------------"
> >>>>> +echo "running tags test"
> >>>>> +echo "--------------------"
> >>>>> +./tags_test
> >>>>> +if [ $? -ne 0 ]; then
> >>>>> + echo "[FAIL]"
> >>>>> +else
> >>>>> + echo "[PASS]"
> >>>>> +fi
> >>>>> diff --git a/tools/testing/selftests/arm64/tags_test.c b/tools/testing/selftests/arm64/tags_test.c
> >>>>> new file mode 100644
> >>>>> index 000000000000..22a1b266e373
> >>>>> --- /dev/null
> >>>>> +++ b/tools/testing/selftests/arm64/tags_test.c
> >>>>> @@ -0,0 +1,29 @@
> >>>>> +// SPDX-License-Identifier: GPL-2.0
> >>>>> +
> >>>>> +#include <stdio.h>
> >>>>> +#include <stdlib.h>
> >>>>> +#include <unistd.h>
> >>>>> +#include <stdint.h>
> >>>>> +#include <sys/prctl.h>
> >>>>> +#include <sys/utsname.h>
> >>>>> +
> >>>>> +#define SHIFT_TAG(tag) ((uint64_t)(tag) << 56)
> >>>>> +#define SET_TAG(ptr, tag) (((uint64_t)(ptr) & ~SHIFT_TAG(0xff)) | \
> >>>>> + SHIFT_TAG(tag))
> >>>>> +
> >>>>> +int main(void)
> >>>>> +{
> >>>>> + static int tbi_enabled = 0;
> >>>>> + struct utsname *ptr, *tagged_ptr;
> >>>>> + int err;
> >>>>> +
> >>>>> + if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> >>>>> + tbi_enabled = 1;
> >>>>> + ptr = (struct utsname *)malloc(sizeof(*ptr));
> >>>>> + if (tbi_enabled)
> >>>>> + tagged_ptr = (struct utsname *)SET_TAG(ptr, 0x42);
> >>>>> + err = uname(tagged_ptr);
> >>>>> + free(ptr);
> >>>>> +
> >>>>> + return err;
> >>>>> +}
> >>>>>
> >>>>
> >>
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v18 00/15] arm64: untag user pointers passed to the kernel
2019-06-24 14:32 [PATCH v18 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
` (14 preceding siblings ...)
2019-06-24 14:33 ` [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel Andrey Konovalov
@ 2019-06-26 17:18 ` Catalin Marinas
15 siblings, 0 replies; 47+ messages in thread
From: Catalin Marinas @ 2019-06-26 17:18 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Konovalov, linux-arm-kernel, linux-mm, linux-kernel,
amd-gfx, dri-devel, linux-rdma, linux-media, kvm,
linux-kselftest, Vincenzo Frascino, Will Deacon, Mark Rutland,
Greg Kroah-Hartman, Kees Cook, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander, Alex Williamson, Leon Romanovsky,
Luc Van Oostenryck, Dave Martin, Khalid Aziz, enh,
Jason Gunthorpe, Christoph Hellwig, Dmitry Vyukov,
Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
Robin Murphy, Kevin Brodsky, Szabolcs Nagy
Hi Andrew,
On Mon, Jun 24, 2019 at 04:32:45PM +0200, Andrey Konovalov wrote:
> Andrey Konovalov (14):
> arm64: untag user pointers in access_ok and __uaccess_mask_ptr
> lib: untag user pointers in strn*_user
> mm: untag user pointers passed to memory syscalls
> mm: untag user pointers in mm/gup.c
> mm: untag user pointers in get_vaddr_frames
> fs/namespace: untag user pointers in copy_mount_options
> userfaultfd: untag user pointers
> drm/amdgpu: untag user pointers
> drm/radeon: untag user pointers in radeon_gem_userptr_ioctl
> IB/mlx4: untag user pointers in mlx4_get_umem_mr
> media/v4l2-core: untag user pointers in videobuf_dma_contig_user_get
> tee/shm: untag user pointers in tee_shm_register
> vfio/type1: untag user pointers in vaddr_get_pfn
> selftests, arm64: add a selftest for passing tagged pointers to kernel
>
> Catalin Marinas (1):
> arm64: Introduce prctl() options to control the tagged user addresses
> ABI
>
> arch/arm64/Kconfig | 9 +++
> arch/arm64/include/asm/processor.h | 8 +++
> arch/arm64/include/asm/thread_info.h | 1 +
> arch/arm64/include/asm/uaccess.h | 12 +++-
> arch/arm64/kernel/process.c | 72 +++++++++++++++++++
> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +
> drivers/gpu/drm/radeon/radeon_gem.c | 2 +
> drivers/infiniband/hw/mlx4/mr.c | 7 +-
> drivers/media/v4l2-core/videobuf-dma-contig.c | 9 +--
> drivers/tee/tee_shm.c | 1 +
> drivers/vfio/vfio_iommu_type1.c | 2 +
> fs/namespace.c | 2 +-
> fs/userfaultfd.c | 22 +++---
> include/uapi/linux/prctl.h | 5 ++
> kernel/sys.c | 12 ++++
> lib/strncpy_from_user.c | 3 +-
> lib/strnlen_user.c | 3 +-
> mm/frame_vector.c | 2 +
> mm/gup.c | 4 ++
> mm/madvise.c | 2 +
> mm/mempolicy.c | 3 +
> mm/migrate.c | 2 +-
> mm/mincore.c | 2 +
> mm/mlock.c | 4 ++
> mm/mprotect.c | 2 +
> mm/mremap.c | 7 ++
> mm/msync.c | 2 +
> tools/testing/selftests/arm64/.gitignore | 1 +
> tools/testing/selftests/arm64/Makefile | 11 +++
> .../testing/selftests/arm64/run_tags_test.sh | 12 ++++
> tools/testing/selftests/arm64/tags_test.c | 29 ++++++++
> 32 files changed, 232 insertions(+), 25 deletions(-)
It looks like we got to an agreement on how to deal with tagged user
addresses between SPARC ADI and ARM Memory Tagging. If there are no
other objections, what's your preferred way of getting this series into
-next first and then mainline? Are you ok to merge them into the mm
tree?
Thanks.
--
Catalin
^ permalink raw reply [flat|nested] 47+ messages in thread