* [PATCH 0/2] objtool uaccess fixes @ 2019-04-24 7:19 Peter Zijlstra 2019-04-24 7:19 ` [PATCH 1/2] x86/uaccess: Dont leak the AC flag into __put_user() argument evaluation Peter Zijlstra 2019-04-24 7:19 ` [PATCH 2/2] mm/uaccess: Use unsigned long Peter Zijlstra 0 siblings, 2 replies; 5+ messages in thread From: Peter Zijlstra @ 2019-04-24 7:19 UTC (permalink / raw) To: mingo, tglx, x86; +Cc: linux-kernel, luto, peterz Hi, Here the two patches required to build a clean x86_64-allyesconfig with older GCC (<8). ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] x86/uaccess: Dont leak the AC flag into __put_user() argument evaluation 2019-04-24 7:19 [PATCH 0/2] objtool uaccess fixes Peter Zijlstra @ 2019-04-24 7:19 ` Peter Zijlstra 2019-04-24 11:10 ` [tip:core/objtool] " tip-bot for Peter Zijlstra 2019-04-24 7:19 ` [PATCH 2/2] mm/uaccess: Use unsigned long Peter Zijlstra 1 sibling, 1 reply; 5+ messages in thread From: Peter Zijlstra @ 2019-04-24 7:19 UTC (permalink / raw) To: mingo, tglx, x86; +Cc: linux-kernel, luto, peterz, Randy Dunlap, Linus Torvalds The __put_user() macro evaluates it's @ptr argument inside the __uaccess_begin() / __uaccess_end() region. While this would normally not be expected to be an issue, an UBSAN bug (it ignored -fwrapv, fixed in GCC 8+) would transform the @ptr evaluation for: drivers/gpu/drm/i915/i915_gem_execbuffer.c: if (unlikely(__put_user(offset, &urelocs[r-stack].presumed_offset))) { into a signed-overflow-UB check and trigger the objtool AC validation. Finish commit 2a418cf3f5f1 ("x86/uaccess: Don't leak the AC flag into __put_user() value evaluation") and explicitly evaluate all 3 argument early. Reported-by: Randy Dunlap <rdunlap@infradead.org> Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Fixes: 2a418cf3f5f1 ("x86/uaccess: Don't leak the AC flag into __put_user() value evaluation") Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- arch/x86/include/asm/uaccess.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) --- a/arch/x86/include/asm/uaccess.h +++ b/arch/x86/include/asm/uaccess.h @@ -427,10 +427,11 @@ do { \ ({ \ __label__ __pu_label; \ int __pu_err = -EFAULT; \ - __typeof__(*(ptr)) __pu_val; \ - __pu_val = x; \ + __typeof__(*(ptr)) __pu_val = (x); \ + __typeof__(ptr) __pu_ptr = (ptr); \ + __typeof__(size) __pu_size = (size); \ __uaccess_begin(); \ - __put_user_size(__pu_val, (ptr), (size), __pu_label); \ + __put_user_size(__pu_val, __pu_ptr, __pu_size, __pu_label); \ __pu_err = 0; \ __pu_label: \ __uaccess_end(); \ ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:core/objtool] x86/uaccess: Dont leak the AC flag into __put_user() argument evaluation 2019-04-24 7:19 ` [PATCH 1/2] x86/uaccess: Dont leak the AC flag into __put_user() argument evaluation Peter Zijlstra @ 2019-04-24 11:10 ` tip-bot for Peter Zijlstra 0 siblings, 0 replies; 5+ messages in thread From: tip-bot for Peter Zijlstra @ 2019-04-24 11:10 UTC (permalink / raw) To: linux-tip-commits Cc: rdunlap, mingo, torvalds, linux-kernel, peterz, hpa, tglx Commit-ID: 6ae865615fc43d014da2fd1f1bba7e81ee622d1b Gitweb: https://git.kernel.org/tip/6ae865615fc43d014da2fd1f1bba7e81ee622d1b Author: Peter Zijlstra <peterz@infradead.org> AuthorDate: Wed, 24 Apr 2019 09:19:24 +0200 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Wed, 24 Apr 2019 12:19:45 +0200 x86/uaccess: Dont leak the AC flag into __put_user() argument evaluation The __put_user() macro evaluates it's @ptr argument inside the __uaccess_begin() / __uaccess_end() region. While this would normally not be expected to be an issue, an UBSAN bug (it ignored -fwrapv, fixed in GCC 8+) would transform the @ptr evaluation for: drivers/gpu/drm/i915/i915_gem_execbuffer.c: if (unlikely(__put_user(offset, &urelocs[r-stack].presumed_offset))) { into a signed-overflow-UB check and trigger the objtool AC validation. Finish this commit: 2a418cf3f5f1 ("x86/uaccess: Don't leak the AC flag into __put_user() value evaluation") and explicitly evaluate all 3 arguments early. Reported-by: Randy Dunlap <rdunlap@infradead.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: luto@kernel.org Fixes: 2a418cf3f5f1 ("x86/uaccess: Don't leak the AC flag into __put_user() value evaluation") Link: http://lkml.kernel.org/r/20190424072208.695962771@infradead.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/include/asm/uaccess.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h index 5ca7b91faf67..bb21913885a3 100644 --- a/arch/x86/include/asm/uaccess.h +++ b/arch/x86/include/asm/uaccess.h @@ -427,10 +427,11 @@ do { \ ({ \ __label__ __pu_label; \ int __pu_err = -EFAULT; \ - __typeof__(*(ptr)) __pu_val; \ - __pu_val = x; \ + __typeof__(*(ptr)) __pu_val = (x); \ + __typeof__(ptr) __pu_ptr = (ptr); \ + __typeof__(size) __pu_size = (size); \ __uaccess_begin(); \ - __put_user_size(__pu_val, (ptr), (size), __pu_label); \ + __put_user_size(__pu_val, __pu_ptr, __pu_size, __pu_label); \ __pu_err = 0; \ __pu_label: \ __uaccess_end(); \ ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] mm/uaccess: Use unsigned long 2019-04-24 7:19 [PATCH 0/2] objtool uaccess fixes Peter Zijlstra 2019-04-24 7:19 ` [PATCH 1/2] x86/uaccess: Dont leak the AC flag into __put_user() argument evaluation Peter Zijlstra @ 2019-04-24 7:19 ` Peter Zijlstra 2019-04-24 11:10 ` [tip:core/objtool] mm/uaccess: Use 'unsigned long' to placate UBSAN warnings on older GCC versions tip-bot for Peter Zijlstra 1 sibling, 1 reply; 5+ messages in thread From: Peter Zijlstra @ 2019-04-24 7:19 UTC (permalink / raw) To: mingo, tglx, x86; +Cc: linux-kernel, luto, peterz, Randy Dunlap, Linus Torvalds Randy reported objtool triggered on his (GCC-7.4) build: lib/strncpy_from_user.o: warning: objtool: strncpy_from_user()+0x315: call to __ubsan_handle_add_overflow() with UACCESS enabled lib/strnlen_user.o: warning: objtool: strnlen_user()+0x337: call to __ubsan_handle_sub_overflow() with UACCESS enabled This is due to UBSAN generating signed-overflow-UB warnings where it should not. Prior to GCC-8 UBSAN ignored -fwrapv (which the kernel uses through -fno-strict-overflow). Make the functions use 'unsigned long' throughout. Reported-by: Randy Dunlap <rdunlap@infradead.org> Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- lib/strncpy_from_user.c | 5 +++-- lib/strnlen_user.c | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) --- a/lib/strncpy_from_user.c +++ b/lib/strncpy_from_user.c @@ -23,10 +23,11 @@ * hit it), 'max' is the address space maximum (and we return * -EFAULT if we hit it). */ -static inline long do_strncpy_from_user(char *dst, const char __user *src, long count, unsigned long max) +static inline long do_strncpy_from_user(char *dst, const char __user *src, + unsigned long count, unsigned long max) { const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS; - long res = 0; + unsigned long res = 0; /* * Truncate 'max' to the user-specified limit, so that --- a/lib/strnlen_user.c +++ b/lib/strnlen_user.c @@ -28,7 +28,7 @@ static inline long do_strnlen_user(const char __user *src, unsigned long count, unsigned long max) { const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS; - long align, res = 0; + unsigned long align, res = 0; unsigned long c; /* @@ -42,7 +42,7 @@ static inline long do_strnlen_user(const * Do everything aligned. But that means that we * need to also expand the maximum.. */ - align = (sizeof(long) - 1) & (unsigned long)src; + align = (sizeof(unsigned long) - 1) & (unsigned long)src; src -= align; max += align; ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:core/objtool] mm/uaccess: Use 'unsigned long' to placate UBSAN warnings on older GCC versions 2019-04-24 7:19 ` [PATCH 2/2] mm/uaccess: Use unsigned long Peter Zijlstra @ 2019-04-24 11:10 ` tip-bot for Peter Zijlstra 0 siblings, 0 replies; 5+ messages in thread From: tip-bot for Peter Zijlstra @ 2019-04-24 11:10 UTC (permalink / raw) To: linux-tip-commits Cc: rdunlap, peterz, linux-kernel, torvalds, hpa, tglx, mingo Commit-ID: 29da93fea3ea39ab9b12270cc6be1b70ef201c9e Gitweb: https://git.kernel.org/tip/29da93fea3ea39ab9b12270cc6be1b70ef201c9e Author: Peter Zijlstra <peterz@infradead.org> AuthorDate: Wed, 24 Apr 2019 09:19:25 +0200 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Wed, 24 Apr 2019 12:19:45 +0200 mm/uaccess: Use 'unsigned long' to placate UBSAN warnings on older GCC versions Randy reported objtool triggered on his (GCC-7.4) build: lib/strncpy_from_user.o: warning: objtool: strncpy_from_user()+0x315: call to __ubsan_handle_add_overflow() with UACCESS enabled lib/strnlen_user.o: warning: objtool: strnlen_user()+0x337: call to __ubsan_handle_sub_overflow() with UACCESS enabled This is due to UBSAN generating signed-overflow-UB warnings where it should not. Prior to GCC-8 UBSAN ignored -fwrapv (which the kernel uses through -fno-strict-overflow). Make the functions use 'unsigned long' throughout. Reported-by: Randy Dunlap <rdunlap@infradead.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: luto@kernel.org Link: http://lkml.kernel.org/r/20190424072208.754094071@infradead.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- lib/strncpy_from_user.c | 5 +++-- lib/strnlen_user.c | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c index 58eacd41526c..023ba9f3b99f 100644 --- a/lib/strncpy_from_user.c +++ b/lib/strncpy_from_user.c @@ -23,10 +23,11 @@ * hit it), 'max' is the address space maximum (and we return * -EFAULT if we hit it). */ -static inline long do_strncpy_from_user(char *dst, const char __user *src, long count, unsigned long max) +static inline long do_strncpy_from_user(char *dst, const char __user *src, + unsigned long count, unsigned long max) { const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS; - long res = 0; + unsigned long res = 0; /* * Truncate 'max' to the user-specified limit, so that diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c index 1c1a1b0e38a5..7f2db3fe311f 100644 --- a/lib/strnlen_user.c +++ b/lib/strnlen_user.c @@ -28,7 +28,7 @@ static inline long do_strnlen_user(const char __user *src, unsigned long count, unsigned long max) { const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS; - long align, res = 0; + unsigned long align, res = 0; unsigned long c; /* @@ -42,7 +42,7 @@ static inline long do_strnlen_user(const char __user *src, unsigned long count, * Do everything aligned. But that means that we * need to also expand the maximum.. */ - align = (sizeof(long) - 1) & (unsigned long)src; + align = (sizeof(unsigned long) - 1) & (unsigned long)src; src -= align; max += align; ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-24 11:11 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-24 7:19 [PATCH 0/2] objtool uaccess fixes Peter Zijlstra 2019-04-24 7:19 ` [PATCH 1/2] x86/uaccess: Dont leak the AC flag into __put_user() argument evaluation Peter Zijlstra 2019-04-24 11:10 ` [tip:core/objtool] " tip-bot for Peter Zijlstra 2019-04-24 7:19 ` [PATCH 2/2] mm/uaccess: Use unsigned long Peter Zijlstra 2019-04-24 11:10 ` [tip:core/objtool] mm/uaccess: Use 'unsigned long' to placate UBSAN warnings on older GCC versions tip-bot for Peter Zijlstra
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.