From: Laura Abbott <labbott@redhat.com> To: kernel-hardening@lists.openwall.com Cc: Laura Abbott <labbott@redhat.com>, linux-kernel@vger.kernel.org, Mark Rutland <mark.rutland@arm.com>, Kees Cook <keescook@chromium.org>, x86@kernel.org Subject: [RFC PATCH 1/2] x86: Avoid multiple evaluations in __{get,put}_user_size Date: Fri, 3 Nov 2017 16:04:25 -0700 [thread overview] Message-ID: <20171103230426.19114-1-labbott@redhat.com> (raw) In-Reply-To: <20171026090942.7041-1-mark.rutland@arm.com> Currently __{get,put}_user_size() expand their ptr argument in several places, and some callers pass in expressions with side effects. For example, fs/binfmt_elf.c, passes sp++ as the ptr argument to a chain of __put_user() calls. So far this isn't a problem, as each of these uses is mutually exclusive. However, in subsequent patches we will need to make use of the ptr argument several times, and ensure that we evaluate the ptr expression exactly once to avoid corrupting the pointer. In preparation for such uses, this patch reorganises __{get,put}_user_size to evaluate the ptr argument into a temporary __{gu,pu}_ptr variable, ensuring that side-effects occur exaclty once. There should be no functional change as a result of this patch. Based on work done for arm64 by Mark Rutland. Signed-off-by: Laura Abbott <labbott@redhat.com> --- This is setup patch for checking __{get,put}_user on x86 based on Mark Rutland's work for arm64 lkml.kernel.org/r/<20171026090942.7041-1-mark.rutland@arm.com> --- arch/x86/include/asm/uaccess.h | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h index 574dff4d2913..d23fb5844404 100644 --- a/arch/x86/include/asm/uaccess.h +++ b/arch/x86/include/asm/uaccess.h @@ -275,21 +275,25 @@ extern void __put_user_8(void); #define __put_user_size(x, ptr, size, retval, errret) \ do { \ + typeof(ptr) __pu_ptr = (ptr); \ retval = 0; \ - __chk_user_ptr(ptr); \ + __chk_user_ptr(__pu_ptr); \ switch (size) { \ case 1: \ - __put_user_asm(x, ptr, retval, "b", "b", "iq", errret); \ + __put_user_asm(x, __pu_ptr, retval, "b", "b", "iq", \ + errret); \ break; \ case 2: \ - __put_user_asm(x, ptr, retval, "w", "w", "ir", errret); \ + __put_user_asm(x, __pu_ptr, retval, "w", "w", "ir", \ + errret); \ break; \ case 4: \ - __put_user_asm(x, ptr, retval, "l", "k", "ir", errret); \ + __put_user_asm(x, __pu_ptr, retval, "l", "k", "ir", \ + errret); \ break; \ case 8: \ - __put_user_asm_u64((__typeof__(*ptr))(x), ptr, retval, \ - errret); \ + __put_user_asm_u64((__typeof__(*ptr))(x), __pu_ptr, \ + retval, \ errret); \ break; \ default: \ __put_user_bad(); \ @@ -352,20 +356,24 @@ do { \ #define __get_user_size(x, ptr, size, retval, errret) \ do { \ + typeof(ptr) __gu_ptr = (ptr); \ retval = 0; \ - __chk_user_ptr(ptr); \ + __chk_user_ptr(__gu_ptr); \ switch (size) { \ case 1: \ - __get_user_asm(x, ptr, retval, "b", "b", "=q", errret); \ + __get_user_asm(x, __gu_ptr, retval, "b", "b", "=q", \ + errret); \ break; \ case 2: \ - __get_user_asm(x, ptr, retval, "w", "w", "=r", errret); \ + __get_user_asm(x, __gu_ptr, retval, "w", "w", "=r", \ + errret); \ break; \ case 4: \ - __get_user_asm(x, ptr, retval, "l", "k", "=r", errret); \ + __get_user_asm(x, __gu_ptr, retval, "l", "k", "=r", \ + errret); \ break; \ case 8: \ - __get_user_asm_u64(x, ptr, retval, errret); \ + __get_user_asm_u64(x, __gu_ptr, retval, errret); \ break; \ default: \ (x) = __get_user_bad(); \ -- 2.13.5
WARNING: multiple messages have this Message-ID (diff)
From: Laura Abbott <labbott@redhat.com> To: kernel-hardening@lists.openwall.com Cc: Laura Abbott <labbott@redhat.com>, linux-kernel@vger.kernel.org, Mark Rutland <mark.rutland@arm.com>, Kees Cook <keescook@chromium.org>, x86@kernel.org Subject: [kernel-hardening] [RFC PATCH 1/2] x86: Avoid multiple evaluations in __{get,put}_user_size Date: Fri, 3 Nov 2017 16:04:25 -0700 [thread overview] Message-ID: <20171103230426.19114-1-labbott@redhat.com> (raw) In-Reply-To: <20171026090942.7041-1-mark.rutland@arm.com> Currently __{get,put}_user_size() expand their ptr argument in several places, and some callers pass in expressions with side effects. For example, fs/binfmt_elf.c, passes sp++ as the ptr argument to a chain of __put_user() calls. So far this isn't a problem, as each of these uses is mutually exclusive. However, in subsequent patches we will need to make use of the ptr argument several times, and ensure that we evaluate the ptr expression exactly once to avoid corrupting the pointer. In preparation for such uses, this patch reorganises __{get,put}_user_size to evaluate the ptr argument into a temporary __{gu,pu}_ptr variable, ensuring that side-effects occur exaclty once. There should be no functional change as a result of this patch. Based on work done for arm64 by Mark Rutland. Signed-off-by: Laura Abbott <labbott@redhat.com> --- This is setup patch for checking __{get,put}_user on x86 based on Mark Rutland's work for arm64 lkml.kernel.org/r/<20171026090942.7041-1-mark.rutland@arm.com> --- arch/x86/include/asm/uaccess.h | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h index 574dff4d2913..d23fb5844404 100644 --- a/arch/x86/include/asm/uaccess.h +++ b/arch/x86/include/asm/uaccess.h @@ -275,21 +275,25 @@ extern void __put_user_8(void); #define __put_user_size(x, ptr, size, retval, errret) \ do { \ + typeof(ptr) __pu_ptr = (ptr); \ retval = 0; \ - __chk_user_ptr(ptr); \ + __chk_user_ptr(__pu_ptr); \ switch (size) { \ case 1: \ - __put_user_asm(x, ptr, retval, "b", "b", "iq", errret); \ + __put_user_asm(x, __pu_ptr, retval, "b", "b", "iq", \ + errret); \ break; \ case 2: \ - __put_user_asm(x, ptr, retval, "w", "w", "ir", errret); \ + __put_user_asm(x, __pu_ptr, retval, "w", "w", "ir", \ + errret); \ break; \ case 4: \ - __put_user_asm(x, ptr, retval, "l", "k", "ir", errret); \ + __put_user_asm(x, __pu_ptr, retval, "l", "k", "ir", \ + errret); \ break; \ case 8: \ - __put_user_asm_u64((__typeof__(*ptr))(x), ptr, retval, \ - errret); \ + __put_user_asm_u64((__typeof__(*ptr))(x), __pu_ptr, \ + retval, \ errret); \ break; \ default: \ __put_user_bad(); \ @@ -352,20 +356,24 @@ do { \ #define __get_user_size(x, ptr, size, retval, errret) \ do { \ + typeof(ptr) __gu_ptr = (ptr); \ retval = 0; \ - __chk_user_ptr(ptr); \ + __chk_user_ptr(__gu_ptr); \ switch (size) { \ case 1: \ - __get_user_asm(x, ptr, retval, "b", "b", "=q", errret); \ + __get_user_asm(x, __gu_ptr, retval, "b", "b", "=q", \ + errret); \ break; \ case 2: \ - __get_user_asm(x, ptr, retval, "w", "w", "=r", errret); \ + __get_user_asm(x, __gu_ptr, retval, "w", "w", "=r", \ + errret); \ break; \ case 4: \ - __get_user_asm(x, ptr, retval, "l", "k", "=r", errret); \ + __get_user_asm(x, __gu_ptr, retval, "l", "k", "=r", \ + errret); \ break; \ case 8: \ - __get_user_asm_u64(x, ptr, retval, errret); \ + __get_user_asm_u64(x, __gu_ptr, retval, errret); \ break; \ default: \ (x) = __get_user_bad(); \ -- 2.13.5
next prev parent reply other threads:[~2017-11-03 23:04 UTC|newest] Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-10-26 9:09 [RFC PATCH 0/2] arm64: optional paranoid __{get,put}_user checks Mark Rutland 2017-10-26 9:09 ` [kernel-hardening] " Mark Rutland 2017-10-26 9:09 ` Mark Rutland 2017-10-26 9:09 ` [RFC PATCH 1/2] arm64: write __range_ok() in C Mark Rutland 2017-10-26 9:09 ` [kernel-hardening] " Mark Rutland 2017-10-26 9:09 ` Mark Rutland 2017-11-16 15:28 ` Will Deacon 2017-11-16 15:28 ` [kernel-hardening] " Will Deacon 2017-11-16 15:28 ` Will Deacon 2017-11-20 12:22 ` Mark Rutland 2017-11-20 12:22 ` [kernel-hardening] " Mark Rutland 2017-11-20 12:22 ` Mark Rutland 2017-10-26 9:09 ` [RFC PATCH 2/2] arm64: allow paranoid __{get,put}user Mark Rutland 2017-10-26 9:09 ` [kernel-hardening] " Mark Rutland 2017-10-26 9:09 ` Mark Rutland 2017-10-27 15:41 ` [RFC PATCH 0/2] arm64: optional paranoid __{get,put}_user checks Will Deacon 2017-10-27 15:41 ` [kernel-hardening] " Will Deacon 2017-10-27 15:41 ` Will Deacon 2017-10-27 20:44 ` Mark Rutland 2017-10-27 20:44 ` [kernel-hardening] " Mark Rutland 2017-10-27 20:44 ` Mark Rutland 2017-10-28 8:47 ` Russell King - ARM Linux 2017-10-28 8:47 ` [kernel-hardening] " Russell King - ARM Linux 2017-10-28 8:47 ` Russell King - ARM Linux 2017-10-31 23:56 ` Laura Abbott 2017-10-31 23:56 ` [kernel-hardening] " Laura Abbott 2017-10-31 23:56 ` Laura Abbott 2017-11-01 12:05 ` Mark Rutland 2017-11-01 12:05 ` [kernel-hardening] " Mark Rutland 2017-11-01 12:05 ` Mark Rutland 2017-11-01 21:13 ` Laura Abbott 2017-11-01 21:13 ` [kernel-hardening] " Laura Abbott 2017-11-01 21:13 ` Laura Abbott 2017-11-01 22:28 ` Kees Cook 2017-11-01 22:28 ` [kernel-hardening] " Kees Cook 2017-11-01 22:28 ` Kees Cook 2017-11-01 23:05 ` Laura Abbott 2017-11-01 23:05 ` [kernel-hardening] " Laura Abbott 2017-11-01 23:05 ` Laura Abbott 2017-11-01 23:29 ` Kees Cook 2017-11-01 23:29 ` [kernel-hardening] " Kees Cook 2017-11-01 23:29 ` Kees Cook 2017-11-02 1:25 ` Laura Abbott 2017-11-02 1:25 ` [kernel-hardening] " Laura Abbott 2017-11-02 1:25 ` Laura Abbott 2017-11-03 23:04 ` Laura Abbott [this message] 2017-11-03 23:04 ` [kernel-hardening] [RFC PATCH 1/2] x86: Avoid multiple evaluations in __{get,put}_user_size Laura Abbott 2017-11-03 23:04 ` [RFC PATCH 2/2] x86: Allow paranoid __{get,put}_user Laura Abbott 2017-11-03 23:04 ` [kernel-hardening] " Laura Abbott 2017-11-04 0:14 ` Kees Cook 2017-11-04 0:14 ` [kernel-hardening] " Kees Cook 2017-11-04 0:24 ` Al Viro 2017-11-04 0:24 ` [kernel-hardening] " Al Viro 2017-11-04 0:44 ` Al Viro 2017-11-04 0:44 ` [kernel-hardening] " Al Viro 2017-11-04 1:39 ` Kees Cook 2017-11-04 1:39 ` [kernel-hardening] " Kees Cook 2017-11-04 1:41 ` Kees Cook 2017-11-04 1:41 ` [kernel-hardening] " Kees Cook 2017-11-04 1:58 ` Mark Rutland 2017-11-04 1:58 ` [kernel-hardening] " Mark Rutland 2017-11-06 20:38 ` Laura Abbott 2017-11-06 20:38 ` [kernel-hardening] " Laura Abbott
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20171103230426.19114-1-labbott@redhat.com \ --to=labbott@redhat.com \ --cc=keescook@chromium.org \ --cc=kernel-hardening@lists.openwall.com \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.