Merge pull request #1173 from jpoimboe/syscall-macros

Add syscall macros

doc/patch-author-guide.md

@@ -9,7 +9,27 @@ There are many pitfalls that can be encountered when creating a live patch.
 This document attempts to guide the patch creation process.  It's a work in
 progress.  If you find it useful, please contribute!
 
-Patch Analysis
+Table of contents
+=================
+
+- [Patch analysis](#patch-analysis)
+- [kpatch vs livepatch vs kGraft](#kpatch-vs-livepatch-vs-kgraft)
+- [Patch upgrades](#patch-upgrades)
+- [Data structure changes](#data-structure-changes)
+- [Data semantic changes](#data-semantic-changes)
+- [Init code changes](#init-code-changes)
+- [Header file changes](#header-file-changes)
+- [Dealing with unexpected changed functions](#dealing-with-unexpected-changed-functions)
+- [Removing references to static local variables](#removing-references-to-static-local-variables)
+- [Code removal](#code-removal)
+- [Once macros](#once-macros)
+- [inline implies notrace](#inline-implies-notrace)
+- [Jump labels](#jump-labels)
+- [Sibling calls](#sibling-calls)
+- [Exported symbol versioning](#exported-symbol-versioning)
+- [System calls](#system-calls)
+
+Patch analysis
 --------------
 
 kpatch provides _some_ guarantees, but it does not guarantee that all patches
@@ -759,6 +779,10 @@ if (static_key_enabled(&false_key))
 if (likely(static_key_enabled(&key)))
 ```
 
+Note that with Linux 5.8+, jump labels in patched functions are now supported
+when the static key was originally defined in the kernel proper (vmlinux).  The
+above error will not be seen unless the static key lives in a module.
+
 Sibling calls
 -------------
 
@@ -870,3 +894,14 @@ To track down specifically what caused a symbol CRC change, tools like
 detailed symbol definition report.  For a kpatch-build, kabi-dw can be modified
 to operate on .o object files (not just .ko and vmlinux files) and the
 `$CACHEDIR/tmp/{orig, patched}` directories compared.
+
+System calls
+------------
+
+Attempting to patch a syscall typically results in an error, due to a missing
+fentry hook in the inner `__do_sys##name()` function.  The fentry hook is
+missing because of the 'inline' annotation, which invokes 'notrace'.
+
+This problem can be worked around by adding `#include "kpatch-syscall.h"` and
+replacing the use of the `SYSCALL_DEFINE1` (or similar) macro with the
+`KPATCH_` prefixed version.

kmod/patch/kpatch-macros.h

@@ -4,6 +4,7 @@
 #include <linux/compiler.h>
 #include <linux/jiffies.h>
 #include <linux/version.h>
+#include "kpatch-syscall.h"
 
 /* upstream 33def8498fdd "treewide: Convert macro and uses of __section(foo) to __section("foo")" */
 #if LINUX_VERSION_CODE < KERNEL_VERSION(5, 10, 0)

kmod/patch/kpatch-syscall.h

@@ -0,0 +1,155 @@
+#ifndef __KPATCH_SYSCALL_H_
+#define __KPATCH_SYSCALL_H_
+
+#include "kpatch-macros.h"
+
+/*
+ * These kpatch-specific syscall definition macros can be used for patching a
+ * syscall.
+ *
+ * Attempting to patch a syscall typically results in an error, due to a
+ * missing fentry hook in the inner __do_sys##name() function.  The fentry hook
+ * is missing because of the 'inline' annotation, which invokes 'notrace'.
+ *
+ * These macros are copied almost verbatim from the kernel, the main difference
+ * being a 'kpatch' prefix added to the __do_sys##name() function name.  This
+ * causes kpatch-build to treat it as a new function (due to
+ * its new name), and its caller __se_sys##name() function is inlined by its own
+ * caller __x64_sys##name() function, which has an fentry hook.
+
+ * To patch a syscall, just replace the use of the SYSCALL_DEFINE1 (or similar)
+ * macro with the "KPATCH_" prefixed version.
+ */
+
+#define KPATCH_IGNORE_SYSCALL_SECTIONS					\
+	KPATCH_IGNORE_SECTION("__syscalls_metadata");			\
+	KPATCH_IGNORE_SECTION("_ftrace_events")
+
+#define KPATCH_SYSCALL_DEFINE1(name, ...) KPATCH_SYSCALL_DEFINEx(1, _##name, __VA_ARGS__)
+#define KPATCH_SYSCALL_DEFINE2(name, ...) KPATCH_SYSCALL_DEFINEx(2, _##name, __VA_ARGS__)
+#define KPATCH_SYSCALL_DEFINE3(name, ...) KPATCH_SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
+#define KPATCH_SYSCALL_DEFINE4(name, ...) KPATCH_SYSCALL_DEFINEx(4, _##name, __VA_ARGS__)
+#define KPATCH_SYSCALL_DEFINE5(name, ...) KPATCH_SYSCALL_DEFINEx(5, _##name, __VA_ARGS__)
+#define KPATCH_SYSCALL_DEFINE6(name, ...) KPATCH_SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
+
+#define KPATCH_SYSCALL_DEFINEx(x, sname, ...)				\
+	KPATCH_IGNORE_SYSCALL_SECTIONS;					\
+	__KPATCH_SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
+
+#ifdef CONFIG_X86_64
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 7, 0)
+
+#define __KPATCH_SYSCALL_DEFINEx(x, name, ...)				\
+	static long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));	\
+	static inline long __kpatch_do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
+	__X64_SYS_STUBx(x, name, __VA_ARGS__)				\
+	__IA32_SYS_STUBx(x, name, __VA_ARGS__)				\
+	static long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
+	{								\
+		long ret = __kpatch_do_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));\
+		__MAP(x,__SC_TEST,__VA_ARGS__);				\
+		__PROTECT(x, ret,__MAP(x,__SC_ARGS,__VA_ARGS__));	\
+		return ret;						\
+	}								\
+	static inline long __kpatch_do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
+
+#elif LINUX_VERSION_CODE >= KERNEL_VERSION(4, 17, 0)
+
+#define __KPATCH_SYSCALL_DEFINEx(x, name, ...)				\
+	asmlinkage long __x64_sys##name(const struct pt_regs *regs);	\
+	ALLOW_ERROR_INJECTION(__x64_sys##name, ERRNO);			\
+	static long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));	\
+	static inline long __kpatch_do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
+	asmlinkage long __x64_sys##name(const struct pt_regs *regs)	\
+	{								\
+		return __se_sys##name(SC_X86_64_REGS_TO_ARGS(x,__VA_ARGS__));\
+	}								\
+	__IA32_SYS_STUBx(x, name, __VA_ARGS__)				\
+	static long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
+	{								\
+		long ret = __kpatch_do_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));\
+		__MAP(x,__SC_TEST,__VA_ARGS__);				\
+		__PROTECT(x, ret,__MAP(x,__SC_ARGS,__VA_ARGS__));	\
+		return ret;						\
+	}								\
+	static inline long __kpatch_do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
+
+#else /* LINUX_VERSION_CODE < KERNEL_VERSION(4, 17, 0) */
+
+#define __KPATCH_SYSCALL_DEFINEx(x, name, ...)				\
+	asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));	\
+	static inline long __kpatch_SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
+	asmlinkage long SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
+	{								\
+		long ret = __kpatch_SYSC##name(__MAP(x,__SC_CAST,__VA_ARGS__));\
+		__MAP(x,__SC_TEST,__VA_ARGS__);				\
+		__PROTECT(x, ret,__MAP(x,__SC_ARGS,__VA_ARGS__));	\
+		return ret;						\
+	}								\
+	SYSCALL_ALIAS(sys##name, SyS##name);				\
+	static inline long __kpatch_SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__))
+
+#endif /* LINUX_VERSION_CODE */
+
+
+#elif defined(CONFIG_PPC64)
+
+#define __KPATCH_SYSCALL_DEFINEx(x, name, ...)				\
+	__diag_push();							\
+	__diag_ignore(GCC, 8, "-Wattribute-alias",			\
+		      "Type aliasing is used to sanitize syscall arguments");\
+	asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))	\
+		__attribute__((alias(__stringify(__se_sys##name))));	\
+	ALLOW_ERROR_INJECTION(sys##name, ERRNO);			\
+	static inline long __kpatch_do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
+	asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));	\
+	asmlinkage long __attribute__((optimize("-fno-optimize-sibling-calls")))\
+	__se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))			\
+	{								\
+		long ret = __kpatch_do_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));\
+		__MAP(x,__SC_TEST,__VA_ARGS__);				\
+		__PROTECT(x, ret,__MAP(x,__SC_ARGS,__VA_ARGS__));	\
+		return ret;						\
+	}								\
+	__diag_pop();							\
+	static inline long __kpatch_do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
+
+
+#elif defined(CONFIG_S390)
+
+#define __KPATCH_S390_SYS_STUBx(x, name, ...)					\
+	long __s390_sys##name(struct pt_regs *regs);				\
+	ALLOW_ERROR_INJECTION(__s390_sys##name, ERRNO);				\
+	long __s390_sys##name(struct pt_regs *regs)				\
+	{									\
+		long ret = __kpatch_do_sys##name(SYSCALL_PT_ARGS(x, regs,	\
+			__SC_COMPAT_CAST, __MAP(x, __SC_TYPE, __VA_ARGS__)));	\
+		__MAP(x,__SC_TEST,__VA_ARGS__);					\
+		return ret;							\
+	}
+
+#define __KPATCH_SYSCALL_DEFINEx(x, name, ...)						\
+	__diag_push();									\
+	__diag_ignore(GCC, 8, "-Wattribute-alias",					\
+		      "Type aliasing is used to sanitize syscall arguments");		\
+	long __s390x_sys##name(struct pt_regs *regs)					\
+		__attribute__((alias(__stringify(__se_sys##name))));			\
+	ALLOW_ERROR_INJECTION(__s390x_sys##name, ERRNO);				\
+	static inline long __kpatch_do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));	\
+	long __se_sys##name(struct pt_regs *regs);					\
+	__KPATCH_S390_SYS_STUBx(x, name, __VA_ARGS__)					\
+	long __se_sys##name(struct pt_regs *regs)					\
+	{										\
+		long ret = __kpatch_do_sys##name(SYSCALL_PT_ARGS(x, regs,		\
+				    __SC_CAST, __MAP(x, __SC_TYPE, __VA_ARGS__)));	\
+		__MAP(x,__SC_TEST,__VA_ARGS__);						\
+		return ret;								\
+	}										\
+	__diag_pop();									\
+	static inline long __kpatch_do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
+
+
+#endif /* CONFIG_X86_64 */
+
+#endif /* __KPATCH_SYSCALL_H_ */

test/integration/linux-5.10.11/syscall-LOADED.test

@@ -0,0 +1,3 @@
+#!/bin/bash
+
+uname -s | grep -q kpatch

test/integration/linux-5.10.11/syscall.patch

@@ -0,0 +1,21 @@
+diff --git a/kernel/sys.c b/kernel/sys.c
+index 2e2e3f378d97..05271fe26c89 100644
+--- a/kernel/sys.c
++++ b/kernel/sys.c
+@@ -1250,13 +1250,15 @@ static int override_release(char __user *release, size_t len)
+ 	return ret;
+ }
+ 
+-SYSCALL_DEFINE1(newuname, struct new_utsname __user *, name)
++#include "kpatch-syscall.h"
++KPATCH_SYSCALL_DEFINE1(newuname, struct new_utsname __user *, name)
+ {
+ 	struct new_utsname tmp;
+ 
+ 	down_read(&uts_sem);
+ 	memcpy(&tmp, utsname(), sizeof(tmp));
+ 	up_read(&uts_sem);
++	strcat(tmp.sysname, ".kpatch");
+ 	if (copy_to_user(name, &tmp, sizeof(tmp)))
+ 		return -EFAULT;
+ 

test/integration/rhel-7.9/syscall-LOADED.test

@@ -0,0 +1,3 @@
+#!/bin/bash
+
+uname -s | grep -q kpatch

test/integration/rhel-7.9/syscall.patch

@@ -0,0 +1,26 @@
+diff --git a/kernel/sys.c b/kernel/sys.c
+index 1fbf388279832..b5186aa83adfa 100644
+--- a/kernel/sys.c
++++ b/kernel/sys.c
+@@ -1392,14 +1392,18 @@ static int override_release(char __user *release, size_t len)
+ 	return ret;
+ }
+ 
+-SYSCALL_DEFINE1(newuname, struct new_utsname __user *, name)
++#include "kpatch-syscall.h"
++KPATCH_SYSCALL_DEFINE1(newuname, struct new_utsname __user *, name)
+ {
++	struct new_utsname tmp;
+ 	int errno = 0;
+ 
+ 	down_read(&uts_sem);
+-	if (copy_to_user(name, utsname(), sizeof *name))
+-		errno = -EFAULT;
++	memcpy(&tmp, utsname(), sizeof(tmp));
+ 	up_read(&uts_sem);
++	strcat(tmp.sysname, ".kpatch");
++	if (copy_to_user(name, &tmp, sizeof(tmp)))
++		errno = -EFAULT;
+ 
+ 	if (!errno && override_release(name->release, sizeof(name->release)))
+ 		errno = -EFAULT;

test/integration/rhel-8.4/syscall-LOADED.test

@@ -0,0 +1,3 @@
+#!/bin/bash
+
+uname -s | grep -q kpatch

test/integration/rhel-8.4/syscall.patch

@@ -0,0 +1,26 @@
+diff --git a/kernel/sys.c b/kernel/sys.c
+index 871c0848f05c8..479bf8725d2e6 100644
+--- a/kernel/sys.c
++++ b/kernel/sys.c
+@@ -1241,14 +1241,18 @@ static int override_release(char __user *release, size_t len)
+ 	return ret;
+ }
+ 
+-SYSCALL_DEFINE1(newuname, struct new_utsname __user *, name)
++#include "kpatch-syscall.h"
++KPATCH_SYSCALL_DEFINE1(newuname, struct new_utsname __user *, name)
+ {
++	struct new_utsname tmp;
+ 	int errno = 0;
+ 
+ 	down_read(&uts_sem);
+-	if (copy_to_user(name, utsname(), sizeof *name))
+-		errno = -EFAULT;
++	memcpy(&tmp, utsname(), sizeof(tmp));
+ 	up_read(&uts_sem);
++	strcat(tmp.sysname, ".kpatch");
++	if (copy_to_user(name, &tmp, sizeof(tmp)))
++		errno = -EFAULT;
+ 
+ 	if (!errno && override_release(name->release, sizeof(name->release)))
+ 		errno = -EFAULT;