jpoimboe · arges · Oct 17, 2014 · Oct 17, 2014 · Oct 21, 2014 · Oct 21, 2014
diff --git a/Makefile.inc b/Makefile.inc
@@ -11,7 +11,7 @@ MODULESDIR = $(DESTDIR)$(PREFIX)/$(LIBDIR)/kpatch
 LIBEXECDIR = $(DESTDIR)$(PREFIX)/libexec/kpatch
 DATADIR    = $(DESTDIR)$(PREFIX)/share/kpatch
 MANDIR     = $(DESTDIR)$(PREFIX)/share/man/man1
-DRACUTDIR  = $(DESTDIR)/usr/$(LIBDIR)/dracut/modules.d/99kpatch
+SYSTEMDDIR = $(DESTDIR)$(PREFIX)/lib/systemd/system
 
 .PHONY: all install clean
 .DEFAULT: all
diff --git a/README.md b/README.md
@@ -8,9 +8,6 @@ without having to wait for long-running tasks to complete, for users to log
 off, or for scheduled reboot windows.  It gives more control over uptime
 without sacrificing security or stability.
 
-kpatch is currently in active development.  For now, it should _not_ be used
-in production environments.
-
 **WARNING: Use with caution!  Kernel crashes, spontaneous reboots, and data loss
 may occur!**
 
@@ -28,7 +25,7 @@ Installation
 
 ###Prerequisites
 
-####Fedora 20
+####Fedora 21
 
 *NOTE: You'll need about 15GB of free disk space for the kpatch-build cache in
 `~/.kpatch` and for ccache.*
@@ -42,7 +39,7 @@ sudo yum install gcc kernel-devel elfutils elfutils-devel
 Install the dependencies for the "kpatch-build" command:
 
 ```bash
-sudo yum install rpmdevtools pesign yum-utils openssl wget
+sudo yum install rpmdevtools pesign yum-utils openssl wget numactl-devel
 sudo yum-builddep kernel
 sudo debuginfo-install kernel
 
@@ -68,7 +65,7 @@ Install the dependencies for the "kpatch-build" command:
 sudo yum-config-manager --enable rhel-7-server-optional-rpms
 sudo yum install rpmdevtools pesign yum-utils zlib-devel \
   binutils-devel newt-devel python-devel perl-ExtUtils-Embed \
-  audit-libs-devel numactl-devel pciutils-devel bison
+  audit-libs-devel numactl-devel pciutils-devel bison ncurses-devel
 
 sudo yum-builddep kernel
 sudo debuginfo-install kernel
@@ -397,12 +394,6 @@ function's arguments and stack, and "returns" to the new function.
 Limitations
 -----------
 
-- Patches to functions which are always on the stack of at least one
-  process in the system are not supported.  Examples: schedule(),
-  sys_poll(), sys_select(), sys_read(), sys_nanosleep().  Attempting to
-  apply such a patch will cause the insmod of the patch module to return
-  an error.
-
 - Patches which modify init functions (annotated with `__init`) are not
   supported.  kpatch-build will return an error if the patch attempts
   to do so.
@@ -431,6 +422,21 @@ Limitations
 Frequently Asked Questions
 --------------------------
 
+**Q. What's the relationship between kpatch and the upstream Linux live kernel
+patching component (livepatch)?**
+
+Starting with Linux 4.0, the Linux kernel has livepatch, which is a new
+converged live kernel patching framework.  Livepatch is similar in
+functionality to the kpatch core module, though it doesn't yet have all the
+features that kpatch does.
+
+kpatch-build already works with both livepatch and kpatch.  If your kernel has
+CONFIG\_LIVEPATCH enabled, it detects that and builds a patch module in the
+livepatch format.  Otherwise it builds a kpatch patch module.
+
+Soon the kpatch script will also support both patch module formats (TODO issue
+[#479](https://github.com/dynup/kpatch/issues/479)).
+
 **Q. Isn't this just a virus/rootkit injection framework?**
 
 kpatch uses kernel modules to replace code.  It requires the `CAP_SYS_MODULE`
@@ -511,12 +517,6 @@ We hope to make the following changes to other projects:
 - kernel:
 	- ftrace improvements to close any windows that would allow a patch to
 	  be inadvertently disabled
-	- hot patch taint flag
-	- possibly the kpatch core module itself
-
-- crash:
-	- point it to where the patch modules and corresponding debug symbols
-	  live on the file system
 
 **Q: Is it possible to register a function that gets called atomically with
 `stop_machine` when the patch module loads and unloads?**
@@ -549,15 +549,25 @@ There could be a variety of reasons for this, such as:
 - The compiler decided to inline a changed function, resulting in the outer
   function getting recompiled.  This is common in the case where the inner
   function is static and is only called once.
-- The function uses a WARN() or WARN_ON() macro.  These macros embed the source
-  code line number (`__LINE__`) into an instruction.  If a function was changed
-  higher up in the file, it will affect the line numbers for all subsequent
-  WARN calls in the file, resulting in recompilation of their functions.  If
-  this happens to you, you can usually just ignore it, as patching a few extra
-  functions isn't typically a problem.  If it becomes a problem for whatever
-  reason, you can change the source patch to redefine the WARN macro for the
-  affected files, such that it hard codes the old line number instead of using
-  `__LINE__`, for example.
+
+**Q. How do I patch a function which is always on the stack of at least one
+task, such as schedule(), sys_poll(), sys_select(), sys_read(),
+sys_nanosleep(), etc?**
+
+- If you're sure it would be safe for the old function and the new function to
+  run simultaneously, use the `KPATCH_FORCE_UNSAFE` macro to skip the
+  activeness safety check for the function.  See `kmod/patch/kpatch-macros.h`
+  for more details.
+
+**Q. Are patching of kernel modules supported?**
+
+- Yes.
+
+**Q. Can you patch out-of-tree modules?**
+
+- Yes, though it's currently a bit of a manual process.  See this
+  [message](https://www.redhat.com/archives/kpatch/2015-June/msg00004.html) on
+  the kpatch mailing list for more information.
 
 
 Get involved

diff --git a/contrib/Makefile b/contrib/Makefile
@@ -3,10 +3,11 @@ include ../Makefile.inc
 all:
 
 install: all
-	$(INSTALL) -d $(DRACUTDIR)
-	$(INSTALL) module-setup.sh kpatch-load-all.sh $(DRACUTDIR)
+	$(INSTALL) -d $(SYSTEMDDIR)
+	$(INSTALL) -m 0644 kpatch.service $(SYSTEMDDIR)
+	sed -i 's~PREFIX~$(PREFIX)~' $(SYSTEMDDIR)/kpatch.service
 
 uninstall:
-	$(RM) -R $(DRACUTDIR)
+	$(RM) $(SYSTEMDDIR)/kpatch.service
 
 clean:
diff --git a/contrib/kpatch-load-all.sh b/contrib/kpatch-load-all.sh
diff --git a/contrib/kpatch.service b/contrib/kpatch.service
@@ -0,0 +1,11 @@
+[Unit]
+Description="Apply kpatch kernel patches"
+
+[Service]
+Type=oneshot
+RemainAfterExit=yes
+ExecStart=PREFIX/sbin/kpatch load --all
+ExecStop=PREFIX/sbin/kpatch unload --all
+
+[Install]
+WantedBy=multi-user.target
diff --git a/contrib/kpatch.spec b/contrib/kpatch.spec
@@ -1,6 +1,6 @@
 Name: kpatch
 Summary: Dynamic kernel patching
-Version: 0.2.1
+Version: 0.2.2
 License: GPLv2 
 Group: System Environment/Kernel
 URL: http://github.com/dynup/kpatch
@@ -61,16 +61,14 @@ sacrificing security or stability.
 
 %prep
 %setup -q 
-cp Makefile.inc Makefile.inc.ORG
-%{__sed} 's|/usr/local|/%{_usr}|' Makefile.inc.ORG > Makefile.inc
 
 %build
 make %{_smp_mflags} 
 
 %install
 rm -rf %{buildroot}
 
-make install DESTDIR=%{buildroot}
+make install PREFIX=/%{_usr} DESTDIR=%{buildroot}
 
 %clean
 rm -rf %{buildroot}
@@ -80,8 +78,7 @@ rm -rf %{buildroot}
 %doc COPYING README.md
 %{_sbindir}/kpatch
 %{_mandir}/man1/kpatch.1*
-%{_usr}/lib/dracut/modules.d/99%{name}/*
-
+%{_usr}/lib/systemd/system/*
 
 %files %{KVER}
 %defattr(-,root,root,-)
@@ -95,6 +92,9 @@ rm -rf %{buildroot}
 %{_mandir}/man1/kpatch-build.1*
 
 %changelog
+* Wed Dec 3 2014 Josh Poimboeuf <jpoimboe@redhat.com> - 0.2.2-1
+- rebased to current version
+
 * Tue Sep 2 2014 Josh Poimboeuf <jpoimboe@redhat.com> - 0.2.1-1
 - rebased to current version
 

diff --git a/contrib/module-setup.sh b/contrib/module-setup.sh
diff --git a/examples/tcp_cubic-better-follow-cubic-curve-converted.patch b/examples/tcp_cubic-better-follow-cubic-curve-converted.patch
@@ -0,0 +1,105 @@
+The original patch changes the initialization of 'cubictcp' instance of
+struct tcp_congestion_ops ('cubictcp.cwnd_event' field). Kpatch
+intentionally rejects to process such changes.
+
+This modification of the patch uses Kpatch load/unload hooks to set
+'cubictcp.cwnd_event' when the binary patch is loaded and reset it to NULL
+when the patch is unloaded.
+
+It is still needed to check if changing that field could be problematic
+due to concurrency issues, etc.
+
+'cwnd_event' callback is used only via tcp_ca_event() function.
+
+include/net/tcp.h:
+
+static inline void tcp_ca_event(struct sock *sk, const enum tcp_ca_event event)
+{
+	const struct inet_connection_sock *icsk = inet_csk(sk);
+
+	if (icsk->icsk_ca_ops->cwnd_event)
+		icsk->icsk_ca_ops->cwnd_event(sk, event);
+}
+
+In turn, tcp_ca_event() is called in a number of places in
+net/ipv4/tcp_output.c and net/ipv4/tcp_input.c.
+
+One problem with this modification of the patch is that it may not be safe
+to unload it. If it is possible for tcp_ca_event() to run concurrently with
+the unloading of the patch, it may happen that 'icsk->icsk_ca_ops->cwnd_event'
+is the address of bictcp_cwnd_event() when tcp_ca_event() checks it but is
+set to NULL right after. So 'icsk->icsk_ca_ops->cwnd_event(sk, event)' would
+result in a kernel oops.
+
+Whether such scenario is possible or not, it should be analyzed. If it is,
+then at least, the body of tcp_ca_event() should be made atomic w.r.t.
+changing 'cwnd_event' in the patch somehow. Perhaps, RCU could be suitable
+for that: a read-side critical section for the body of tcp_ca_event() with
+a single read of icsk->icsk_ca_ops->cwnd_event pointer with rcu_dereference().
+The pointer could be set by the patch with rcu_assign_pointer().
+
+An alternative suggested by Josh Poimboeuf would be to patch the functions
+that call 'cwnd_event' callback (tcp_ca_event() in this case) so that they
+call bictcp_cwnd_event() directly when they detect the cubictcp struct [1].
+
+Note that tcp_ca_event() is inlined in a number of places, so the binary
+patch will provide replacements for all of the corresponding functions
+rather than for just one. It is still needed to check if replacing these
+functions in runtime is safe.
+
+References:
+[1] https://www.redhat.com/archives/kpatch/2015-September/msg00005.html
+
+diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
+index 894b7ce..9bff8a0 100644
+--- a/net/ipv4/tcp_cubic.c
++++ b/net/ipv4/tcp_cubic.c
+@@ -153,6 +153,27 @@ static void bictcp_init(struct sock *sk)
+ 		tcp_sk(sk)->snd_ssthresh = initial_ssthresh;
+ }
+
++static void bictcp_cwnd_event(struct sock *sk, enum tcp_ca_event event)
++{
++	if (event == CA_EVENT_TX_START) {
++		struct bictcp *ca = inet_csk_ca(sk);
++		u32 now = tcp_time_stamp;
++		s32 delta;
++
++		delta = now - tcp_sk(sk)->lsndtime;
++
++		/* We were application limited (idle) for a while.
++		 * Shift epoch_start to keep cwnd growth to cubic curve.
++		 */
++		if (ca->epoch_start && delta > 0) {
++			ca->epoch_start += delta;
++			if (after(ca->epoch_start, now))
++				ca->epoch_start = now;
++		}
++		return;
++	}
++}
++
+ /* calculate the cubic root of x using a table lookup followed by one
+  * Newton-Raphson iteration.
+  * Avg err ~= 0.195%
+@@ -444,6 +465,20 @@ static struct tcp_congestion_ops cubictcp __read_mostly = {
+ 	.name		= "cubic",
+ };
+
++void kpatch_load_cubictcp_cwnd_event(void)
++{
++	cubictcp.cwnd_event = bictcp_cwnd_event;
++}
++
++void kpatch_unload_cubictcp_cwnd_event(void)
++{
++	cubictcp.cwnd_event = NULL;
++}
++
++#include "kpatch-macros.h"
++KPATCH_LOAD_HOOK(kpatch_load_cubictcp_cwnd_event);
++KPATCH_UNLOAD_HOOK(kpatch_unload_cubictcp_cwnd_event);
++
+ static int __init cubictcp_register(void)
+ {
+ 	BUILD_BUG_ON(sizeof(struct bictcp) > ICSK_CA_PRIV_SIZE);