* [PATCH] objtool,x86: Verify poke_int3_handler() is self contained
@ 2020-07-27 10:40 peterz
2020-07-27 11:21 ` Ingo Molnar
2020-07-27 20:56 ` Thomas Gleixner
0 siblings, 2 replies; 6+ messages in thread
From: peterz @ 2020-07-27 10:40 UTC (permalink / raw)
To: Josh Poimboeuf, Thomas Gleixner; +Cc: x86, linux-kernel, mbenes
Abuse the SMAP rules to ensure poke_int3_handler() doesn't call out to
anything.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
tools/objtool/check.c | 8 ++++++++
1 file changed, 8 insertions(+)
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -551,6 +551,14 @@ static const char *uaccess_safe_builtin[
"__memcpy_mcsafe",
"mcsafe_handle_tail",
"ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */
+ /*
+ * Abuse alert!
+ *
+ * poke_int3_handler() is not in fact related to uaccess at all, we
+ * abuse the uaccess rules to ensure poke_int3_handler() is self
+ * contained and doesn't CALL out to other code.
+ */
+ "poke_int3_handler",
NULL
};
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] objtool,x86: Verify poke_int3_handler() is self contained
2020-07-27 10:40 [PATCH] objtool,x86: Verify poke_int3_handler() is self contained peterz
@ 2020-07-27 11:21 ` Ingo Molnar
2020-07-27 12:50 ` peterz
2020-07-27 20:56 ` Thomas Gleixner
1 sibling, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2020-07-27 11:21 UTC (permalink / raw)
To: peterz; +Cc: Josh Poimboeuf, Thomas Gleixner, x86, linux-kernel, mbenes
* peterz@infradead.org <peterz@infradead.org> wrote:
>
> Abuse the SMAP rules to ensure poke_int3_handler() doesn't call out to
> anything.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> tools/objtool/check.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -551,6 +551,14 @@ static const char *uaccess_safe_builtin[
> "__memcpy_mcsafe",
> "mcsafe_handle_tail",
> "ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */
> + /*
> + * Abuse alert!
> + *
> + * poke_int3_handler() is not in fact related to uaccess at all, we
> + * abuse the uaccess rules to ensure poke_int3_handler() is self
> + * contained and doesn't CALL out to other code.
> + */
> + "poke_int3_handler",
So ->uaccess_safe makes sure that we don't CALL into non-uaccess-safe
functions, but it still allows CALLs into *other* uaccess-safe
functions, right?
So unless I missed something in the logic, the comment should say
something like "doesn't CALL out to other non-uaccess safe code" or
so? Which is, arguably, like 99% of all functions - but still, a whole
bunch are allowed, such as low level instrumentation and other utility
functions.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] objtool,x86: Verify poke_int3_handler() is self contained
2020-07-27 11:21 ` Ingo Molnar
@ 2020-07-27 12:50 ` peterz
2020-07-27 13:11 ` Ingo Molnar
0 siblings, 1 reply; 6+ messages in thread
From: peterz @ 2020-07-27 12:50 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Josh Poimboeuf, Thomas Gleixner, x86, linux-kernel, mbenes
On Mon, Jul 27, 2020 at 01:21:44PM +0200, Ingo Molnar wrote:
>
> * peterz@infradead.org <peterz@infradead.org> wrote:
>
> >
> > Abuse the SMAP rules to ensure poke_int3_handler() doesn't call out to
> > anything.
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > ---
> > tools/objtool/check.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > --- a/tools/objtool/check.c
> > +++ b/tools/objtool/check.c
> > @@ -551,6 +551,14 @@ static const char *uaccess_safe_builtin[
> > "__memcpy_mcsafe",
> > "mcsafe_handle_tail",
> > "ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */
> > + /*
> > + * Abuse alert!
> > + *
> > + * poke_int3_handler() is not in fact related to uaccess at all, we
> > + * abuse the uaccess rules to ensure poke_int3_handler() is self
> > + * contained and doesn't CALL out to other code.
> > + */
> > + "poke_int3_handler",
>
> So ->uaccess_safe makes sure that we don't CALL into non-uaccess-safe
> functions, but it still allows CALLs into *other* uaccess-safe
> functions, right?
>
> So unless I missed something in the logic, the comment should say
> something like "doesn't CALL out to other non-uaccess safe code" or
> so? Which is, arguably, like 99% of all functions - but still, a whole
> bunch are allowed, such as low level instrumentation and other utility
> functions.
Right, so poke_int3_handler() is also noinstr and by that isn't allowed
to call out into !noinstr code. The intersection should be small.
But yeah, perhaps this is a bad idea and I should add another annotation
for this,.. dunno.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] objtool,x86: Verify poke_int3_handler() is self contained
2020-07-27 12:50 ` peterz
@ 2020-07-27 13:11 ` Ingo Molnar
0 siblings, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2020-07-27 13:11 UTC (permalink / raw)
To: peterz; +Cc: Josh Poimboeuf, Thomas Gleixner, x86, linux-kernel, mbenes
* peterz@infradead.org <peterz@infradead.org> wrote:
> On Mon, Jul 27, 2020 at 01:21:44PM +0200, Ingo Molnar wrote:
> >
> > * peterz@infradead.org <peterz@infradead.org> wrote:
> >
> > >
> > > Abuse the SMAP rules to ensure poke_int3_handler() doesn't call out to
> > > anything.
> > >
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > > ---
> > > tools/objtool/check.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > --- a/tools/objtool/check.c
> > > +++ b/tools/objtool/check.c
> > > @@ -551,6 +551,14 @@ static const char *uaccess_safe_builtin[
> > > "__memcpy_mcsafe",
> > > "mcsafe_handle_tail",
> > > "ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */
> > > + /*
> > > + * Abuse alert!
> > > + *
> > > + * poke_int3_handler() is not in fact related to uaccess at all, we
> > > + * abuse the uaccess rules to ensure poke_int3_handler() is self
> > > + * contained and doesn't CALL out to other code.
> > > + */
> > > + "poke_int3_handler",
> >
> > So ->uaccess_safe makes sure that we don't CALL into non-uaccess-safe
> > functions, but it still allows CALLs into *other* uaccess-safe
> > functions, right?
> >
> > So unless I missed something in the logic, the comment should say
> > something like "doesn't CALL out to other non-uaccess safe code" or
> > so? Which is, arguably, like 99% of all functions - but still, a whole
> > bunch are allowed, such as low level instrumentation and other utility
> > functions.
>
> Right, so poke_int3_handler() is also noinstr and by that isn't allowed
> to call out into !noinstr code. The intersection should be small.
>
> But yeah, perhaps this is a bad idea and I should add another annotation
> for this,.. dunno.
'nocall' ? :-)
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] objtool,x86: Verify poke_int3_handler() is self contained
2020-07-27 10:40 [PATCH] objtool,x86: Verify poke_int3_handler() is self contained peterz
2020-07-27 11:21 ` Ingo Molnar
@ 2020-07-27 20:56 ` Thomas Gleixner
2020-07-28 8:34 ` peterz
1 sibling, 1 reply; 6+ messages in thread
From: Thomas Gleixner @ 2020-07-27 20:56 UTC (permalink / raw)
To: peterz, Josh Poimboeuf; +Cc: x86, linux-kernel, mbenes
peterz@infradead.org writes:
> Abuse the SMAP rules to ensure poke_int3_handler() doesn't call out to
> anything.
Yuck. Isn't that what noinstr is for or am I missing something?
Thanks,
tglx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] objtool,x86: Verify poke_int3_handler() is self contained
2020-07-27 20:56 ` Thomas Gleixner
@ 2020-07-28 8:34 ` peterz
0 siblings, 0 replies; 6+ messages in thread
From: peterz @ 2020-07-28 8:34 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Josh Poimboeuf, x86, linux-kernel, mbenes
On Mon, Jul 27, 2020 at 10:56:03PM +0200, Thomas Gleixner wrote:
> peterz@infradead.org writes:
> > Abuse the SMAP rules to ensure poke_int3_handler() doesn't call out to
> > anything.
>
> Yuck. Isn't that what noinstr is for or am I missing something?
Well, we don't want poke_int3_handler() to call out to noinstr either.
So noinstr only allows calling noinstr, The above hack only allows it
calling uaccess-safe functions, and the group of functions that is both
noinstr and uaccess-safe is 'small'.
But like I said yesterday, perhaps this wants it own annotation. Then we
can also verify the lack of any dynamic code.
So yes, yuck and a bad idea, ignore this ;-)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-28 8:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-27 10:40 [PATCH] objtool,x86: Verify poke_int3_handler() is self contained peterz
2020-07-27 11:21 ` Ingo Molnar
2020-07-27 12:50 ` peterz
2020-07-27 13:11 ` Ingo Molnar
2020-07-27 20:56 ` Thomas Gleixner
2020-07-28 8:34 ` peterz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).