Live-Patching Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] Revert "kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled"
@ 2020-07-17 18:29 Josh Poimboeuf
  2020-07-20  3:35 ` Joe Lawrence
  2020-07-21 11:17 ` Miroslav Benes
  0 siblings, 2 replies; 9+ messages in thread
From: Josh Poimboeuf @ 2020-07-17 18:29 UTC (permalink / raw)
  To: live-patching; +Cc: linux-kernel, Peter Zijlstra, Randy Dunlap

Use of the new -flive-patching flag was introduced with the following
commit:

  43bd3a95c98e ("kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled")

This flag has several drawbacks:

- It disables some optimizations, so it can have a negative effect on
  performance.

- According to the GCC documentation it's not compatible with LTO, which
  will become a compatibility issue as LTO support gets upstreamed in
  the kernel.

- It was intended to be used for source-based patch generation tooling,
  as opposed to binary-based patch generation tooling (e.g.,
  kpatch-build).  It probably should have at least been behind a
  separate config option so as not to negatively affect other livepatch
  users.

- Clang doesn't have the flag, so as far as I can tell, this method of
  generating patches is incompatible with Clang, which like LTO is
  becoming more mainstream.

- It breaks GCC's implicit noreturn detection for local functions.  This
  is the cause of several "unreachable instruction" objtool warnings.

- The broken noreturn detection is an obvious GCC regression, but we
  haven't yet gotten GCC developers to acknowledge that, which doesn't
  inspire confidence in their willingness to keep the feature working as
  optimizations are added or changed going forward.

- While there *is* a distro which relies on this flag for their distro
  livepatch module builds, there's not a publicly documented way to
  create safe livepatch modules with it.  Its use seems to be based on
  tribal knowledge.  It serves no benefit to those who don't know how to
  use it.

  (In fact, I believe the current livepatch documentation and samples
  are misleading and dangerous, and should be corrected.  Or at least
  amended with a disclaimer.  But I don't feel qualified to make such
  changes.)

Also, we have an idea for using objtool to detect function changes,
which could potentially obsolete the need for this flag anyway.

At this point the flag has no benefits for upstream which would
counteract the above drawbacks.  Revert it until it becomes more ready.

This reverts commit 43bd3a95c98e1a86b8b55d97f745c224ecff02b9.

Fixes: 43bd3a95c98e ("kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled")
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---

NOTE: I tried to be objective, factual, and thorough, to the best of my
knowledge.  Any suggestions for corrections to the commit message are
definitely welcome.

 Makefile | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/Makefile b/Makefile
index 0b5f8538bde5..3b37d25aa028 100644
--- a/Makefile
+++ b/Makefile
@@ -876,10 +876,6 @@ KBUILD_CFLAGS_KERNEL += -ffunction-sections -fdata-sections
 LDFLAGS_vmlinux += --gc-sections
 endif
 
-ifdef CONFIG_LIVEPATCH
-KBUILD_CFLAGS += $(call cc-option, -flive-patching=inline-clone)
-endif
-
 ifdef CONFIG_SHADOW_CALL_STACK
 CC_FLAGS_SCS	:= -fsanitize=shadow-call-stack
 KBUILD_CFLAGS	+= $(CC_FLAGS_SCS)
-- 
2.25.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17 18:29 [PATCH] Revert "kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled" Josh Poimboeuf
2020-07-20  3:35 ` Joe Lawrence
2020-07-20  8:50   ` Kamalesh Babulal
2020-07-20 11:47     ` Joe Lawrence
2020-07-21 11:27   ` Miroslav Benes
2020-07-21 11:17 ` Miroslav Benes
2020-08-06  9:24   ` Petr Mladek
2020-09-01 17:24     ` Josh Poimboeuf
2020-09-03 10:02       ` Petr Mladek

Live-Patching Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/live-patching/0 live-patching/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 live-patching live-patching/ https://lore.kernel.org/live-patching \
		live-patching@vger.kernel.org
	public-inbox-index live-patching

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.live-patching


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git