* [PATCH v2 0/2] locking/refcounts, x86/asm: Use unique .text section for refcount exceptions @ 2017-09-02 20:09 Kees Cook 2017-09-02 20:09 ` [PATCH v2 1/2] " Kees Cook 2017-09-02 20:09 ` [PATCH v2 2/2] locking/refcounts, x86/asm: Enable CONFIG_ARCH_HAS_REFCOUNT Kees Cook 0 siblings, 2 replies; 5+ messages in thread From: Kees Cook @ 2017-09-02 20:09 UTC (permalink / raw) To: Ingo Molnar Cc: Kees Cook, Mike Galbraith, x86, linux-arch, LKML, Reshetova, Elena, Peter Zijlstra, Ard Biesheuvel This splits the earlier patch to have the Kconfig enablement separate, as requested by Ingo. Thanks! -Kees ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] locking/refcounts, x86/asm: Use unique .text section for refcount exceptions 2017-09-02 20:09 [PATCH v2 0/2] locking/refcounts, x86/asm: Use unique .text section for refcount exceptions Kees Cook @ 2017-09-02 20:09 ` Kees Cook 2017-09-28 10:58 ` [tip:locking/core] " tip-bot for Kees Cook 2017-09-02 20:09 ` [PATCH v2 2/2] locking/refcounts, x86/asm: Enable CONFIG_ARCH_HAS_REFCOUNT Kees Cook 1 sibling, 1 reply; 5+ messages in thread From: Kees Cook @ 2017-09-02 20:09 UTC (permalink / raw) To: Ingo Molnar Cc: Kees Cook, Mike Galbraith, x86, linux-arch, LKML, Reshetova, Elena, Peter Zijlstra, Ard Biesheuvel Using .text.unlikely for refcount exceptions isn't safe because gcc may move entire functions into .text.unlikely (e.g. in6_dev_dev()), which would cause any uses of a protected refcount_t function to stay inline with the function, triggering the protection unconditionally: .section .text.unlikely,"ax",@progbits .type in6_dev_get, @function in6_dev_getx: .LFB4673: .loc 2 4128 0 .cfi_startproc ... lock; incl 480(%rbx) js 111f .pushsection .text.unlikely 111: lea 480(%rbx), %rcx 112: .byte 0x0f, 0xff .popsection 113: This creates a unique .text..refcount section and adds an additional test to the exception handler to WARN in the case of having none of OF, SF, nor ZF set so we can see things like this more easily in the future. The double dot for the section name keeps it out of the TEXT_MAIN macro namespace (see commit cb87481ee89db ("kbuild: linker script do not match C names unless LD_DEAD_CODE_DATA_ELIMINATION is configured"), which matches C names: [a-zA-Z0-9_] but not ".") to avoid collisions and so it can be put at the end with text.unlikely to keep the cold code together. Reported-by: Mike Galbraith <efault@gmx.de> Fixes: 7a46ec0e2f48 ("locking/refcounts, x86/asm: Implement fast refcount overflow protection") Signed-off-by: Kees Cook <keescook@chromium.org> --- This will apply to -tip with fuzz, but since the TEXT_MAIN delta is in -next and is targeted for stable, it seemed best to diff against -next. --- arch/x86/include/asm/refcount.h | 2 +- arch/x86/mm/extable.c | 7 ++++++- include/asm-generic/vmlinux.lds.h | 1 + 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/refcount.h b/arch/x86/include/asm/refcount.h index ff871210b9f2..4e44250e7d0d 100644 --- a/arch/x86/include/asm/refcount.h +++ b/arch/x86/include/asm/refcount.h @@ -15,7 +15,7 @@ * back to the regular execution flow in .text. */ #define _REFCOUNT_EXCEPTION \ - ".pushsection .text.unlikely\n" \ + ".pushsection .text..refcount\n" \ "111:\tlea %[counter], %%" _ASM_CX "\n" \ "112:\t" ASM_UD0 "\n" \ ASM_UNREACHABLE \ diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c index c076f710de4c..cf0d74b47ae0 100644 --- a/arch/x86/mm/extable.c +++ b/arch/x86/mm/extable.c @@ -66,12 +66,17 @@ bool ex_handler_refcount(const struct exception_table_entry *fixup, * wrapped around) will be set. Additionally, seeing the refcount * reach 0 will set ZF (Zero Flag: result was zero). In each of * these cases we want a report, since it's a boundary condition. - * + * The SF case is not reported since it indicates post-boundary + * manipulations below zero or above INT_MAX. And if none of the + * flags are set, something has gone very wrong, so report it. */ if (regs->flags & (X86_EFLAGS_OF | X86_EFLAGS_ZF)) { bool zero = regs->flags & X86_EFLAGS_ZF; refcount_error_report(regs, zero ? "hit zero" : "overflow"); + } else if ((regs->flags & X86_EFLAGS_SF) == 0) { + /* Report if none of OF, ZF, nor SF are set. */ + refcount_error_report(regs, "unexpected saturation"); } return true; diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index 8acfc1e099e1..e549bff87c5b 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -459,6 +459,7 @@ #define TEXT_TEXT \ ALIGN_FUNCTION(); \ *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ + *(.text..refcount) \ *(.ref.text) \ MEM_KEEP(init.text) \ MEM_KEEP(exit.text) \ -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:locking/core] locking/refcounts, x86/asm: Use unique .text section for refcount exceptions 2017-09-02 20:09 ` [PATCH v2 1/2] " Kees Cook @ 2017-09-28 10:58 ` tip-bot for Kees Cook 0 siblings, 0 replies; 5+ messages in thread From: tip-bot for Kees Cook @ 2017-09-28 10:58 UTC (permalink / raw) To: linux-tip-commits Cc: torvalds, tglx, ard.biesheuvel, keescook, hpa, linux-kernel, elena.reshetova, peterz, linux-arch, mingo, efault Commit-ID: 564c9cc84e2adf8a6671c1937f0a9fe3da2a4b0e Gitweb: https://git.kernel.org/tip/564c9cc84e2adf8a6671c1937f0a9fe3da2a4b0e Author: Kees Cook <keescook@chromium.org> AuthorDate: Sat, 2 Sep 2017 13:09:45 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Thu, 28 Sep 2017 09:45:05 +0200 locking/refcounts, x86/asm: Use unique .text section for refcount exceptions Using .text.unlikely for refcount exceptions isn't safe because gcc may move entire functions into .text.unlikely (e.g. in6_dev_dev()), which would cause any uses of a protected refcount_t function to stay inline with the function, triggering the protection unconditionally: .section .text.unlikely,"ax",@progbits .type in6_dev_get, @function in6_dev_getx: .LFB4673: .loc 2 4128 0 .cfi_startproc ... lock; incl 480(%rbx) js 111f .pushsection .text.unlikely 111: lea 480(%rbx), %rcx 112: .byte 0x0f, 0xff .popsection 113: This creates a unique .text..refcount section and adds an additional test to the exception handler to WARN in the case of having none of OF, SF, nor ZF set so we can see things like this more easily in the future. The double dot for the section name keeps it out of the TEXT_MAIN macro namespace, to avoid collisions and so it can be put at the end with text.unlikely to keep the cold code together. See commit: cb87481ee89db ("kbuild: linker script do not match C names unless LD_DEAD_CODE_DATA_ELIMINATION is configured") ... which matches C names: [a-zA-Z0-9_] but not ".". Reported-by: Mike Galbraith <efault@gmx.de> Signed-off-by: Kees Cook <keescook@chromium.org> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Elena <elena.reshetova@intel.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: linux-arch <linux-arch@vger.kernel.org> Fixes: 7a46ec0e2f48 ("locking/refcounts, x86/asm: Implement fast refcount overflow protection") Link: http://lkml.kernel.org/r/1504382986-49301-2-git-send-email-keescook@chromium.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/include/asm/refcount.h | 2 +- arch/x86/mm/extable.c | 7 ++++++- include/asm-generic/vmlinux.lds.h | 1 + 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/refcount.h b/arch/x86/include/asm/refcount.h index ff87121..4e44250 100644 --- a/arch/x86/include/asm/refcount.h +++ b/arch/x86/include/asm/refcount.h @@ -15,7 +15,7 @@ * back to the regular execution flow in .text. */ #define _REFCOUNT_EXCEPTION \ - ".pushsection .text.unlikely\n" \ + ".pushsection .text..refcount\n" \ "111:\tlea %[counter], %%" _ASM_CX "\n" \ "112:\t" ASM_UD0 "\n" \ ASM_UNREACHABLE \ diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c index c3521e2..3321b44 100644 --- a/arch/x86/mm/extable.c +++ b/arch/x86/mm/extable.c @@ -67,12 +67,17 @@ bool ex_handler_refcount(const struct exception_table_entry *fixup, * wrapped around) will be set. Additionally, seeing the refcount * reach 0 will set ZF (Zero Flag: result was zero). In each of * these cases we want a report, since it's a boundary condition. - * + * The SF case is not reported since it indicates post-boundary + * manipulations below zero or above INT_MAX. And if none of the + * flags are set, something has gone very wrong, so report it. */ if (regs->flags & (X86_EFLAGS_OF | X86_EFLAGS_ZF)) { bool zero = regs->flags & X86_EFLAGS_ZF; refcount_error_report(regs, zero ? "hit zero" : "overflow"); + } else if ((regs->flags & X86_EFLAGS_SF) == 0) { + /* Report if none of OF, ZF, nor SF are set. */ + refcount_error_report(regs, "unexpected saturation"); } return true; diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index 8acfc1e..e549bff 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -459,6 +459,7 @@ #define TEXT_TEXT \ ALIGN_FUNCTION(); \ *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ + *(.text..refcount) \ *(.ref.text) \ MEM_KEEP(init.text) \ MEM_KEEP(exit.text) \ ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] locking/refcounts, x86/asm: Enable CONFIG_ARCH_HAS_REFCOUNT 2017-09-02 20:09 [PATCH v2 0/2] locking/refcounts, x86/asm: Use unique .text section for refcount exceptions Kees Cook 2017-09-02 20:09 ` [PATCH v2 1/2] " Kees Cook @ 2017-09-02 20:09 ` Kees Cook 2017-09-28 10:58 ` [tip:locking/core] " tip-bot for Kees Cook 1 sibling, 1 reply; 5+ messages in thread From: Kees Cook @ 2017-09-02 20:09 UTC (permalink / raw) To: Ingo Molnar Cc: Kees Cook, Mike Galbraith, x86, linux-arch, LKML, Reshetova, Elena, Peter Zijlstra, Ard Biesheuvel With the section inlining bug fixed for the x86 refcount protection, we can turn the config back on. Cc: Mike Galbraith <efault@gmx.de> Signed-off-by: Kees Cook <keescook@chromium.org> --- arch/x86/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index eaa8ff41f424..c6acdcdb3fc6 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -56,7 +56,7 @@ config X86 select ARCH_HAS_MMIO_FLUSH select ARCH_HAS_PMEM_API if X86_64 # Causing hangs/crashes, see the commit that added this change for details. - select ARCH_HAS_REFCOUNT if BROKEN + select ARCH_HAS_REFCOUNT select ARCH_HAS_UACCESS_FLUSHCACHE if X86_64 select ARCH_HAS_SET_MEMORY select ARCH_HAS_SG_CHAIN -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:locking/core] locking/refcounts, x86/asm: Enable CONFIG_ARCH_HAS_REFCOUNT 2017-09-02 20:09 ` [PATCH v2 2/2] locking/refcounts, x86/asm: Enable CONFIG_ARCH_HAS_REFCOUNT Kees Cook @ 2017-09-28 10:58 ` tip-bot for Kees Cook 0 siblings, 0 replies; 5+ messages in thread From: tip-bot for Kees Cook @ 2017-09-28 10:58 UTC (permalink / raw) To: linux-tip-commits Cc: keescook, hpa, tglx, ard.biesheuvel, peterz, linux-arch, torvalds, elena.reshetova, mingo, linux-kernel, efault Commit-ID: 39208aa7ecb7d9c4e86df782b5693270313cbab1 Gitweb: https://git.kernel.org/tip/39208aa7ecb7d9c4e86df782b5693270313cbab1 Author: Kees Cook <keescook@chromium.org> AuthorDate: Sat, 2 Sep 2017 13:09:46 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Thu, 28 Sep 2017 09:45:05 +0200 locking/refcounts, x86/asm: Enable CONFIG_ARCH_HAS_REFCOUNT With the section inlining bug fixed for the x86 refcount protection, we can turn the config back on. Signed-off-by: Kees Cook <keescook@chromium.org> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Elena <elena.reshetova@intel.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Mike Galbraith <efault@gmx.de> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: linux-arch <linux-arch@vger.kernel.org> Link: http://lkml.kernel.org/r/1504382986-49301-3-git-send-email-keescook@chromium.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 971feac..9053564 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -55,7 +55,7 @@ config X86 select ARCH_HAS_KCOV if X86_64 select ARCH_HAS_PMEM_API if X86_64 # Causing hangs/crashes, see the commit that added this change for details. - select ARCH_HAS_REFCOUNT if BROKEN + select ARCH_HAS_REFCOUNT select ARCH_HAS_UACCESS_FLUSHCACHE if X86_64 select ARCH_HAS_SET_MEMORY select ARCH_HAS_SG_CHAIN ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-09-28 11:03 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-02 20:09 [PATCH v2 0/2] locking/refcounts, x86/asm: Use unique .text section for refcount exceptions Kees Cook 2017-09-02 20:09 ` [PATCH v2 1/2] " Kees Cook 2017-09-28 10:58 ` [tip:locking/core] " tip-bot for Kees Cook 2017-09-02 20:09 ` [PATCH v2 2/2] locking/refcounts, x86/asm: Enable CONFIG_ARCH_HAS_REFCOUNT Kees Cook 2017-09-28 10:58 ` [tip:locking/core] " tip-bot for Kees Cook
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome