public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Adam Zabrocki <pi3@pi3.com.pl>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	linux-kernel@vger.kernel.org,
	"Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
	Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	Solar Designer <solar@openwall.com>
Subject: Re: KRETPROBES are broken since kernel 5.8
Date: Thu, 10 Dec 2020 18:14:30 +0100	[thread overview]
Message-ID: <20201210171430.GA20584@pi3.com.pl> (raw)
In-Reply-To: <20201210220937.5232571bf5b03237e7018012@kernel.org>

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


  reply	other threads:[~2020-12-10 17:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-09 21:50 KRETPROBES are broken since kernel 5.8 Adam Zabrocki
2020-12-10  1:25 ` Masami Hiramatsu
2020-12-10  7:17   ` Adam Zabrocki
2020-12-10 13:09     ` Masami Hiramatsu
2020-12-10 17:14       ` Adam Zabrocki [this message]
2020-12-11  1:44         ` Masami Hiramatsu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201210171430.GA20584@pi3.com.pl \
    --to=pi3@pi3.com.pl \
    --cc=anil.s.keshavamurthy@intel.com \
    --cc=bp@alien8.de \
    --cc=davem@davemloft.net \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=naveen.n.rao@linux.ibm.com \
    --cc=solar@openwall.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox