Re: KRETPROBES are broken since kernel 5.8

From: Adam Zabrocki
Date: Thu Dec 10 2020 - 12:41:13 EST


Hi,

> > However, there might be another issue which I wanted to brought / discuss -
> > problem with optimizer. Until kernel 5.9 KRETPROBE on e.g.
> > 'ftrace_enable_sysctl' function was correctly optimized without any problems.
>
> Did you check it on other functions? Did you see it only on the "ftrace_enable_sysctl"?
>

Yes, I see it in most of the functions with padding.

> > Since 5.9 it can't be optimized anynmore. I didn't see any changes in the
> > sources regarding the optimizer, neither function itself.
> > When I looked at the generated vmlinux binary, I can see that GCC generated
> > padding at the end of this function using INT3 opcode:
> >
> > ...
> > ffffffff8130528b: 41 bd f0 ff ff ff mov $0xfffffff0,%r13d
> > ffffffff81305291: e9 fe fe ff ff jmpq ffffffff81305194 <ftrace_enable_sysctl+0x114>
> > ffffffff81305296: cc int3
> > ffffffff81305297: cc int3
> > ffffffff81305298: cc int3
> > ffffffff81305299: cc int3
> > ffffffff8130529a: cc int3
> > ffffffff8130529b: cc int3
> > ffffffff8130529c: cc int3
> > ffffffff8130529d: cc int3
> > ffffffff8130529e: cc int3
> > ffffffff8130529f: cc int3
>
> So these int3 is generated by GCC for padding, right?
>

I've just browsed a few commits and I've found that one:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7705dc8557973d8ad8f10840f61d8ec805695e9e

It looks like INT3 is now a default padding used by linker.

> > However, that's not the case here. INT3_INSN_OPCODE is placed at the end of
> > the function as padding (and protect from NOP-padding problems).
> >
> > I wonder, if optimizer should take this special case into account? Is it worth
> > to still optimize such functions when they are padded with INT3?
>
> Indeed. I expected int3 is used from other subsystems (e.g. kgdb) and,
> in that case the optimization can confuse them.

Right. The same can happen when text section is being actively modified.
However, this case could be covered by running the optimizer logic under
text_mutex.

> But if the gcc uses int3 to pad the room between functions, it should be
> reconsidered.
>

Looks like it's a default behavior now.

> Thank you,
>
> > If it is OK, we should backport those to stable tree.
>
> Agreed.

It is also important to make sure that distro kernels would pick-up such
backported fix.

Thanks,
Adam

--
pi3 (pi3ki31ny) - pi3 (at) itsec pl
http://pi3.com.pl