CONFIG_RETHUNK int3 filling prevents kprobes in function body

CONFIG_RETHUNK int3 filling prevents kprobes in function body

[Masami Hiramatsu]

Hi Peter,

I found that the CONFIG_RETHUNK code (path_return) fills the unused bytes
with int3 for padding. Unfortunately, this prevents kprobes on the function
body after the return code (e.g. branch blocks placed behind the return.)

This is because kprobes decodes function body to ensure the probed address
is an instruction boundary, and if it finds the 0xcc (int3), it stops
decoding and reject probing because the int3 is usually used for a
software breakpoint and is replacing some other instruction. Without
recovering the instruction, it can not continue decoding safely.

Can we use another instruction for padding instead of INT3? (e.g. NOP or UD2)

Or, can I expect the instruction length in __return_sites[] are always 5?
If so, I can just skip 5 bytes if the address is in __return_sites[].

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: CONFIG_RETHUNK int3 filling prevents kprobes in function body

[Steven Rostedt]

On Sun, 4 Sep 2022 23:07:13 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> Can we use another instruction for padding instead of INT3? (e.g. NOP or UD2)
> 
> Or, can I expect the instruction length in __return_sites[] are always 5?
> If so, I can just skip 5 bytes if the address is in __return_sites[].

Perhaps another option is to have a table of where the padding is placed
(tagged), and that kprobes could check to see if the int3 is due to this
padding or not?

-- Steve

Re: CONFIG_RETHUNK int3 filling prevents kprobes in function body

[Peter Zijlstra]

On Mon, Sep 05, 2022 at 10:57:58AM -0400, Steven Rostedt wrote:
> On Sun, 4 Sep 2022 23:07:13 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > Can we use another instruction for padding instead of INT3? (e.g. NOP or UD2)
> > 
> > Or, can I expect the instruction length in __return_sites[] are always 5?
> > If so, I can just skip 5 bytes if the address is in __return_sites[].
> 
> Perhaps another option is to have a table of where the padding is placed
> (tagged), and that kprobes could check to see if the int3 is due to this
> padding or not?

I don't see need for that. If you want to be strict you can simply
follow the branches found earlier, if you want to be lazy, you can
decode until you run out of the symbol size.

Re: CONFIG_RETHUNK int3 filling prevents kprobes in function body

[Peter Zijlstra]

On Mon, Sep 05, 2022 at 05:10:27PM +0200, Peter Zijlstra wrote:
> On Mon, Sep 05, 2022 at 10:57:58AM -0400, Steven Rostedt wrote:
> > On Sun, 4 Sep 2022 23:07:13 +0900
> > Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > 
> > > Can we use another instruction for padding instead of INT3? (e.g. NOP or UD2)
> > > 
> > > Or, can I expect the instruction length in __return_sites[] are always 5?
> > > If so, I can just skip 5 bytes if the address is in __return_sites[].
> > 
> > Perhaps another option is to have a table of where the padding is placed
> > (tagged), and that kprobes could check to see if the int3 is due to this
> > padding or not?
> 
> I don't see need for that. If you want to be strict you can simply
> follow the branches found earlier, if you want to be lazy, you can
> decode until you run out of the symbol size.

Another lazy option is to teach the thing that 'ret' is followed by 0,1
or 4 'int3' instructions depending on CONFIG_SLS, CONFIG_RETHUNK, but
that'll get you into trouble with future SLS compiler options, like the
aforementioned JMP-SLS option.

Re: CONFIG_RETHUNK int3 filling prevents kprobes in function body

[Masami Hiramatsu]

On Mon, 5 Sep 2022 17:20:30 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Mon, Sep 05, 2022 at 05:10:27PM +0200, Peter Zijlstra wrote:
> > On Mon, Sep 05, 2022 at 10:57:58AM -0400, Steven Rostedt wrote:
> > > On Sun, 4 Sep 2022 23:07:13 +0900
> > > Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > > 
> > > > Can we use another instruction for padding instead of INT3? (e.g. NOP or UD2)
> > > > 
> > > > Or, can I expect the instruction length in __return_sites[] are always 5?
> > > > If so, I can just skip 5 bytes if the address is in __return_sites[].
> > > 
> > > Perhaps another option is to have a table of where the padding is placed
> > > (tagged), and that kprobes could check to see if the int3 is due to this
> > > padding or not?
> > 
> > I don't see need for that. If you want to be strict you can simply
> > follow the branches found earlier, if you want to be lazy, you can
> > decode until you run out of the symbol size.
> 
> Another lazy option is to teach the thing that 'ret' is followed by 0,1
> or 4 'int3' instructions depending on CONFIG_SLS, CONFIG_RETHUNK, but
> that'll get you into trouble with future SLS compiler options, like the
> aforementioned JMP-SLS option.

Yes, I agree... OK, let me try to decode branches to find the
nearest earlier target instruction.

Thank you!

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: CONFIG_RETHUNK int3 filling prevents kprobes in function body

[Peter Zijlstra]

On Sun, Sep 04, 2022 at 11:07:13PM +0900, Masami Hiramatsu wrote:
> Hi Peter,
> 
> I found that the CONFIG_RETHUNK code (path_return) fills the unused bytes
> with int3 for padding. Unfortunately, this prevents kprobes on the function
> body after the return code (e.g. branch blocks placed behind the return.)

Prior to that CONFIG_SLS would already use "ret; int3"

> This is because kprobes decodes function body to ensure the probed address
> is an instruction boundary, and if it finds the 0xcc (int3), it stops
> decoding and reject probing because the int3 is usually used for a
> software breakpoint and is replacing some other instruction. Without
> recovering the instruction, it can not continue decoding safely.

I can't follow this logic. Decoding the single byte int3 instruction is
trivial. If you want a sanity check, follow the branches you found while
decoding the instruction starting at +0.

> Can we use another instruction for padding instead of INT3? (e.g. NOP or UD2)

No. NOP is not a trap instruction and UD2 is longer than it needs to be.

Re: CONFIG_RETHUNK int3 filling prevents kprobes in function body

[Peter Zijlstra]

On Mon, Sep 05, 2022 at 05:09:16PM +0200, Peter Zijlstra wrote:
> On Sun, Sep 04, 2022 at 11:07:13PM +0900, Masami Hiramatsu wrote:
> > Hi Peter,
> > 
> > I found that the CONFIG_RETHUNK code (path_return) fills the unused bytes
> > with int3 for padding. Unfortunately, this prevents kprobes on the function
> > body after the return code (e.g. branch blocks placed behind the return.)
> 
> Prior to that CONFIG_SLS would already use "ret; int3"

FWIW, there is a compiler option pending to also stick an int3 after
every unconditional jmp.

Re: CONFIG_RETHUNK int3 filling prevents kprobes in function body

[Peter Zijlstra]

On Mon, Sep 05, 2022 at 05:09:16PM +0200, Peter Zijlstra wrote:

> > This is because kprobes decodes function body to ensure the probed address
> > is an instruction boundary, and if it finds the 0xcc (int3), it stops
> > decoding and reject probing because the int3 is usually used for a
> > software breakpoint and is replacing some other instruction. Without
> > recovering the instruction, it can not continue decoding safely.
> 
> I can't follow this logic. Decoding the single byte int3 instruction is
> trivial. If you want a sanity check, follow the branches you found while
> decoding the instruction starting at +0.

Specifically, kprobe is the only one scribbling random [*] instructions
with int3 in kernel text, so if kprobes doesn't know about the int3, it
must be padding.

[*] there's also static_call, jump_label and ftrace that use
text_poke_bp() to scribble instructions but those are well known
locations.

Re: CONFIG_RETHUNK int3 filling prevents kprobes in function body

[Masami Hiramatsu]

On Mon, 5 Sep 2022 17:52:29 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Mon, Sep 05, 2022 at 05:09:16PM +0200, Peter Zijlstra wrote:
> 
> > > This is because kprobes decodes function body to ensure the probed address
> > > is an instruction boundary, and if it finds the 0xcc (int3), it stops
> > > decoding and reject probing because the int3 is usually used for a
> > > software breakpoint and is replacing some other instruction. Without
> > > recovering the instruction, it can not continue decoding safely.
> > 
> > I can't follow this logic. Decoding the single byte int3 instruction is
> > trivial. If you want a sanity check, follow the branches you found while
> > decoding the instruction starting at +0.
> 
> Specifically, kprobe is the only one scribbling random [*] instructions
> with int3 in kernel text, so if kprobes doesn't know about the int3, it
> must be padding.

No, kgdb is also handles int3 for its breakpoint. Of course we can
ignore it or ask kgdb to expose the API to decode it.

> 
> [*] there's also static_call, jump_label and ftrace that use
> text_poke_bp() to scribble instructions but those are well known
> locations.

Yeah, but it is a temporal use, so user can try it again.

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: CONFIG_RETHUNK int3 filling prevents kprobes in function body

[Peter Zijlstra]

On Tue, Sep 06, 2022 at 10:33:29AM +0900, Masami Hiramatsu wrote:
> On Mon, 5 Sep 2022 17:52:29 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Mon, Sep 05, 2022 at 05:09:16PM +0200, Peter Zijlstra wrote:
> > 
> > > > This is because kprobes decodes function body to ensure the probed address
> > > > is an instruction boundary, and if it finds the 0xcc (int3), it stops
> > > > decoding and reject probing because the int3 is usually used for a
> > > > software breakpoint and is replacing some other instruction. Without
> > > > recovering the instruction, it can not continue decoding safely.
> > > 
> > > I can't follow this logic. Decoding the single byte int3 instruction is
> > > trivial. If you want a sanity check, follow the branches you found while
> > > decoding the instruction starting at +0.
> > 
> > Specifically, kprobe is the only one scribbling random [*] instructions
> > with int3 in kernel text, so if kprobes doesn't know about the int3, it
> > must be padding.
> 
> No, kgdb is also handles int3 for its breakpoint. Of course we can
> ignore it or ask kgdb to expose the API to decode it.

I'm thinking kgdb has worse issues anyway. Much of it seems to be
wishful thinking.