[PATCH 0/2] x86/unwind/orc: Handle missing ORC data better

[PATCH 0/2] x86/unwind/orc: Handle missing ORC data better

[Josh Poimboeuf]

A couple of patches for improving the ORC unwinder's handling of missing
ORC data.

Josh Poimboeuf (2):
  x86/unwind/orc: Disable KASAN checking in the ORC unwinder, part 2
  x86/unwind/orc: Silence warnings caused by missing ORC data

 arch/x86/kernel/unwind_orc.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

-- 
2.29.2

[PATCH 1/2] x86/unwind/orc: Disable KASAN checking in the ORC unwinder, part 2

[Josh Poimboeuf]

KASAN reserves "redzone" areas between stack frames in order to detect
stack overruns.  A read or write to such an area triggers a KASAN
"stack-out-of-bounds" BUG.

Normally, the ORC unwinder stays in-bounds and doesn't access the
redzone.  But sometimes it can't find ORC metadata for a given
instruction.  This can happen for code which is missing ORC metadata, or
for generated code.  In such cases, the unwinder attempts to fall back
to frame pointers, as a best-effort type thing.

This fallback often works, but when it doesn't, the unwinder can get
confused and go off into the weeds into the KASAN redzone, triggering
the aforementioned KASAN BUG.

But in this case, the unwinder's confusion is actually harmless and
working as designed.  It already has checks in place to prevent
off-stack accesses, but those checks get short-circuited by the KASAN
BUG.  And a BUG is a lot more disruptive than a harmless unwinder
warning.

Disable the KASAN checks by using READ_ONCE_NOCHECK() for all stack
accesses.  This finishes the job started by commit 881125bfe65b
("x86/unwind: Disable KASAN checking in the ORC unwinder"), which only
partially fixed the issue.

Fixes: ee9f8fce9964 ("x86/unwind: Add the ORC unwinder")
Reported-by: Ivan Babrou <ivan@cloudflare.com>
Cc: stable@vger.kernel.org
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/kernel/unwind_orc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 73f800100066..c451d5f6422f 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -367,8 +367,8 @@ static bool deref_stack_regs(struct unwind_state *state, unsigned long addr,
 	if (!stack_access_ok(state, addr, sizeof(struct pt_regs)))
 		return false;
 
-	*ip = regs->ip;
-	*sp = regs->sp;
+	*ip = READ_ONCE_NOCHECK(regs->ip);
+	*sp = READ_ONCE_NOCHECK(regs->sp);
 	return true;
 }
 
@@ -380,8 +380,8 @@ static bool deref_stack_iret_regs(struct unwind_state *state, unsigned long addr
 	if (!stack_access_ok(state, addr, IRET_FRAME_SIZE))
 		return false;
 
-	*ip = regs->ip;
-	*sp = regs->sp;
+	*ip = READ_ONCE_NOCHECK(regs->ip);
+	*sp = READ_ONCE_NOCHECK(regs->sp);
 	return true;
 }
 
@@ -402,12 +402,12 @@ static bool get_reg(struct unwind_state *state, unsigned int reg_off,
 		return false;
 
 	if (state->full_regs) {
-		*val = ((unsigned long *)state->regs)[reg];
+		*val = READ_ONCE_NOCHECK(((unsigned long *)state->regs)[reg]);
 		return true;
 	}
 
 	if (state->prev_regs) {
-		*val = ((unsigned long *)state->prev_regs)[reg];
+		*val = READ_ONCE_NOCHECK(((unsigned long *)state->prev_regs)[reg]);
 		return true;
 	}
 
-- 
2.29.2

[PATCH 2/2] x86/unwind/orc: Silence warnings caused by missing ORC data

[Josh Poimboeuf]

The ORC unwinder attempts to fall back to frame pointers when ORC data
is missing for a given instruction.  It sets state->error, but then
tries to keep going as a best-effort type of thing.  That may result in
further warnings if the unwinder gets lost.

Until we have some way to register generated code with the unwinder,
missing ORC will be expected, and occasionally going off the rails will
also be expected.  So don't warn about it.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/kernel/unwind_orc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index c451d5f6422f..027b72b5c9ed 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -13,7 +13,7 @@
 
 #define orc_warn_current(args...)					\
 ({									\
-	if (state->task == current)					\
+	if (state->task == current && !state->error)			\
 		orc_warn(args);						\
 })
 
-- 
2.29.2

Re: [PATCH 1/2] x86/unwind/orc: Disable KASAN checking in the ORC unwinder, part 2

[Steven Rostedt]

On Fri,  5 Feb 2021 08:24:02 -0600
Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> KASAN reserves "redzone" areas between stack frames in order to detect
> stack overruns.  A read or write to such an area triggers a KASAN
> "stack-out-of-bounds" BUG.
> 
> Normally, the ORC unwinder stays in-bounds and doesn't access the
> redzone.  But sometimes it can't find ORC metadata for a given
> instruction.  This can happen for code which is missing ORC metadata, or
> for generated code.  In such cases, the unwinder attempts to fall back
> to frame pointers, as a best-effort type thing.
> 
> This fallback often works, but when it doesn't, the unwinder can get
> confused and go off into the weeds into the KASAN redzone, triggering
> the aforementioned KASAN BUG.
> 
> But in this case, the unwinder's confusion is actually harmless and
> working as designed.  It already has checks in place to prevent
> off-stack accesses, but those checks get short-circuited by the KASAN
> BUG.  And a BUG is a lot more disruptive than a harmless unwinder
> warning.
> 
> Disable the KASAN checks by using READ_ONCE_NOCHECK() for all stack
> accesses.  This finishes the job started by commit 881125bfe65b
> ("x86/unwind: Disable KASAN checking in the ORC unwinder"), which only
> partially fixed the issue.
> 
> Fixes: ee9f8fce9964 ("x86/unwind: Add the ORC unwinder")
> Reported-by: Ivan Babrou <ivan@cloudflare.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

Re: [PATCH 1/2] x86/unwind/orc: Disable KASAN checking in the ORC unwinder, part 2

[Ivan Babrou]

On Fri, Feb 5, 2021 at 6:24 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> KASAN reserves "redzone" areas between stack frames in order to detect
> stack overruns.  A read or write to such an area triggers a KASAN
> "stack-out-of-bounds" BUG.
>
> Normally, the ORC unwinder stays in-bounds and doesn't access the
> redzone.  But sometimes it can't find ORC metadata for a given
> instruction.  This can happen for code which is missing ORC metadata, or
> for generated code.  In such cases, the unwinder attempts to fall back
> to frame pointers, as a best-effort type thing.
>
> This fallback often works, but when it doesn't, the unwinder can get
> confused and go off into the weeds into the KASAN redzone, triggering
> the aforementioned KASAN BUG.
>
> But in this case, the unwinder's confusion is actually harmless and
> working as designed.  It already has checks in place to prevent
> off-stack accesses, but those checks get short-circuited by the KASAN
> BUG.  And a BUG is a lot more disruptive than a harmless unwinder
> warning.
>
> Disable the KASAN checks by using READ_ONCE_NOCHECK() for all stack
> accesses.  This finishes the job started by commit 881125bfe65b
> ("x86/unwind: Disable KASAN checking in the ORC unwinder"), which only
> partially fixed the issue.
>
> Fixes: ee9f8fce9964 ("x86/unwind: Add the ORC unwinder")
> Reported-by: Ivan Babrou <ivan@cloudflare.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

I haven't seen any previously observed issues with this after a day of uptime.

Tested-by: Ivan Babrou <ivan@cloudflare.com>

Re: [PATCH 2/2] x86/unwind/orc: Silence warnings caused by missing ORC data

[Josh Poimboeuf]

On Fri, Feb 05, 2021 at 08:24:03AM -0600, Josh Poimboeuf wrote:
> The ORC unwinder attempts to fall back to frame pointers when ORC data
> is missing for a given instruction.  It sets state->error, but then
> tries to keep going as a best-effort type of thing.  That may result in
> further warnings if the unwinder gets lost.
> 
> Until we have some way to register generated code with the unwinder,
> missing ORC will be expected, and occasionally going off the rails will
> also be expected.  So don't warn about it.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

Cc: stable@vger.kernel.org

-- 
Josh

Re: [PATCH 2/2] x86/unwind/orc: Silence warnings caused by missing ORC data

[Ivan Babrou]

On Mon, Feb 8, 2021 at 11:56 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> On Fri, Feb 05, 2021 at 08:24:03AM -0600, Josh Poimboeuf wrote:
> > The ORC unwinder attempts to fall back to frame pointers when ORC data
> > is missing for a given instruction.  It sets state->error, but then
> > tries to keep going as a best-effort type of thing.  That may result in
> > further warnings if the unwinder gets lost.
> >
> > Until we have some way to register generated code with the unwinder,
> > missing ORC will be expected, and occasionally going off the rails will
> > also be expected.  So don't warn about it.
> >
> > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
>
> Cc: stable@vger.kernel.org

I was the one who asked for this to be backported, since it solved the
warnings for me.

Tested-by: Ivan Babrou <ivan@cloudflare.com>

Re: [PATCH 0/2] x86/unwind/orc: Handle missing ORC data better

[Miroslav Benes]

On Fri, 5 Feb 2021, Josh Poimboeuf wrote:

> A couple of patches for improving the ORC unwinder's handling of missing
> ORC data.
> 
> Josh Poimboeuf (2):
>   x86/unwind/orc: Disable KASAN checking in the ORC unwinder, part 2
>   x86/unwind/orc: Silence warnings caused by missing ORC data
> 
>  arch/x86/kernel/unwind_orc.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

I apologize for a really late reply. It seems it has not been merged yet, 
so

Reviewed-by: Miroslav Benes <mbenes@suse.cz>

M

Re: [PATCH 2/2] x86/unwind/orc: Silence warnings caused by missing ORC data

[Peter Zijlstra]

On Fri, Feb 05, 2021 at 08:24:03AM -0600, Josh Poimboeuf wrote:
> The ORC unwinder attempts to fall back to frame pointers when ORC data
> is missing for a given instruction.  It sets state->error, but then
> tries to keep going as a best-effort type of thing.  That may result in
> further warnings if the unwinder gets lost.
> 
> Until we have some way to register generated code with the unwinder,
> missing ORC will be expected, and occasionally going off the rails will
> also be expected.  So don't warn about it.

I recently ran into another variant of missing ORC data, some files are
simply not processed by objtool, eg. arch/x86/realmode/init.c. Would it
make sense to have the vmlinux pass (when it isn't used to generate orc
in the first place) also check that all code it finds has ORC data?

It's not fool proof, but it should help find files we're missing for
some raisin.

Re: [PATCH 2/2] x86/unwind/orc: Silence warnings caused by missing ORC data

[Josh Poimboeuf]

On Wed, Feb 24, 2021 at 03:52:01PM +0100, Peter Zijlstra wrote:
> On Fri, Feb 05, 2021 at 08:24:03AM -0600, Josh Poimboeuf wrote:
> > The ORC unwinder attempts to fall back to frame pointers when ORC data
> > is missing for a given instruction.  It sets state->error, but then
> > tries to keep going as a best-effort type of thing.  That may result in
> > further warnings if the unwinder gets lost.
> > 
> > Until we have some way to register generated code with the unwinder,
> > missing ORC will be expected, and occasionally going off the rails will
> > also be expected.  So don't warn about it.
> 
> I recently ran into another variant of missing ORC data, some files are
> simply not processed by objtool, eg. arch/x86/realmode/init.c. Would it
> make sense to have the vmlinux pass (when it isn't used to generate orc
> in the first place) also check that all code it finds has ORC data?
> 
> It's not fool proof, but it should help find files we're missing for
> some raisin.

Doesn't validate_reachable_instructions() basically already do that?

-- 
Josh

Re: [PATCH 2/2] x86/unwind/orc: Silence warnings caused by missing ORC data

[Peter Zijlstra]

On Wed, Feb 24, 2021 at 09:18:05AM -0600, Josh Poimboeuf wrote:
> On Wed, Feb 24, 2021 at 03:52:01PM +0100, Peter Zijlstra wrote:
> > On Fri, Feb 05, 2021 at 08:24:03AM -0600, Josh Poimboeuf wrote:
> > > The ORC unwinder attempts to fall back to frame pointers when ORC data
> > > is missing for a given instruction.  It sets state->error, but then
> > > tries to keep going as a best-effort type of thing.  That may result in
> > > further warnings if the unwinder gets lost.
> > > 
> > > Until we have some way to register generated code with the unwinder,
> > > missing ORC will be expected, and occasionally going off the rails will
> > > also be expected.  So don't warn about it.
> > 
> > I recently ran into another variant of missing ORC data, some files are
> > simply not processed by objtool, eg. arch/x86/realmode/init.c. Would it
> > make sense to have the vmlinux pass (when it isn't used to generate orc
> > in the first place) also check that all code it finds has ORC data?
> > 
> > It's not fool proof, but it should help find files we're missing for
> > some raisin.
> 
> Doesn't validate_reachable_instructions() basically already do that?

Nope, I'm talking about the case where we generate ORC for each .o file
(and 'forget' to run objtool on some of them). And then run objtool
again on vmlinux to validate (things like noinstr).

At that point it might make sense to also check that all code does
indeed have an ORC to double check our initial (per translation unit)
invocation didn't accidentally miss someone.

Re: [PATCH 2/2] x86/unwind/orc: Silence warnings caused by missing ORC data

[Josh Poimboeuf]

On Wed, Feb 24, 2021 at 07:07:31PM +0100, Peter Zijlstra wrote:
> On Wed, Feb 24, 2021 at 09:18:05AM -0600, Josh Poimboeuf wrote:
> > On Wed, Feb 24, 2021 at 03:52:01PM +0100, Peter Zijlstra wrote:
> > > On Fri, Feb 05, 2021 at 08:24:03AM -0600, Josh Poimboeuf wrote:
> > > > The ORC unwinder attempts to fall back to frame pointers when ORC data
> > > > is missing for a given instruction.  It sets state->error, but then
> > > > tries to keep going as a best-effort type of thing.  That may result in
> > > > further warnings if the unwinder gets lost.
> > > > 
> > > > Until we have some way to register generated code with the unwinder,
> > > > missing ORC will be expected, and occasionally going off the rails will
> > > > also be expected.  So don't warn about it.
> > > 
> > > I recently ran into another variant of missing ORC data, some files are
> > > simply not processed by objtool, eg. arch/x86/realmode/init.c. Would it
> > > make sense to have the vmlinux pass (when it isn't used to generate orc
> > > in the first place) also check that all code it finds has ORC data?
> > > 
> > > It's not fool proof, but it should help find files we're missing for
> > > some raisin.
> > 
> > Doesn't validate_reachable_instructions() basically already do that?
> 
> Nope, I'm talking about the case where we generate ORC for each .o file
> (and 'forget' to run objtool on some of them). And then run objtool
> again on vmlinux to validate (things like noinstr).
> 
> At that point it might make sense to also check that all code does
> indeed have an ORC to double check our initial (per translation unit)
> invocation didn't accidentally miss someone.

What I meant was, validate_reachable_instructions() should already be
able to report that situation, if it were called on vmlinux.  Right now
I think noinstr avoids calling it.

I'm already working on some vmlinux rework which will enable that
checking.  Then we'll have to go through the unreachable warnings and
try to fix them.

-- 
Josh

[tip: x86/urgent] x86/unwind/orc: Silence warnings caused by missing ORC data

[tip-bot2 for Josh Poimboeuf]

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     86402dcc894951c0a363b6aee12d955ff923b35e
Gitweb:        https://git.kernel.org/tip/86402dcc894951c0a363b6aee12d955ff923b35e
Author:        Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate:    Fri, 05 Feb 2021 08:24:03 -06:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 03 Mar 2021 16:56:30 +01:00

x86/unwind/orc: Silence warnings caused by missing ORC data

The ORC unwinder attempts to fall back to frame pointers when ORC data
is missing for a given instruction.  It sets state->error, but then
tries to keep going as a best-effort type of thing.  That may result in
further warnings if the unwinder gets lost.

Until we have some way to register generated code with the unwinder,
missing ORC will be expected, and occasionally going off the rails will
also be expected.  So don't warn about it.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Tested-by: Ivan Babrou <ivan@cloudflare.com>
Link: https://lkml.kernel.org/r/06d02c4bbb220bd31668db579278b0352538efbb.1612534649.git.jpoimboe@redhat.com
---
 arch/x86/kernel/unwind_orc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 1bcc14c..a120253 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -13,7 +13,7 @@
 
 #define orc_warn_current(args...)					\
 ({									\
-	if (state->task == current)					\
+	if (state->task == current && !state->error)			\
 		orc_warn(args);						\
 })
 

[tip: x86/urgent] x86/unwind/orc: Disable KASAN checking in the ORC unwinder, part 2

[tip-bot2 for Josh Poimboeuf]

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     38b6eb474ed2df3d159396c3d4312c8a7b2d5196
Gitweb:        https://git.kernel.org/tip/38b6eb474ed2df3d159396c3d4312c8a7b2d5196
Author:        Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate:    Fri, 05 Feb 2021 08:24:02 -06:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 03 Mar 2021 16:56:29 +01:00

x86/unwind/orc: Disable KASAN checking in the ORC unwinder, part 2

KASAN reserves "redzone" areas between stack frames in order to detect
stack overruns.  A read or write to such an area triggers a KASAN
"stack-out-of-bounds" BUG.

Normally, the ORC unwinder stays in-bounds and doesn't access the
redzone.  But sometimes it can't find ORC metadata for a given
instruction.  This can happen for code which is missing ORC metadata, or
for generated code.  In such cases, the unwinder attempts to fall back
to frame pointers, as a best-effort type thing.

This fallback often works, but when it doesn't, the unwinder can get
confused and go off into the weeds into the KASAN redzone, triggering
the aforementioned KASAN BUG.

But in this case, the unwinder's confusion is actually harmless and
working as designed.  It already has checks in place to prevent
off-stack accesses, but those checks get short-circuited by the KASAN
BUG.  And a BUG is a lot more disruptive than a harmless unwinder
warning.

Disable the KASAN checks by using READ_ONCE_NOCHECK() for all stack
accesses.  This finishes the job started by commit 881125bfe65b
("x86/unwind: Disable KASAN checking in the ORC unwinder"), which only
partially fixed the issue.

Fixes: ee9f8fce9964 ("x86/unwind: Add the ORC unwinder")
Reported-by: Ivan Babrou <ivan@cloudflare.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Tested-by: Ivan Babrou <ivan@cloudflare.com>
Cc: stable@kernel.org
Link: https://lkml.kernel.org/r/9583327904ebbbeda399eca9c56d6c7085ac20fe.1612534649.git.jpoimboe@redhat.com
---
 arch/x86/kernel/unwind_orc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 2a1d47f..1bcc14c 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -367,8 +367,8 @@ static bool deref_stack_regs(struct unwind_state *state, unsigned long addr,
 	if (!stack_access_ok(state, addr, sizeof(struct pt_regs)))
 		return false;
 
-	*ip = regs->ip;
-	*sp = regs->sp;
+	*ip = READ_ONCE_NOCHECK(regs->ip);
+	*sp = READ_ONCE_NOCHECK(regs->sp);
 	return true;
 }
 
@@ -380,8 +380,8 @@ static bool deref_stack_iret_regs(struct unwind_state *state, unsigned long addr
 	if (!stack_access_ok(state, addr, IRET_FRAME_SIZE))
 		return false;
 
-	*ip = regs->ip;
-	*sp = regs->sp;
+	*ip = READ_ONCE_NOCHECK(regs->ip);
+	*sp = READ_ONCE_NOCHECK(regs->sp);
 	return true;
 }
 
@@ -402,12 +402,12 @@ static bool get_reg(struct unwind_state *state, unsigned int reg_off,
 		return false;
 
 	if (state->full_regs) {
-		*val = ((unsigned long *)state->regs)[reg];
+		*val = READ_ONCE_NOCHECK(((unsigned long *)state->regs)[reg]);
 		return true;
 	}
 
 	if (state->prev_regs) {
-		*val = ((unsigned long *)state->prev_regs)[reg];
+		*val = READ_ONCE_NOCHECK(((unsigned long *)state->prev_regs)[reg]);
 		return true;
 	}
 

[tip: x86/urgent] x86/unwind/orc: Silence warnings caused by missing ORC data

[tip-bot2 for Josh Poimboeuf]

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     d072f941c1e234f8495cc4828370b180318bf49b
Gitweb:        https://git.kernel.org/tip/d072f941c1e234f8495cc4828370b180318bf49b
Author:        Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate:    Fri, 05 Feb 2021 08:24:03 -06:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Sat, 06 Mar 2021 11:37:00 +01:00

x86/unwind/orc: Silence warnings caused by missing ORC data

The ORC unwinder attempts to fall back to frame pointers when ORC data
is missing for a given instruction.  It sets state->error, but then
tries to keep going as a best-effort type of thing.  That may result in
further warnings if the unwinder gets lost.

Until we have some way to register generated code with the unwinder,
missing ORC will be expected, and occasionally going off the rails will
also be expected.  So don't warn about it.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Tested-by: Ivan Babrou <ivan@cloudflare.com>
Link: https://lkml.kernel.org/r/06d02c4bbb220bd31668db579278b0352538efbb.1612534649.git.jpoimboe@redhat.com
---
 arch/x86/kernel/unwind_orc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 1bcc14c..a120253 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -13,7 +13,7 @@
 
 #define orc_warn_current(args...)					\
 ({									\
-	if (state->task == current)					\
+	if (state->task == current && !state->error)			\
 		orc_warn(args);						\
 })
 

[tip: x86/urgent] x86/unwind/orc: Disable KASAN checking in the ORC unwinder, part 2

[tip-bot2 for Josh Poimboeuf]

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     8bd7b3980ca62904814d536b3a2453001992a0c3
Gitweb:        https://git.kernel.org/tip/8bd7b3980ca62904814d536b3a2453001992a0c3
Author:        Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate:    Fri, 05 Feb 2021 08:24:02 -06:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Sat, 06 Mar 2021 11:37:00 +01:00

x86/unwind/orc: Disable KASAN checking in the ORC unwinder, part 2

KASAN reserves "redzone" areas between stack frames in order to detect
stack overruns.  A read or write to such an area triggers a KASAN
"stack-out-of-bounds" BUG.

Normally, the ORC unwinder stays in-bounds and doesn't access the
redzone.  But sometimes it can't find ORC metadata for a given
instruction.  This can happen for code which is missing ORC metadata, or
for generated code.  In such cases, the unwinder attempts to fall back
to frame pointers, as a best-effort type thing.

This fallback often works, but when it doesn't, the unwinder can get
confused and go off into the weeds into the KASAN redzone, triggering
the aforementioned KASAN BUG.

But in this case, the unwinder's confusion is actually harmless and
working as designed.  It already has checks in place to prevent
off-stack accesses, but those checks get short-circuited by the KASAN
BUG.  And a BUG is a lot more disruptive than a harmless unwinder
warning.

Disable the KASAN checks by using READ_ONCE_NOCHECK() for all stack
accesses.  This finishes the job started by commit 881125bfe65b
("x86/unwind: Disable KASAN checking in the ORC unwinder"), which only
partially fixed the issue.

Fixes: ee9f8fce9964 ("x86/unwind: Add the ORC unwinder")
Reported-by: Ivan Babrou <ivan@cloudflare.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Tested-by: Ivan Babrou <ivan@cloudflare.com>
Cc: stable@kernel.org
Link: https://lkml.kernel.org/r/9583327904ebbbeda399eca9c56d6c7085ac20fe.1612534649.git.jpoimboe@redhat.com
---
 arch/x86/kernel/unwind_orc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 2a1d47f..1bcc14c 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -367,8 +367,8 @@ static bool deref_stack_regs(struct unwind_state *state, unsigned long addr,
 	if (!stack_access_ok(state, addr, sizeof(struct pt_regs)))
 		return false;
 
-	*ip = regs->ip;
-	*sp = regs->sp;
+	*ip = READ_ONCE_NOCHECK(regs->ip);
+	*sp = READ_ONCE_NOCHECK(regs->sp);
 	return true;
 }
 
@@ -380,8 +380,8 @@ static bool deref_stack_iret_regs(struct unwind_state *state, unsigned long addr
 	if (!stack_access_ok(state, addr, IRET_FRAME_SIZE))
 		return false;
 
-	*ip = regs->ip;
-	*sp = regs->sp;
+	*ip = READ_ONCE_NOCHECK(regs->ip);
+	*sp = READ_ONCE_NOCHECK(regs->sp);
 	return true;
 }
 
@@ -402,12 +402,12 @@ static bool get_reg(struct unwind_state *state, unsigned int reg_off,
 		return false;
 
 	if (state->full_regs) {
-		*val = ((unsigned long *)state->regs)[reg];
+		*val = READ_ONCE_NOCHECK(((unsigned long *)state->regs)[reg]);
 		return true;
 	}
 
 	if (state->prev_regs) {
-		*val = ((unsigned long *)state->prev_regs)[reg];
+		*val = READ_ONCE_NOCHECK(((unsigned long *)state->prev_regs)[reg]);
 		return true;
 	}
 

[tip: x86/urgent] x86/unwind/orc: Disable KASAN checking in the ORC unwinder, part 2

[tip-bot2 for Josh Poimboeuf]

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     e504e74cc3a2c092b05577ce3e8e013fae7d94e6
Gitweb:        https://git.kernel.org/tip/e504e74cc3a2c092b05577ce3e8e013fae7d94e6
Author:        Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate:    Fri, 05 Feb 2021 08:24:02 -06:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Sat, 06 Mar 2021 13:09:37 +01:00

x86/unwind/orc: Disable KASAN checking in the ORC unwinder, part 2

KASAN reserves "redzone" areas between stack frames in order to detect
stack overruns.  A read or write to such an area triggers a KASAN
"stack-out-of-bounds" BUG.

Normally, the ORC unwinder stays in-bounds and doesn't access the
redzone.  But sometimes it can't find ORC metadata for a given
instruction.  This can happen for code which is missing ORC metadata, or
for generated code.  In such cases, the unwinder attempts to fall back
to frame pointers, as a best-effort type thing.

This fallback often works, but when it doesn't, the unwinder can get
confused and go off into the weeds into the KASAN redzone, triggering
the aforementioned KASAN BUG.

But in this case, the unwinder's confusion is actually harmless and
working as designed.  It already has checks in place to prevent
off-stack accesses, but those checks get short-circuited by the KASAN
BUG.  And a BUG is a lot more disruptive than a harmless unwinder
warning.

Disable the KASAN checks by using READ_ONCE_NOCHECK() for all stack
accesses.  This finishes the job started by commit 881125bfe65b
("x86/unwind: Disable KASAN checking in the ORC unwinder"), which only
partially fixed the issue.

Fixes: ee9f8fce9964 ("x86/unwind: Add the ORC unwinder")
Reported-by: Ivan Babrou <ivan@cloudflare.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Tested-by: Ivan Babrou <ivan@cloudflare.com>
Cc: stable@kernel.org
Link: https://lkml.kernel.org/r/9583327904ebbbeda399eca9c56d6c7085ac20fe.1612534649.git.jpoimboe@redhat.com
---
 arch/x86/kernel/unwind_orc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 2a1d47f..1bcc14c 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -367,8 +367,8 @@ static bool deref_stack_regs(struct unwind_state *state, unsigned long addr,
 	if (!stack_access_ok(state, addr, sizeof(struct pt_regs)))
 		return false;
 
-	*ip = regs->ip;
-	*sp = regs->sp;
+	*ip = READ_ONCE_NOCHECK(regs->ip);
+	*sp = READ_ONCE_NOCHECK(regs->sp);
 	return true;
 }
 
@@ -380,8 +380,8 @@ static bool deref_stack_iret_regs(struct unwind_state *state, unsigned long addr
 	if (!stack_access_ok(state, addr, IRET_FRAME_SIZE))
 		return false;
 
-	*ip = regs->ip;
-	*sp = regs->sp;
+	*ip = READ_ONCE_NOCHECK(regs->ip);
+	*sp = READ_ONCE_NOCHECK(regs->sp);
 	return true;
 }
 
@@ -402,12 +402,12 @@ static bool get_reg(struct unwind_state *state, unsigned int reg_off,
 		return false;
 
 	if (state->full_regs) {
-		*val = ((unsigned long *)state->regs)[reg];
+		*val = READ_ONCE_NOCHECK(((unsigned long *)state->regs)[reg]);
 		return true;
 	}
 
 	if (state->prev_regs) {
-		*val = ((unsigned long *)state->prev_regs)[reg];
+		*val = READ_ONCE_NOCHECK(((unsigned long *)state->prev_regs)[reg]);
 		return true;
 	}
 

[tip: x86/urgent] x86/unwind/orc: Silence warnings caused by missing ORC data

[tip-bot2 for Josh Poimboeuf]

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     b59cc97674c947861783ca92b9a6e7d043adba96
Gitweb:        https://git.kernel.org/tip/b59cc97674c947861783ca92b9a6e7d043adba96
Author:        Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate:    Fri, 05 Feb 2021 08:24:03 -06:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Sat, 06 Mar 2021 13:09:45 +01:00

x86/unwind/orc: Silence warnings caused by missing ORC data

The ORC unwinder attempts to fall back to frame pointers when ORC data
is missing for a given instruction.  It sets state->error, but then
tries to keep going as a best-effort type of thing.  That may result in
further warnings if the unwinder gets lost.

Until we have some way to register generated code with the unwinder,
missing ORC will be expected, and occasionally going off the rails will
also be expected.  So don't warn about it.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
Tested-by: Ivan Babrou <ivan@cloudflare.com>
Link: https://lkml.kernel.org/r/06d02c4bbb220bd31668db579278b0352538efbb.1612534649.git.jpoimboe@redhat.com
---
 arch/x86/kernel/unwind_orc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 1bcc14c..a120253 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -13,7 +13,7 @@
 
 #define orc_warn_current(args...)					\
 ({									\
-	if (state->task == current)					\
+	if (state->task == current && !state->error)			\
 		orc_warn(args);						\
 })