2020-07-17 03:05:23

by Jarkko Sakkinen

Subject: [PATCH v4 0/7] arch/x86: kprobes: Remove MODULES dependency

Remove MODULES dependency by migrating from module_alloc() to the new
text_alloc() API. Essentially these changes provide preliminaries for
allowing to compile a static kernel with a proper tracing support.

The same API can be used later on in other sites that allocate space for
trampolines, and trivially scaled to other arch's. An arch can inform
with CONFIG_ARCH_HAS_TEXT_ALLOC that it's providing implementation for
text_alloc().

I tested this by creating a trivial (x86_64_defconfig) kernel and initrd
(BusyBox) and then run the most basic kprobe:

# ./kprobe p:do_sys_open
Tracing kprobe do_sys_open. Ctrl-C to end.
cat-1018 [000] .... 277.635966: do_sys_open: (do_sys_open+0x0/0x80)
cat-1018 [000] .... 277.635966: do_sys_open: (do_sys_open+0x0/0x80)
cat-1018 [000] .... 277.636966: do_sys_open: (do_sys_open+0x0/0x80)
cat-1018 [000] .... 277.636966: do_sys_open: (do_sys_open+0x0/0x80)
cat-1018 [000] .... 277.636966: do_sys_open: (do_sys_open+0x0/0x80)
cat-1018 [000] .... 277.636966: do_sys_open: (do_sys_open+0x0/0x80)
cat-1018 [000] .... 277.636966: do_sys_open: (do_sys_open+0x0/0x80)
cat-1018 [000] .... 277.636966: do_sys_open: (do_sys_open+0x0/0x80)
cat-1018 [000] .... 277.636966: do_sys_open: (do_sys_open+0x0/0x80)
cat-1018 [000] .... 277.640966: do_sys_open: (do_sys_open+0x0/0x80)
cat-1018 [000] .... 277.654963: do_sys_open: (do_sys_open+0x0/0x80)

I did only "sed -i 's/=m/=y/' .config" and disabled CONFIG_MODULES. The
test was run under QEMU:

qemu-system-x86_64 -kernel output/images/bzImage \
-m 1G -initrd output/images/rootfs.cpio \
-append "root=/dev/sda rw console=ttyS0,115200 acpi=off nokaslr" \
-serial stdio -display none

v3:
* Make text_alloc() API disjoint.
* Remove all the possible extra clutter not absolutely required and
split into more logical pieces.

Jarkko Sakkinen (7):
module: Add lock_modules() and unlock_modules()
kprobes: Use lock_modules() and unlock_modules()
vmalloc: Add text_alloc() and text_free()
arch/x86: Implement text_alloc() and text_free()
arch/x86: kprobes: Use text_alloc() in alloc_insn_page()
kprobes: Use text_alloc() and text_free()
kprobes: Flag out CONFIG_MODULES dependent code

arch/Kconfig | 2 +-
arch/x86/Kconfig | 3 ++
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/kprobes/core.c | 8 +----
arch/x86/kernel/text_alloc.c | 41 ++++++++++++++++++++++++
include/linux/module.h | 32 ++++++++++++++-----
include/linux/vmalloc.h | 23 ++++++++++++++
kernel/kprobes.c | 57 +++++++++++++++++++++-------------
kernel/trace/trace_kprobe.c | 20 +++++++++---
9 files changed, 146 insertions(+), 41 deletions(-)
create mode 100644 arch/x86/kernel/text_alloc.c

--
2.25.1

2020-07-17 03:05:52

by Jarkko Sakkinen

Subject: [PATCH v4 1/7] module: Add lock_modules() and unlock_modules()

Add wrapper functions for acquiring module_mutex so that the locking can
be implicitly compiled out when CONFIG_MODULES is not enabled.

Cc: Andi Kleen <[email protected]>
Suggested-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
include/linux/module.h | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/include/linux/module.h b/include/linux/module.h
index 2e6670860d27..8850b9692b8f 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -705,6 +705,16 @@ static inline bool is_livepatch_module(struct module *mod)
bool is_module_sig_enforced(void);
void set_module_sig_enforced(void);

+static inline void lock_modules(void)
+{
+ mutex_lock(&module_mutex);
+}
+
+static inline void unlock_modules(void)
+{
+ mutex_unlock(&module_mutex);
+}
+
#else /* !CONFIG_MODULES... */

static inline struct module *__module_address(unsigned long addr)
@@ -852,6 +862,14 @@ void *dereference_module_function_descriptor(struct module *mod, void *ptr)
return ptr;
}

+static inline void lock_modules(void)
+{
+}
+
+static inline void unlock_modules(void)
+{
+}
+
#endif /* CONFIG_MODULES */

#ifdef CONFIG_SYSFS
--
2.25.1

2020-07-17 03:06:08

by Jarkko Sakkinen

Subject: [PATCH v4 2/7] kprobes: Use lock_modules() and unlock_modules()

Use lock_modules() and unlock_modules() in order to remove compile time
dependency to the module subsystem.

Cc: Andi Kleen <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
kernel/kprobes.c | 4 ++--
kernel/trace/trace_kprobe.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 2e97febeef77..4e46d96d4e16 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -564,7 +564,7 @@ static void kprobe_optimizer(struct work_struct *work)
cpus_read_lock();
mutex_lock(&text_mutex);
/* Lock modules while optimizing kprobes */
- mutex_lock(&module_mutex);
+ lock_modules();

/*
* Step 1: Unoptimize kprobes and collect cleaned (unused and disarmed)
@@ -589,7 +589,7 @@ static void kprobe_optimizer(struct work_struct *work)
/* Step 4: Free cleaned kprobes after quiesence period */
do_free_cleaned_kprobes();

- mutex_unlock(&module_mutex);
+ unlock_modules();
mutex_unlock(&text_mutex);
cpus_read_unlock();

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index aefb6065b508..710ec6a6aa8f 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -122,9 +122,9 @@ static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
if (!p)
return true;
*p = '\0';
- mutex_lock(&module_mutex);
+ lock_modules();
ret = !!find_module(tk->symbol);
- mutex_unlock(&module_mutex);
+ unlock_modules();
*p = ':';

return ret;
--
2.25.1

2020-07-17 03:06:36

by Jarkko Sakkinen

Subject: [PATCH v4 3/7] vmalloc: Add text_alloc() and text_free()

Introduce functions for allocating memory for dynamic trampolines, such
as kprobes. An arch can promote the availability of these functions with
CONFIG_ARCH_HAS_TEXT_ALLOC. Provide default/fallback implementation
wrapping module_alloc() and module_memfree().

Cc: Andi Kleen <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Suggested-by: Peter Zijlstra <[email protected]>
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
include/linux/vmalloc.h | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 0221f852a7e1..e981436e30b6 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -9,6 +9,7 @@
#include <asm/page.h> /* pgprot_t */
#include <linux/rbtree.h>
#include <linux/overflow.h>
+#include <linux/moduleloader.h>

#include <asm/vmalloc.h>

@@ -249,4 +250,26 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
int register_vmap_purge_notifier(struct notifier_block *nb);
int unregister_vmap_purge_notifier(struct notifier_block *nb);

+#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
+/*
+ * Allocate memory to be used for dynamic trampoline code.
+ */
+void *text_alloc(unsigned long size);
+
+/*
+ * Free memory returned from text_alloc().
+ */
+void text_free(void *region);
+#else
+static inline void *text_alloc(unsigned long size)
+{
+ return module_alloc(size);
+}
+
+static inline void text_free(void *region)
+{
+ module_memfree(region);
+}
+#endif
+
#endif /* _LINUX_VMALLOC_H */
--
2.25.1

2020-07-17 03:06:44

by Jarkko Sakkinen

Subject: [PATCH v4 4/7] arch/x86: Implement text_alloc() and text_free()

Implement text_alloc() and text_free() with vmalloc() and vfree(), thus
dropping the dependency to the module subsystem.

Cc: Masami Hiramatsu <[email protected]>
Cc: Andi Kleen <[email protected]>
Suggested-by: Peter Zijlstra <[email protected]>
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
arch/x86/Kconfig | 3 +++
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/text_alloc.c | 41 ++++++++++++++++++++++++++++++++++++
3 files changed, 45 insertions(+)
create mode 100644 arch/x86/kernel/text_alloc.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0dea7fdd7a00..a4ee5d1300f6 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2035,6 +2035,9 @@ config KEXEC_FILE
config ARCH_HAS_KEXEC_PURGATORY
def_bool KEXEC_FILE

+config ARCH_HAS_TEXT_ALLOC
+ def_bool y
+
config KEXEC_SIG
bool "Verify kernel signature during kexec_file_load() syscall"
depends on KEXEC_FILE
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index e77261db2391..fa1415424ae3 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -68,6 +68,7 @@ obj-y += tsc.o tsc_msr.o io_delay.o rtc.o
obj-y += pci-iommu_table.o
obj-y += resource.o
obj-y += irqflags.o
+obj-y += text_alloc.o

obj-y += process.o
obj-y += fpu/
diff --git a/arch/x86/kernel/text_alloc.c b/arch/x86/kernel/text_alloc.c
new file mode 100644
index 000000000000..c8e5296ebceb
--- /dev/null
+++ b/arch/x86/kernel/text_alloc.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Kernel module help for x86.
+ * Copyright (C) 2001 Rusty Russell.
+ */
+
+#include <linux/kasan.h>
+#include <linux/mm.h>
+#include <linux/moduleloader.h>
+#include <linux/vmalloc.h>
+#include <asm/setup.h>
+
+void *text_alloc(unsigned long size)
+{
+ void *p;
+
+ if (PAGE_ALIGN(size) > MODULES_LEN)
+ return NULL;
+
+ p = __vmalloc_node_range(size, MODULE_ALIGN, MODULES_VADDR, MODULES_END,
+ GFP_KERNEL, PAGE_KERNEL, 0, NUMA_NO_NODE,
+ __builtin_return_address(0));
+
+ if (p && (kasan_module_alloc(p, size) < 0)) {
+ vfree(p);
+ return NULL;
+ }
+
+ return p;
+}
+
+void text_free(void *region)
+{
+ /*
+ * This memory may be RO, and freeing RO memory in an interrupt is not
+ * supported by vmalloc.
+ */
+ WARN_ON(in_interrupt());
+
+ vfree(region);
+}
--
2.25.1

2020-07-17 03:06:53

by Jarkko Sakkinen

Subject: [PATCH v4 6/7] kprobes: Use text_alloc() and text_free()

Use text_alloc() and text_free(). Those arch's that provide their
allocators will benefit from this because they can provide their custom
allocator and render out the compile time dep to the module subsystem.
Other arch's will continue to work as the fallback implementations call
module_alloc() and module_memfree().

Cc: Andi Kleen <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
kernel/kprobes.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 4e46d96d4e16..f73cf71ef47d 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -40,6 +40,7 @@
#include <asm/cacheflush.h>
#include <asm/errno.h>
#include <linux/uaccess.h>
+#include <linux/vmalloc.h>

#define KPROBE_HASH_BITS 6
#define KPROBE_TABLE_SIZE (1 << KPROBE_HASH_BITS)
@@ -111,12 +112,12 @@ enum kprobe_slot_state {

void __weak *alloc_insn_page(void)
{
- return module_alloc(PAGE_SIZE);
+ return text_alloc(PAGE_SIZE);
}

void __weak free_insn_page(void *page)
{
- module_memfree(page);
+ text_free(page);
}

struct kprobe_insn_cache kprobe_insn_slots = {
--
2.25.1

2020-07-17 03:08:43

by Jarkko Sakkinen

Subject: [PATCH v4 7/7] kprobes: Flag out CONFIG_MODULES dependent code

Remove CONFIG_MODULES dependency by flagging out the dependent code. This
allows to use kprobes in a kernel without support for loadable modules,
which could be useful for a test kernel or perhaps an embedded kernel.

Cc: Andi Kleen <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
arch/Kconfig | 2 +-
include/linux/module.h | 14 +++++------
kernel/kprobes.c | 48 +++++++++++++++++++++++--------------
kernel/trace/trace_kprobe.c | 16 +++++++++++--
4 files changed, 52 insertions(+), 28 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 8cc35dc556c7..e3d6b6868ccb 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -61,7 +61,7 @@ config OPROFILE_NMI_TIMER

config KPROBES
bool "Kprobes"
- depends on MODULES
+ depends on MODULES || ARCH_HAS_TEXT_ALLOC
depends on HAVE_KPROBES
select KALLSYMS
help
diff --git a/include/linux/module.h b/include/linux/module.h
index 8850b9692b8f..22c646cff6bd 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -290,6 +290,13 @@ extern typeof(name) __mod_##type##__##name##_device_table \

struct notifier_block;

+enum module_state {
+ MODULE_STATE_LIVE, /* Normal state. */
+ MODULE_STATE_COMING, /* Full formed, running module_init. */
+ MODULE_STATE_GOING, /* Going away. */
+ MODULE_STATE_UNFORMED, /* Still setting it up. */
+};
+
#ifdef CONFIG_MODULES

extern int modules_disabled; /* for sysctl */
@@ -305,13 +312,6 @@ struct module_use {
struct module *source, *target;
};

-enum module_state {
- MODULE_STATE_LIVE, /* Normal state. */
- MODULE_STATE_COMING, /* Full formed, running module_init. */
- MODULE_STATE_GOING, /* Going away. */
- MODULE_STATE_UNFORMED, /* Still setting it up. */
-};
-
struct mod_tree_node {
struct module *mod;
struct latch_tree_node node;
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index f73cf71ef47d..6ee7286b1f7e 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1609,6 +1609,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
goto out;
}

+#ifdef CONFIG_MODULES
/*
* If the module freed .init.text, we couldn't insert
* kprobes in there.
@@ -1619,6 +1620,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
*probed_mod = NULL;
ret = -ENOENT;
}
+#endif
}
out:
preempt_enable();
@@ -2215,24 +2217,6 @@ int kprobe_add_area_blacklist(unsigned long start, unsigned long end)
return 0;
}

-/* Remove all symbols in given area from kprobe blacklist */
-static void kprobe_remove_area_blacklist(unsigned long start, unsigned long end)
-{
- struct kprobe_blacklist_entry *ent, *n;
-
- list_for_each_entry_safe(ent, n, &kprobe_blacklist, list) {
- if (ent->start_addr < start || ent->start_addr >= end)
- continue;
- list_del(&ent->list);
- kfree(ent);
- }
-}
-
-static void kprobe_remove_ksym_blacklist(unsigned long entry)
-{
- kprobe_remove_area_blacklist(entry, entry + 1);
-}
-
int __init __weak arch_populate_kprobe_blacklist(void)
{
return 0;
@@ -2275,6 +2259,25 @@ static int __init populate_kprobe_blacklist(unsigned long *start,
return ret ? : arch_populate_kprobe_blacklist();
}

+#ifdef CONFIG_MODULES
+/* Remove all symbols in given area from kprobe blacklist */
+static void kprobe_remove_area_blacklist(unsigned long start, unsigned long end)
+{
+ struct kprobe_blacklist_entry *ent, *n;
+
+ list_for_each_entry_safe(ent, n, &kprobe_blacklist, list) {
+ if (ent->start_addr < start || ent->start_addr >= end)
+ continue;
+ list_del(&ent->list);
+ kfree(ent);
+ }
+}
+
+static void kprobe_remove_ksym_blacklist(unsigned long entry)
+{
+ kprobe_remove_area_blacklist(entry, entry + 1);
+}
+
static void add_module_kprobe_blacklist(struct module *mod)
{
unsigned long start, end;
@@ -2320,6 +2323,15 @@ static void remove_module_kprobe_blacklist(struct module *mod)
kprobe_remove_area_blacklist(start, end);
}
}
+#else
+static void add_module_kprobe_blacklist(struct module *mod)
+{
+}
+
+static void remove_module_kprobe_blacklist(struct module *mod)
+{
+}
+#endif /* CONFIG_MODULES */

/* Module notifier call back, checking kprobes on the module */
static int kprobes_module_callback(struct notifier_block *nb,
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 710ec6a6aa8f..881c998d0162 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -103,8 +103,9 @@ static nokprobe_inline bool trace_kprobe_has_gone(struct trace_kprobe *tk)
return !!(kprobe_gone(&tk->rp.kp));
}

+#ifdef CONFIG_MODULES
static nokprobe_inline bool trace_kprobe_within_module(struct trace_kprobe *tk,
- struct module *mod)
+ struct module *mod)
{
int len = strlen(mod->name);
const char *name = trace_kprobe_symbol(tk);
@@ -129,6 +130,17 @@ static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)

return ret;
}
+#else
+static nokprobe_inline bool trace_kprobe_within_module(struct trace_kprobe *tk,
+ struct module *mod)
+{
+ return false;
+}
+static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
+{
+ return false;
+}
+#endif

static bool trace_kprobe_is_busy(struct dyn_event *ev)
{
@@ -688,7 +700,7 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
if (ret)
pr_warn("Failed to re-register probe %s on %s: %d\n",
trace_probe_name(&tk->tp),
- mod->name, ret);
+ module_name(mod), ret);
}
}
mutex_unlock(&event_mutex);
--
2.25.1

2020-07-17 03:08:44

by Jarkko Sakkinen

Subject: [PATCH v4 5/7] arch/x86: kprobes: Use text_alloc() in alloc_insn_page()

Use text_alloc() as part of the arch specific implementation for
alloc_insn_page().

Cc: Andi Kleen <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Signed-off-by: Jarkko Sakkinen <[email protected]>Im
---
arch/x86/kernel/kprobes/core.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index ada39ddbc922..0f20a3e52a06 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -423,7 +423,7 @@ void *alloc_insn_page(void)
{
void *page;

- page = module_alloc(PAGE_SIZE);
+ page = text_alloc(PAGE_SIZE);
if (!page)
return NULL;

@@ -443,12 +443,6 @@ void *alloc_insn_page(void)
return page;
}

-/* Recover page to RW mode before releasing it */
-void free_insn_page(void *page)
-{
- module_memfree(page);
-}
-
static int arch_copy_kprobe(struct kprobe *p)
{
struct insn insn;
--
2.25.1

2020-07-17 05:34:54

by kernel test robot

Subject: Re: [PATCH v4 3/7] vmalloc: Add text_alloc() and text_free()

Hi Jarkko,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.8-rc5 next-20200716]
[cannot apply to tip/x86/core tip/perf/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Jarkko-Sakkinen/arch-x86-kprobes-Remove-MODULES-dependency/20200717-110947
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 07a56bb875afbe39dabbf6ba7b83783d166863db
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

WARNING: unmet direct dependencies detected for FRAME_POINTER
Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS
Selected by
- FAULT_INJECTION_STACKTRACE_FILTER && FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT && !X86_64 && !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86
scripts/Makefile.build:65: 'arch/ia64/kernel/palinfo.ko' 'arch/ia64/kernel/mca_recovery.ko' 'arch/ia64/kernel/err_inject.ko' will not be built even though obj-m is specified.
scripts/Makefile.build:66: You cannot use subdir-y/m to visit a module Makefile. Use obj-y/m instead.
In file included from include/asm-generic/percpu.h:7,
from arch/ia64/include/asm/percpu.h:46,
from arch/ia64/include/asm/processor.h:76,
from arch/ia64/include/asm/thread_info.h:12,
from include/linux/thread_info.h:38,
from include/asm-generic/preempt.h:5,
from ./arch/ia64/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/sched/signal.h:5,
from arch/ia64/kernel/asm-offsets.c:10:
include/linux/topology.h: In function 'cpu_smt_mask':
>> arch/ia64/include/asm/topology.h:45:50: error: 'cpu_sibling_map' undeclared (first use in this function)
45 | #define topology_sibling_cpumask(cpu) (&per_cpu(cpu_sibling_map, cpu))
| ^~~~~~~~~~~~~~~
include/linux/percpu-defs.h:219:47: note: in definition of macro '__verify_pcpu_ptr'
219 | const void __percpu = (typeof((ptr) + 0))NULL; | ^~~
include/linux/percpu-defs.h:269:29: note: in expansion of macro 'per_cpu_ptr'
269 | #define per_cpu(var, cpu) cpu))
| ^~~~~~~~~~~
arch/ia64/include/asm/topology.h:45:42: note: in expansion of macro 'per_cpu'
45 | #define topology_sibling_cpumask(cpu) (&per_cpu(cpu_sibling_map, cpu))
| ^~~~~~~
include/linux/topology.h:204:9: note: in expansion of macro 'topology_sibling_cpumask'
204 | return topology_sibling_cpumask(cpu);
| ^~~~~~~~~~~~~~~~~~~~~~~~
arch/ia64/include/asm/topology.h:45:50: note: each undeclared identifier is reported only once for each function it appears in
45 | #define topology_sibling_cpumask(cpu) (&per_cpu(cpu_sibling_map, cpu))
| ^~~~~~~~~~~~~~~
include/linux/percpu-defs.h:219:47: note: in definition of macro '__verify_pcpu_ptr'
219 | const void __percpu = (typeof((ptr) + 0))NULL; | ^~~
include/linux/percpu-defs.h:269:29: note: in expansion of macro 'per_cpu_ptr'
269 | #define per_cpu(var, cpu) cpu))
| ^~~~~~~~~~~
arch/ia64/include/asm/topology.h:45:42: note: in expansion of macro 'per_cpu'
45 | #define topology_sibling_cpumask(cpu) (&per_cpu(cpu_sibling_map, cpu))
| ^~~~~~~
include/linux/topology.h:204:9: note: in expansion of macro 'topology_sibling_cpumask'
204 | return topology_sibling_cpumask(cpu);
| ^~~~~~~~~~~~~~~~~~~~~~~~
arch/ia64/kernel/asm-offsets.c: At top level:
arch/ia64/kernel/asm-offsets.c:23:6: warning: no previous prototype for 'foo'
23 | void foo(void)
| ^~~
Makefile arch block certs crypto drivers fs include init ipc kernel lib mm net scripts security sound source usr virt [scripts/Makefile.build:114: arch/ia64/kernel/asm-offsets.s] Error 1
Target '__build' not remade because of errors.
Makefile arch block certs crypto drivers fs include init ipc kernel lib mm net scripts security sound source usr virt [Makefile:1175: prepare0] Error 2
Target 'prepare' not remade because of errors.
make: Makefile arch block certs crypto drivers fs include init ipc kernel lib mm net scripts security sound source usr virt [Makefile:185: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.

vim +/cpu_sibling_map +45 arch/ia64/include/asm/topology.h

^1da177e4c3f41 include/asm-ia64/topology.h Linus Torvalds 2005-04-16 40
69dcc99199fe29 include/asm-ia64/topology.h Zhang, Yanmin 2006-02-03 41 #ifdef CONFIG_SMP
69dcc99199fe29 include/asm-ia64/topology.h Zhang, Yanmin 2006-02-03 42 #define topology_physical_package_id(cpu) (cpu_data(cpu)->socket_id)
69dcc99199fe29 include/asm-ia64/topology.h Zhang, Yanmin 2006-02-03 43 #define topology_core_id(cpu) (cpu_data(cpu)->core_id)
333af15341b2f6 arch/ia64/include/asm/topology.h Rusty Russell 2009-01-01 44 #define topology_core_cpumask(cpu) (&cpu_core_map[cpu])
06931e62246844 arch/ia64/include/asm/topology.h Bartosz Golaszewski 2015-05-26 @45 #define topology_sibling_cpumask(cpu) (&per_cpu(cpu_sibling_map, cpu))
69dcc99199fe29 include/asm-ia64/topology.h Zhang, Yanmin 2006-02-03 46 #endif
69dcc99199fe29 include/asm-ia64/topology.h Zhang, Yanmin 2006-02-03 47

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Attachments:

2020-07-17 05:37:07

by kernel test robot

Subject: Re: [PATCH v4 3/7] vmalloc: Add text_alloc() and text_free()

Hi Jarkko,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on jeyu/modules-next v5.8-rc5 next-20200716]
[cannot apply to tip/x86/core tip/perf/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Jarkko-Sakkinen/arch-x86-kprobes-Remove-MODULES-dependency/20200717-110947
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 07a56bb875afbe39dabbf6ba7b83783d166863db
config: sparc-randconfig-s031-20200717 (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.2-49-g707c5017-dirty
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=sparc

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

In file included from arch/sparc/include/asm/page.h:10,
from arch/sparc/include/asm/string_32.h:13,
from arch/sparc/include/asm/string.h:7,
from include/linux/string.h:20,
from include/linux/bitmap.h:9,
from include/linux/cpumask.h:12,
from arch/sparc/include/asm/smp_32.h:15,
from arch/sparc/include/asm/smp.h:7,
from arch/sparc/include/asm/switch_to_32.h:5,
from arch/sparc/include/asm/switch_to.h:7,
from arch/sparc/include/asm/ptrace.h:120,
from arch/sparc/include/asm/thread_info_32.h:19,
from arch/sparc/include/asm/thread_info.h:7,
from include/linux/thread_info.h:38,
from include/asm-generic/preempt.h:5,
from ./arch/sparc/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:16,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
>> include/linux/mmzone.h:1317:29: error: expected identifier or '(' before 'unsigned'
1317 | static inline int pfn_valid(unsigned long pfn)
| ^~~~~~~~
arch/sparc/include/asm/page_32.h:133:28: note: in definition of macro 'pfn_valid'
133 | #define pfn_valid(pfn) (((pfn) >= (pfn_base)) && (((pfn)-(pfn_base)) < max_mapnr))
| ^~~
>> arch/sparc/include/asm/page_32.h:133:33: error: expected ')' before '>=' token
133 | #define pfn_valid(pfn) (((pfn) >= (pfn_base)) && (((pfn)-(pfn_base)) < max_mapnr))
| ^~
include/linux/mmzone.h:1317:19: note: in expansion of macro 'pfn_valid'
1317 | static inline int pfn_valid(unsigned long pfn)
| ^~~~~~~~~
arch/sparc/include/asm/page_32.h:133:48: error: expected ')' before '&&' token
133 | #define pfn_valid(pfn) (((pfn) >= (pfn_base)) && (((pfn)-(pfn_base)) < max_mapnr))
| ^~
include/linux/mmzone.h:1317:19: note: in expansion of macro 'pfn_valid'
1317 | static inline int pfn_valid(unsigned long pfn)
| ^~~~~~~~~
In file included from include/linux/page-flags-layout.h:28,
from include/linux/mmzone.h:19,
from include/linux/gfp.h:6,
from include/linux/umh.h:4,
from include/linux/kmod.h:9,
from include/linux/module.h:16,
from include/linux/moduleloader.h:6,
from include/linux/vmalloc.h:12,
from include/asm-generic/io.h:911,
from arch/sparc/include/asm/io_32.h:14,
from arch/sparc/include/asm/io.h:7,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:18,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
include/linux/mmzone.h: In function 'pfn_in_present_section':
>> arch/sparc/include/asm/sparsemem.h:11:33: error: 'MAX_PHYS_ADDRESS_BITS' undeclared (first use in this function); did you mean 'MAX_PHYSADDR_BITS'?
11 | #define MAX_PHYSMEM_BITS MAX_PHYS_ADDRESS_BITS
| ^~~~~~~~~~~~~~~~~~~~~
include/linux/page-flags-layout.h:31:25: note: in expansion of macro 'MAX_PHYSMEM_BITS'
31 | #define SECTIONS_SHIFT (MAX_PHYSMEM_BITS - SECTION_SIZE_BITS)
| ^~~~~~~~~~~~~~~~
include/linux/mmzone.h:1104:34: note: in expansion of macro 'SECTIONS_SHIFT'
1104 | #define NR_MEM_SECTIONS (1UL << SECTIONS_SHIFT)
| ^~~~~~~~~~~~~~
include/linux/mmzone.h:1336:32: note: in expansion of macro 'NR_MEM_SECTIONS'
1336 | if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
| ^~~~~~~~~~~~~~~
arch/sparc/include/asm/sparsemem.h:11:33: note: each undeclared identifier is reported only once for each function it appears in
11 | #define MAX_PHYSMEM_BITS MAX_PHYS_ADDRESS_BITS
| ^~~~~~~~~~~~~~~~~~~~~
include/linux/page-flags-layout.h:31:25: note: in expansion of macro 'MAX_PHYSMEM_BITS'
31 | #define SECTIONS_SHIFT (MAX_PHYSMEM_BITS - SECTION_SIZE_BITS)
| ^~~~~~~~~~~~~~~~
include/linux/mmzone.h:1104:34: note: in expansion of macro 'SECTIONS_SHIFT'
1104 | #define NR_MEM_SECTIONS (1UL << SECTIONS_SHIFT)
| ^~~~~~~~~~~~~~
include/linux/mmzone.h:1336:32: note: in expansion of macro 'NR_MEM_SECTIONS'
1336 | if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
| ^~~~~~~~~~~~~~~
In file included from include/linux/local_lock.h:5,
from include/linux/radix-tree.h:19,
from include/linux/idr.h:15,
from include/linux/kernfs.h:13,
from include/linux/sysfs.h:16,
from include/linux/kobject.h:20,
from include/linux/of.h:17,
from arch/sparc/include/asm/openprom.h:15,
from arch/sparc/include/asm/oplib_32.h:12,
from arch/sparc/include/asm/oplib.h:7,
from arch/sparc/include/asm/pgtable_32.h:32,
from arch/sparc/include/asm/pgtable.h:7,
from arch/sparc/include/asm/viking.h:13,
from arch/sparc/include/asm/mbus.h:12,
from arch/sparc/include/asm/elf_32.h:94,
from arch/sparc/include/asm/elf.h:7,
from include/linux/elf.h:6,
from include/linux/module.h:18,
from include/linux/moduleloader.h:6,
from include/linux/vmalloc.h:12,
from include/asm-generic/io.h:911,
from arch/sparc/include/asm/io_32.h:14,
from arch/sparc/include/asm/io.h:7,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:18,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
include/linux/local_lock_internal.h: In function 'local_lock_acquire':
>> include/linux/local_lock_internal.h:41:13: error: 'current' undeclared (first use in this function)
41 | l->owner = current;
| ^~~~~~~
In file included from include/linux/kernel.h:11,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:15,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
include/linux/local_lock_internal.h: In function 'local_lock_release':
include/linux/local_lock_internal.h:46:34: error: 'current' undeclared (first use in this function)
46 | DEBUG_LOCKS_WARN_ON(l->owner != current);
| ^~~~~~~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
include/linux/local_lock_internal.h:46:2: note: in expansion of macro 'DEBUG_LOCKS_WARN_ON'
46 | DEBUG_LOCKS_WARN_ON(l->owner != current);
| ^~~~~~~~~~~~~~~~~~~
In file included from include/linux/sched/signal.h:7,
from include/linux/rcuwait.h:6,
from include/linux/percpu-rwsem.h:7,
from include/linux/fs.h:33,
from include/linux/proc_fs.h:10,
from arch/sparc/include/asm/prom.h:18,
from include/linux/of.h:250,
from arch/sparc/include/asm/openprom.h:15,
from arch/sparc/include/asm/oplib_32.h:12,
from arch/sparc/include/asm/oplib.h:7,
from arch/sparc/include/asm/pgtable_32.h:32,
from arch/sparc/include/asm/pgtable.h:7,
from arch/sparc/include/asm/viking.h:13,
from arch/sparc/include/asm/mbus.h:12,
from arch/sparc/include/asm/elf_32.h:94,
from arch/sparc/include/asm/elf.h:7,
from include/linux/elf.h:6,
from include/linux/module.h:18,
from include/linux/moduleloader.h:6,
from include/linux/vmalloc.h:12,
from include/asm-generic/io.h:911,
from arch/sparc/include/asm/io_32.h:14,
from arch/sparc/include/asm/io.h:7,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:18,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
include/linux/sched.h: In function 'is_percpu_thread':
>> include/linux/sched.h:1552:10: error: 'current' undeclared (first use in this function)
1552 | return (current->flags & PF_NO_SETAFFINITY) &&
| ^~~~~~~
include/linux/sched.h: In function 'current_restore_flags':
include/linux/sched.h:1613:2: error: 'current' undeclared (first use in this function)
1613 | current->flags &= ~flags;
| ^~~~~~~
In file included from arch/sparc/include/asm/uaccess.h:7,
from include/linux/uaccess.h:11,
from include/linux/sched/task.h:11,
from include/linux/sched/signal.h:9,
from include/linux/rcuwait.h:6,
from include/linux/percpu-rwsem.h:7,
from include/linux/fs.h:33,
from include/linux/proc_fs.h:10,
from arch/sparc/include/asm/prom.h:18,
from include/linux/of.h:250,
from arch/sparc/include/asm/openprom.h:15,
from arch/sparc/include/asm/oplib_32.h:12,
from arch/sparc/include/asm/oplib.h:7,
from arch/sparc/include/asm/pgtable_32.h:32,
from arch/sparc/include/asm/pgtable.h:7,
from arch/sparc/include/asm/viking.h:13,
from arch/sparc/include/asm/mbus.h:12,
from arch/sparc/include/asm/elf_32.h:94,
from arch/sparc/include/asm/elf.h:7,
from include/linux/elf.h:6,
from include/linux/module.h:18,
from include/linux/moduleloader.h:6,
from include/linux/vmalloc.h:12,
from include/asm-generic/io.h:911,
from arch/sparc/include/asm/io_32.h:14,
from arch/sparc/include/asm/io.h:7,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:18,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
arch/sparc/include/asm/uaccess_32.h: In function 'clear_user':
>> arch/sparc/include/asm/uaccess_32.h:28:19: error: 'current' undeclared (first use in this function)
28 | #define get_fs() (current->thread.current_ds)
| ^~~~~~~
arch/sparc/include/asm/uaccess_32.h:38:49: note: in definition of macro '__user_ok'
38 | #define __user_ok(addr, size) ({ (void)(size); (addr) < STACK_TOP; })
| ^~~~
arch/sparc/include/asm/uaccess_32.h:40:53: note: in expansion of macro 'get_fs'
40 | #define __access_ok(addr, size) (__user_ok((addr) & get_fs().seg, (size)))
| ^~~~~~
arch/sparc/include/asm/uaccess_32.h:273:11: note: in expansion of macro '__access_ok'
273 | if (n && __access_ok((unsigned long) addr, n))
| ^~~~~~~~~~~
In file included from include/linux/kernel.h:11,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:15,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
include/linux/uaccess.h: In function '_copy_from_user':
>> arch/sparc/include/asm/uaccess_32.h:28:19: error: 'current' undeclared (first use in this function)
28 | #define get_fs() (current->thread.current_ds)
| ^~~~~~~
include/linux/compiler.h:77:40: note: in definition of macro 'likely'
77 | # define likely(x) __builtin_expect(!!(x), 1)
| ^
arch/sparc/include/asm/uaccess_32.h:40:34: note: in expansion of macro '__user_ok'
40 | #define __access_ok(addr, size) (__user_ok((addr) & get_fs().seg, (size)))
| ^~~~~~~~~
arch/sparc/include/asm/uaccess_32.h:40:53: note: in expansion of macro 'get_fs'
40 | #define __access_ok(addr, size) (__user_ok((addr) & get_fs().seg, (size)))
| ^~~~~~
arch/sparc/include/asm/uaccess_32.h:41:31: note: in expansion of macro '__access_ok'
41 | #define access_ok(addr, size) __access_ok((unsigned long)(addr), size)
| ^~~~~~~~~~~
include/linux/uaccess.h:111:13: note: in expansion of macro 'access_ok'
111 | if (likely(access_ok(from, n))) {
| ^~~~~~~~~
In file included from arch/sparc/include/asm/uaccess.h:7,
from include/linux/uaccess.h:11,
from include/linux/sched/task.h:11,
from include/linux/sched/signal.h:9,
from include/linux/rcuwait.h:6,
from include/linux/percpu-rwsem.h:7,
from include/linux/fs.h:33,
from include/linux/proc_fs.h:10,
from arch/sparc/include/asm/prom.h:18,
from include/linux/of.h:250,
from arch/sparc/include/asm/openprom.h:15,
from arch/sparc/include/asm/oplib_32.h:12,
from arch/sparc/include/asm/oplib.h:7,
from arch/sparc/include/asm/pgtable_32.h:32,
from arch/sparc/include/asm/pgtable.h:7,
from arch/sparc/include/asm/viking.h:13,
from arch/sparc/include/asm/mbus.h:12,
from arch/sparc/include/asm/elf_32.h:94,
from arch/sparc/include/asm/elf.h:7,
from include/linux/elf.h:6,
from include/linux/module.h:18,
from include/linux/moduleloader.h:6,
from include/linux/vmalloc.h:12,
from include/asm-generic/io.h:911,
from arch/sparc/include/asm/io_32.h:14,
from arch/sparc/include/asm/io.h:7,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:18,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
include/linux/uaccess.h: In function '_copy_to_user':
>> arch/sparc/include/asm/uaccess_32.h:28:19: error: 'current' undeclared (first use in this function)
28 | #define get_fs() (current->thread.current_ds)
| ^~~~~~~
arch/sparc/include/asm/uaccess_32.h:38:49: note: in definition of macro '__user_ok'
38 | #define __user_ok(addr, size) ({ (void)(size); (addr) < STACK_TOP; })
| ^~~~
arch/sparc/include/asm/uaccess_32.h:40:53: note: in expansion of macro 'get_fs'
40 | #define __access_ok(addr, size) (__user_ok((addr) & get_fs().seg, (size)))
| ^~~~~~
arch/sparc/include/asm/uaccess_32.h:41:31: note: in expansion of macro '__access_ok'
41 | #define access_ok(addr, size) __access_ok((unsigned long)(addr), size)
| ^~~~~~~~~~~
include/linux/uaccess.h:129:6: note: in expansion of macro 'access_ok'
129 | if (access_ok(to, n)) {
| ^~~~~~~~~
include/linux/uaccess.h: In function 'copy_in_user':
>> arch/sparc/include/asm/uaccess_32.h:28:19: error: 'current' undeclared (first use in this function)
28 | #define get_fs() (current->thread.current_ds)
| ^~~~~~~
arch/sparc/include/asm/uaccess_32.h:38:49: note: in definition of macro '__user_ok'
38 | #define __user_ok(addr, size) ({ (void)(size); (addr) < STACK_TOP; })
| ^~~~
arch/sparc/include/asm/uaccess_32.h:40:53: note: in expansion of macro 'get_fs'
40 | #define __access_ok(addr, size) (__user_ok((addr) & get_fs().seg, (size)))
| ^~~~~~
arch/sparc/include/asm/uaccess_32.h:41:31: note: in expansion of macro '__access_ok'
41 | #define access_ok(addr, size) __access_ok((unsigned long)(addr), size)
| ^~~~~~~~~~~
include/linux/uaccess.h:160:6: note: in expansion of macro 'access_ok'
160 | if (access_ok(to, n) && access_ok(from, n))
| ^~~~~~~~~
In file included from include/linux/sched/task.h:11,
from include/linux/sched/signal.h:9,
from include/linux/rcuwait.h:6,
from include/linux/percpu-rwsem.h:7,
from include/linux/fs.h:33,
from include/linux/proc_fs.h:10,
from arch/sparc/include/asm/prom.h:18,
from include/linux/of.h:250,
from arch/sparc/include/asm/openprom.h:15,
from arch/sparc/include/asm/oplib_32.h:12,
from arch/sparc/include/asm/oplib.h:7,
from arch/sparc/include/asm/pgtable_32.h:32,
from arch/sparc/include/asm/pgtable.h:7,
from arch/sparc/include/asm/viking.h:13,
from arch/sparc/include/asm/mbus.h:12,
from arch/sparc/include/asm/elf_32.h:94,
from arch/sparc/include/asm/elf.h:7,
from include/linux/elf.h:6,
from include/linux/module.h:18,
from include/linux/moduleloader.h:6,
from include/linux/vmalloc.h:12,
from include/asm-generic/io.h:911,
from arch/sparc/include/asm/io_32.h:14,
from arch/sparc/include/asm/io.h:7,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:18,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
>> include/linux/uaccess.h:161:7: error: implicit declaration of function 'raw_copy_in_user'; did you mean 'raw_copy_to_user'? [-Werror=implicit-function-declaration]
161 | n = raw_copy_in_user(to, from, n);
| ^~~~~~~~~~~~~~~~
| raw_copy_to_user
include/linux/uaccess.h: In function 'pagefault_disabled_inc':
>> include/linux/uaccess.h:168:2: error: 'current' undeclared (first use in this function)
168 | current->pagefault_disabled++;
| ^~~~~~~
include/linux/uaccess.h: In function 'pagefault_disabled_dec':
include/linux/uaccess.h:173:2: error: 'current' undeclared (first use in this function)
173 | current->pagefault_disabled--;
| ^~~~~~~
include/linux/uaccess.h: In function 'pagefault_disabled':
include/linux/uaccess.h:208:9: error: 'current' undeclared (first use in this function)
208 | return current->pagefault_disabled != 0;
| ^~~~~~~
In file included from include/linux/kernel.h:15,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:15,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
include/linux/ratelimit.h: In function 'ratelimit_state_exit':
>> include/linux/ratelimit.h:63:4: error: 'current' undeclared (first use in this function)
63 | current->comm, rs->missed);
| ^~~~~~~
include/linux/printk.h:348:37: note: in definition of macro 'pr_warn'
348 | printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~
In file included from include/linux/rcuwait.h:6,
from include/linux/percpu-rwsem.h:7,
from include/linux/fs.h:33,
from include/linux/proc_fs.h:10,
from arch/sparc/include/asm/prom.h:18,
from include/linux/of.h:250,
from arch/sparc/include/asm/openprom.h:15,
from arch/sparc/include/asm/oplib_32.h:12,
from arch/sparc/include/asm/oplib.h:7,
from arch/sparc/include/asm/pgtable_32.h:32,
from arch/sparc/include/asm/pgtable.h:7,
from arch/sparc/include/asm/viking.h:13,
from arch/sparc/include/asm/mbus.h:12,
from arch/sparc/include/asm/elf_32.h:94,
from arch/sparc/include/asm/elf.h:7,
from include/linux/elf.h:6,
from include/linux/module.h:18,
from include/linux/moduleloader.h:6,
from include/linux/vmalloc.h:12,
from include/asm-generic/io.h:911,
from arch/sparc/include/asm/io_32.h:14,
from arch/sparc/include/asm/io.h:7,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:18,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
include/linux/sched/signal.h: In function 'kernel_dequeue_signal':
>> include/linux/sched/signal.h:280:29: error: 'current' undeclared (first use in this function)
280 | struct task_struct *task = current;
| ^~~~~~~
include/linux/sched/signal.h: In function 'kernel_signal_stop':
include/linux/sched/signal.h:293:17: error: 'current' undeclared (first use in this function)
293 | spin_lock_irq(&current->sighand->siglock);
| ^~~~~~~
include/linux/sched/signal.h: In function 'restart_syscall':
include/linux/sched/signal.h:352:22: error: 'current' undeclared (first use in this function)
352 | set_tsk_thread_flag(current, TIF_SIGPENDING);
| ^~~~~~~
In file included from include/linux/kernel.h:11,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:15,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
include/linux/sched/signal.h: In function 'fault_signal_pending':
include/linux/sched/signal.h:391:26: error: 'current' undeclared (first use in this function)
391 | (fatal_signal_pending(current) ||
| ^~~~~~~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
In file included from include/linux/rcuwait.h:6,
from include/linux/percpu-rwsem.h:7,
from include/linux/fs.h:33,
from include/linux/proc_fs.h:10,
from arch/sparc/include/asm/prom.h:18,
from include/linux/of.h:250,
from arch/sparc/include/asm/openprom.h:15,
from arch/sparc/include/asm/oplib_32.h:12,
from arch/sparc/include/asm/oplib.h:7,
from arch/sparc/include/asm/pgtable_32.h:32,
from arch/sparc/include/asm/pgtable.h:7,
from arch/sparc/include/asm/viking.h:13,
from arch/sparc/include/asm/mbus.h:12,
from arch/sparc/include/asm/elf_32.h:94,
from arch/sparc/include/asm/elf.h:7,
from include/linux/elf.h:6,
from include/linux/module.h:18,
from include/linux/moduleloader.h:6,
from include/linux/vmalloc.h:12,
from include/asm-generic/io.h:911,
from arch/sparc/include/asm/io_32.h:14,
from arch/sparc/include/asm/io.h:7,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:18,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
include/linux/sched/signal.h: In function 'restore_saved_sigmask':
include/linux/sched/signal.h:497:26: error: 'current' undeclared (first use in this function)
497 | __set_current_blocked(&current->saved_sigmask);
| ^~~~~~~
include/linux/sched/signal.h: In function 'sigmask_to_save':
include/linux/sched/signal.h:512:19: error: 'current' undeclared (first use in this function)
512 | sigset_t *res = &current->blocked;
| ^~~~~~~
include/linux/sched/signal.h: In function 'on_sig_stack':
include/linux/sched/signal.h:541:6: error: 'current' undeclared (first use in this function)
541 | if (current->sas_ss_flags & SS_AUTODISARM)
| ^~~~~~~
include/linux/sched/signal.h: In function 'sas_ss_flags':
include/linux/sched/signal.h:555:7: error: 'current' undeclared (first use in this function)
555 | if (!current->sas_ss_size)
| ^~~~~~~
include/linux/sched/signal.h: In function 'sigsp':
include/linux/sched/signal.h:574:10: error: 'current' undeclared (first use in this function)
574 | return current->sas_ss_sp + current->sas_ss_size;
| ^~~~~~~
include/linux/sched/signal.h: In function 'rlimit':
include/linux/sched/signal.h:710:21: error: 'current' undeclared (first use in this function)
710 | return task_rlimit(current, limit);
| ^~~~~~~
include/linux/sched/signal.h: In function 'rlimit_max':
include/linux/sched/signal.h:715:25: error: 'current' undeclared (first use in this function)
715 | return task_rlimit_max(current, limit);
| ^~~~~~~
In file included from include/linux/rbtree.h:22,
from include/linux/vmalloc.h:10,
from include/asm-generic/io.h:911,
from arch/sparc/include/asm/io_32.h:14,
from arch/sparc/include/asm/io.h:7,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:18,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
include/linux/rcuwait.h: In function 'prepare_to_rcuwait':
>> include/linux/rcuwait.h:47:30: error: 'current' undeclared (first use in this function)
47 | rcu_assign_pointer(w->task, current);
| ^~~~~~~
include/linux/rcupdate.h:409:36: note: in definition of macro 'rcu_assign_pointer'
409 | uintptr_t _r_a_p__v = (uintptr_t)(v); \
| ^
In file included from include/linux/sched/signal.h:7,
from include/linux/rcuwait.h:6,
from include/linux/percpu-rwsem.h:7,
from include/linux/fs.h:33,
from include/linux/proc_fs.h:10,
from arch/sparc/include/asm/prom.h:18,
from include/linux/of.h:250,
from arch/sparc/include/asm/openprom.h:15,
from arch/sparc/include/asm/oplib_32.h:12,
from arch/sparc/include/asm/oplib.h:7,
from arch/sparc/include/asm/pgtable_32.h:32,
from arch/sparc/include/asm/pgtable.h:7,
from arch/sparc/include/asm/viking.h:13,
from arch/sparc/include/asm/mbus.h:12,
from arch/sparc/include/asm/elf_32.h:94,
from arch/sparc/include/asm/elf.h:7,
from include/linux/elf.h:6,
from include/linux/module.h:18,
from include/linux/moduleloader.h:6,
from include/linux/vmalloc.h:12,
from include/asm-generic/io.h:911,
from arch/sparc/include/asm/io_32.h:14,
from arch/sparc/include/asm/io.h:7,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:18,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
include/linux/rcuwait.h: In function 'finish_rcuwait':
include/linux/sched.h:133:3: error: 'current' undeclared (first use in this function)
133 | current->task_state_change = _THIS_IP_; \
| ^~~~~~~
include/linux/rcuwait.h:53:2: note: in expansion of macro '__set_current_state'
53 | __set_current_state(TASK_RUNNING);
| ^~~~~~~~~~~~~~~~~~~
In file included from include/linux/fs.h:38,
from include/linux/proc_fs.h:10,
from arch/sparc/include/asm/prom.h:18,
from include/linux/of.h:250,
from arch/sparc/include/asm/openprom.h:15,
from arch/sparc/include/asm/oplib_32.h:12,
from arch/sparc/include/asm/oplib.h:7,
from arch/sparc/include/asm/pgtable_32.h:32,
from arch/sparc/include/asm/pgtable.h:7,
from arch/sparc/include/asm/viking.h:13,
from arch/sparc/include/asm/mbus.h:12,
from arch/sparc/include/asm/elf_32.h:94,
from arch/sparc/include/asm/elf.h:7,
from include/linux/elf.h:6,
from include/linux/module.h:18,
from include/linux/moduleloader.h:6,
from include/linux/vmalloc.h:12,
from include/asm-generic/io.h:911,
from arch/sparc/include/asm/io_32.h:14,
from arch/sparc/include/asm/io.h:7,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:18,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
include/linux/ioprio.h: In function 'get_current_ioprio':
>> include/linux/ioprio.h:79:27: error: 'current' undeclared (first use in this function)
79 | struct io_context *ioc = current->io_context;
| ^~~~~~~
In file included from arch/sparc/include/asm/page_32.h:136,
from arch/sparc/include/asm/page.h:10,
from arch/sparc/include/asm/string_32.h:13,
from arch/sparc/include/asm/string.h:7,
from include/linux/string.h:20,
from include/linux/bitmap.h:9,
from include/linux/cpumask.h:12,
from arch/sparc/include/asm/smp_32.h:15,
from arch/sparc/include/asm/smp.h:7,
from arch/sparc/include/asm/switch_to_32.h:5,
from arch/sparc/include/asm/switch_to.h:7,
from arch/sparc/include/asm/ptrace.h:120,
from arch/sparc/include/asm/thread_info_32.h:19,
from arch/sparc/include/asm/thread_info.h:7,
from include/linux/thread_info.h:38,
from include/asm-generic/preempt.h:5,
from ./arch/sparc/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:16,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
arch/sparc/include/asm/pgtable_32.h: In function 'pmd_page':
include/asm-generic/memory_model.h:54:29: error: 'vmemmap' undeclared (first use in this function); did you mean 'mem_map'?
54 | #define __pfn_to_page(pfn) (vmemmap + (pfn))
| ^~~~~~~
include/asm-generic/memory_model.h:82:21: note: in expansion of macro '__pfn_to_page'
82 | #define pfn_to_page __pfn_to_page
| ^~~~~~~~~~~~~
arch/sparc/include/asm/pgtable_32.h:135:9: note: in expansion of macro 'pfn_to_page'
135 | return pfn_to_page((pmd_val(pmd) & SRMMU_PTD_PMASK) >> (PAGE_SHIFT-4));
| ^~~~~~~~~~~
In file included from arch/sparc/include/asm/page.h:10,
from arch/sparc/include/asm/string_32.h:13,
from arch/sparc/include/asm/string.h:7,
from include/linux/string.h:20,
from include/linux/bitmap.h:9,
from include/linux/cpumask.h:12,
from arch/sparc/include/asm/smp_32.h:15,
from arch/sparc/include/asm/smp.h:7,
from arch/sparc/include/asm/switch_to_32.h:5,
from arch/sparc/include/asm/switch_to.h:7,
from arch/sparc/include/asm/ptrace.h:120,
from arch/sparc/include/asm/thread_info_32.h:19,
from arch/sparc/include/asm/thread_info.h:7,
from include/linux/thread_info.h:38,
from include/asm-generic/preempt.h:5,
from ./arch/sparc/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from arch/sparc/vdso/vdso32/../vclock_gettime.c:16,
from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
arch/sparc/include/asm/pgtable_32.h: In function 'mk_pte':
include/asm-generic/memory_model.h:55:54: error: 'vmemmap' undeclared (first use in this function); did you mean 'mem_map'?
55 | #define __page_to_pfn(page) (unsigned long)((page) - vmemmap)
| ^~~~~~~
arch/sparc/include/asm/page_32.h:99:19: note: in definition of macro '__pte'
99 | #define __pte(x) (x)
| ^
include/asm-generic/memory_model.h:81:21: note: in expansion of macro '__page_to_pfn'
81 | #define page_to_pfn __page_to_pfn
| ^~~~~~~~~~~~~
arch/sparc/include/asm/pgtable_32.h:297:16: note: in expansion of macro 'page_to_pfn'
297 | return __pte((page_to_pfn(page) << (PAGE_SHIFT-4)) | pgprot_val(pgprot));
| ^~~~~~~~~~~
In file included from arch/sparc/vdso/vdso32/vclock_gettime.c:22:
arch/sparc/vdso/vdso32/../vclock_gettime.c: At top level:
arch/sparc/vdso/vdso32/../vclock_gettime.c:254:1: warning: no previous prototype for '__vdso_clock_gettime' [-Wmissing-prototypes]
254 | __vdso_clock_gettime(clockid_t clock, struct __kernel_old_timespec *ts)
| ^~~~~~~~~~~~~~~~~~~~
arch/sparc/vdso/vdso32/../vclock_gettime.c:282:1: warning: no previous prototype for '__vdso_clock_gettime_stick' [-Wmissing-prototypes]
282 | __vdso_clock_gettime_stick(clockid_t clock, struct __kernel_old_timespec *ts)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/sparc/vdso/vdso32/../vclock_gettime.c:307:1: warning: no previous prototype for '__vdso_gettimeofday' [-Wmissing-prototypes]
307 | __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz)
| ^~~~~~~~~~~~~~~~~~~
arch/sparc/vdso/vdso32/../vclock_gettime.c:343:1: warning: no previous prototype for '__vdso_gettimeofday_stick' [-Wmissing-prototypes]
343 | __vdso_gettimeofday_stick(struct __kernel_old_timeval *tv, struct timezone *tz)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors

vim +1317 include/linux/mmzone.h

f46edbd1b1516d Dan Williams 2019-07-18 1315
7b7bf499f79de3 Will Deacon 2011-05-19 1316 #ifndef CONFIG_HAVE_ARCH_PFN_VALID
d41dee369bff3b Andy Whitcroft 2005-06-23 @1317 static inline int pfn_valid(unsigned long pfn)
d41dee369bff3b Andy Whitcroft 2005-06-23 1318 {
f46edbd1b1516d Dan Williams 2019-07-18 1319 struct mem_section *ms;
f46edbd1b1516d Dan Williams 2019-07-18 1320
d41dee369bff3b Andy Whitcroft 2005-06-23 1321 if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
d41dee369bff3b Andy Whitcroft 2005-06-23 1322 return 0;
f46edbd1b1516d Dan Williams 2019-07-18 1323 ms = __nr_to_section(pfn_to_section_nr(pfn));
f46edbd1b1516d Dan Williams 2019-07-18 1324 if (!valid_section(ms))
f46edbd1b1516d Dan Williams 2019-07-18 1325 return 0;
f46edbd1b1516d Dan Williams 2019-07-18 1326 /*
f46edbd1b1516d Dan Williams 2019-07-18 1327 * Traditionally early sections always returned pfn_valid() for
f46edbd1b1516d Dan Williams 2019-07-18 1328 * the entire section-sized span.
f46edbd1b1516d Dan Williams 2019-07-18 1329 */
f46edbd1b1516d Dan Williams 2019-07-18 1330 return early_section(ms) || pfn_section_valid(ms, pfn);
d41dee369bff3b Andy Whitcroft 2005-06-23 1331 }
7b7bf499f79de3 Will Deacon 2011-05-19 1332 #endif
d41dee369bff3b Andy Whitcroft 2005-06-23 1333

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Attachments:

2020-07-17 08:55:40

by Masami Hiramatsu

Subject: Re: [PATCH v4 3/7] vmalloc: Add text_alloc() and text_free()

On Fri, 17 Jul 2020 06:04:17 +0300
Jarkko Sakkinen <[email protected]> wrote:

> Introduce functions for allocating memory for dynamic trampolines, such
> as kprobes. An arch can promote the availability of these functions with
> CONFIG_ARCH_HAS_TEXT_ALLOC. Provide default/fallback implementation
> wrapping module_alloc() and module_memfree().

Doesn't it depend on CONFIG_MODULE?

Thank you,

>
> Cc: Andi Kleen <[email protected]>
> Cc: Masami Hiramatsu <[email protected]>
> Suggested-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Jarkko Sakkinen <[email protected]>
> ---
> include/linux/vmalloc.h | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 0221f852a7e1..e981436e30b6 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -9,6 +9,7 @@
> #include <asm/page.h> /* pgprot_t */
> #include <linux/rbtree.h>
> #include <linux/overflow.h>
> +#include <linux/moduleloader.h>
>
> #include <asm/vmalloc.h>
>
> @@ -249,4 +250,26 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
> int register_vmap_purge_notifier(struct notifier_block *nb);
> int unregister_vmap_purge_notifier(struct notifier_block *nb);
>
> +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> +/*
> + * Allocate memory to be used for dynamic trampoline code.
> + */
> +void *text_alloc(unsigned long size);
> +
> +/*
> + * Free memory returned from text_alloc().
> + */
> +void text_free(void *region);
> +#else
> +static inline void *text_alloc(unsigned long size)
> +{
> + return module_alloc(size);
> +}
> +
> +static inline void text_free(void *region)
> +{
> + module_memfree(region);
> +}
> +#endif
> +
> #endif /* _LINUX_VMALLOC_H */
> --
> 2.25.1
>


--
Masami Hiramatsu <[email protected]>

2020-07-18 16:30:46

by Mike Rapoport

Subject: Re: [PATCH v4 3/7] vmalloc: Add text_alloc() and text_free()

On Fri, Jul 17, 2020 at 06:04:17AM +0300, Jarkko Sakkinen wrote:
> Introduce functions for allocating memory for dynamic trampolines, such
> as kprobes. An arch can promote the availability of these functions with
> CONFIG_ARCH_HAS_TEXT_ALLOC. Provide default/fallback implementation
> wrapping module_alloc() and module_memfree().
>
> Cc: Andi Kleen <[email protected]>
> Cc: Masami Hiramatsu <[email protected]>
> Suggested-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Jarkko Sakkinen <[email protected]>
> ---
> include/linux/vmalloc.h | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 0221f852a7e1..e981436e30b6 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -9,6 +9,7 @@
> #include <asm/page.h> /* pgprot_t */
> #include <linux/rbtree.h>
> #include <linux/overflow.h>
> +#include <linux/moduleloader.h>
>
> #include <asm/vmalloc.h>
>
> @@ -249,4 +250,26 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
> int register_vmap_purge_notifier(struct notifier_block *nb);
> int unregister_vmap_purge_notifier(struct notifier_block *nb);
>
> +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> +/*
> + * Allocate memory to be used for dynamic trampoline code.
> + */
> +void *text_alloc(unsigned long size);
> +
> +/*
> + * Free memory returned from text_alloc().
> + */
> +void text_free(void *region);
> +#else
> +static inline void *text_alloc(unsigned long size)
> +{
> + return module_alloc(size);
> +}
> +
> +static inline void text_free(void *region)
> +{
> + module_memfree(region);
> +}

Using module_alloc() as the default implementation of generic
text_alloc() does not sound right to me.

I would suggest rename module_alloc() to text_alloc() on x86, as Peter
proposed and then add text_alloc_kprobes() that can be overridden by the
architectures. x86 could use text_alloc(), arm64 vmalloc() with options
of their choice and the fallback would remain module_alloc(). Something
like (untested) patch below:


From 928b6903e76ebf5790fc415f9eed390e400e5bc3 Mon Sep 17 00:00:00 2001
From: Mike Rapoport <[email protected]>
Date: Sat, 18 Jul 2020 19:13:02 +0300
Subject: [PATCH] kprobes: introduce text_alloc_kprobes() and
text_free_kprobes()

Signed-off-by: Mike Rapoport <[email protected]>
---
arch/Kconfig | 5 ++++-
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/probes/kprobes.c | 10 ++++++++--
arch/x86/Kconfig | 3 ++-
arch/x86/kernel/kprobes/core.c | 9 ++++-----
include/linux/kprobes.h | 17 +++++++++++++++--
kernel/kprobes.c | 8 ++++----
7 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 8cc35dc556c7..c0589b3b3225 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -61,7 +61,7 @@ config OPROFILE_NMI_TIMER

config KPROBES
bool "Kprobes"
- depends on MODULES
+ depends on MODULES || HAVE_KPROBES_TEXT_ALLOC
depends on HAVE_KPROBES
select KALLSYMS
help
@@ -186,6 +186,9 @@ config HAVE_IOREMAP_PROT
config HAVE_KPROBES
bool

+config HAVE_KPROBES_TEXT_ALLOC
+ bool
+
config HAVE_KRETPROBES
bool

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 66dc41fd49f2..abc0538b13ef 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -177,6 +177,7 @@ config ARM64
select HAVE_STACKPROTECTOR
select HAVE_SYSCALL_TRACEPOINTS
select HAVE_KPROBES
+ select HAVE_KPROBES_TEXT_ALLOC
select HAVE_KRETPROBES
select HAVE_GENERIC_VDSO
select IOMMU_DMA if IOMMU_SUPPORT
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 5290f17a4d80..fac8c6020040 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -118,13 +118,19 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
return 0;
}

-void *alloc_insn_page(void)
+void *text_alloc_kprobes(unsigned long size)
{
- return __vmalloc_node_range(PAGE_SIZE, 1, VMALLOC_START, VMALLOC_END,
+ return __vmalloc_node_range(size, 1, VMALLOC_START, VMALLOC_END,
GFP_KERNEL, PAGE_KERNEL_ROX, VM_FLUSH_RESET_PERMS,
NUMA_NO_NODE, __builtin_return_address(0));
}

+void text_free_kprobes(void *mem)
+{
+ lockdep_assert_irqs_enabled();
+ vfree(mem);
+}
+
/* arm kprobe: install breakpoint in text */
void __kprobes arch_arm_kprobe(struct kprobe *p)
{
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 883da0abf779..d08b92f91531 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -190,6 +190,7 @@ config X86
select HAVE_KERNEL_XZ
select HAVE_KPROBES
select HAVE_KPROBES_ON_FTRACE
+ select HAVE_KPROBES_TEXT_ALLOC
select HAVE_FUNCTION_ERROR_INJECTION
select HAVE_KRETPROBES
select HAVE_KVM
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index ada39ddbc922..a3e8f01c62d4 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -418,12 +418,11 @@ static int prepare_boost(kprobe_opcode_t *buf, struct kprobe *p,
return len;
}

-/* Make page to RO mode when allocate it */
-void *alloc_insn_page(void)
+void *text_alloc_kprobes(unsigned long size)
{
void *page;

- page = module_alloc(PAGE_SIZE);
+ page = text_alloc(size);
if (!page)
return NULL;

@@ -444,9 +443,9 @@ void *alloc_insn_page(void)
}

/* Recover page to RW mode before releasing it */
-void free_insn_page(void *page)
+void text_free_kprobes(void *page)
{
- module_memfree(page);
+ text_free(page);
}

static int arch_copy_kprobe(struct kprobe *p)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 6adf90f248d7..dbc9c71d4ec4 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -27,6 +27,7 @@
#include <linux/rcupdate.h>
#include <linux/mutex.h>
#include <linux/ftrace.h>
+#include <linux/moduleloader.h>
#include <asm/kprobes.h>

#ifdef CONFIG_KPROBES
@@ -374,8 +375,20 @@ int enable_kprobe(struct kprobe *kp);

void dump_kprobe(struct kprobe *kp);

-void *alloc_insn_page(void);
-void free_insn_page(void *page);
+#ifdef CONFIG_HAVE_KPROBES_TEXT_ALLOC
+void *text_alloc_kprobes(unsigned long size);
+void text_free_kprobes(void *page);
+#else
+static inline void *text_alloc_kprobes(unsigned long size)
+{
+ return module_alloc(size);
+}
+
+static inline void text_free_kprobes(void *page)
+{
+ module_memfree(page);
+}
+#endif

#else /* !CONFIG_KPROBES: */

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 2e97febeef77..c4f107682250 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -109,14 +109,14 @@ enum kprobe_slot_state {
SLOT_USED = 2,
};

-void __weak *alloc_insn_page(void)
+static void *alloc_insn_page(void)
{
- return module_alloc(PAGE_SIZE);
+ return text_alloc_kprobes(PAGE_SIZE);
}

-void __weak free_insn_page(void *page)
+static void free_insn_page(void *page)
{
- module_memfree(page);
+ text_free_kprobes(page);
}

struct kprobe_insn_cache kprobe_insn_slots = {
--
2.26.2


> +#endif
> +
> #endif /* _LINUX_VMALLOC_H */
> --
> 2.25.1
>

--
Sincerely yours,
Mike.

2020-07-20 12:05:21

by Masami Hiramatsu

Subject: Re: [PATCH v4 3/7] vmalloc: Add text_alloc() and text_free()

On Sat, 18 Jul 2020 19:23:59 +0300
Mike Rapoport <[email protected]> wrote:

> On Fri, Jul 17, 2020 at 06:04:17AM +0300, Jarkko Sakkinen wrote:
> > Introduce functions for allocating memory for dynamic trampolines, such
> > as kprobes. An arch can promote the availability of these functions with
> > CONFIG_ARCH_HAS_TEXT_ALLOC. Provide default/fallback implementation
> > wrapping module_alloc() and module_memfree().
> >
> > Cc: Andi Kleen <[email protected]>
> > Cc: Masami Hiramatsu <[email protected]>
> > Suggested-by: Peter Zijlstra <[email protected]>
> > Signed-off-by: Jarkko Sakkinen <[email protected]>
> > ---
> > include/linux/vmalloc.h | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> > index 0221f852a7e1..e981436e30b6 100644
> > --- a/include/linux/vmalloc.h
> > +++ b/include/linux/vmalloc.h
> > @@ -9,6 +9,7 @@
> > #include <asm/page.h> /* pgprot_t */
> > #include <linux/rbtree.h>
> > #include <linux/overflow.h>
> > +#include <linux/moduleloader.h>
> >
> > #include <asm/vmalloc.h>
> >
> > @@ -249,4 +250,26 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
> > int register_vmap_purge_notifier(struct notifier_block *nb);
> > int unregister_vmap_purge_notifier(struct notifier_block *nb);
> >
> > +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> > +/*
> > + * Allocate memory to be used for dynamic trampoline code.
> > + */
> > +void *text_alloc(unsigned long size);
> > +
> > +/*
> > + * Free memory returned from text_alloc().
> > + */
> > +void text_free(void *region);
> > +#else
> > +static inline void *text_alloc(unsigned long size)
> > +{
> > + return module_alloc(size);
> > +}
> > +
> > +static inline void text_free(void *region)
> > +{
> > + module_memfree(region);
> > +}
>
> Using module_alloc() as the default implementation of generic
> text_alloc() does not sound right to me.
>
> I would suggest rename module_alloc() to text_alloc() on x86, as Peter
> proposed and then add text_alloc_kprobes() that can be overridden by the
> architectures. x86 could use text_alloc(), arm64 vmalloc() with options
> of their choice and the fallback would remain module_alloc(). Something
> like (untested) patch below:

Hmm, of course we need some more work on this patch (e.g. MODULE_STATE_*
for callbacks), but basically, this direction seems good to me for the
first step.
If other dynamic trampoline code (ftrace and bpf) are interested in replacing
the module_alloc() with text_alloc_*(), we should revisit how we can unify
it (or at least share the common parts.)

BTW, this changes CONFIG_MODULES dependency according to the architecture
implementation. It also should be noted somewhere.

Thank you,


> From 928b6903e76ebf5790fc415f9eed390e400e5bc3 Mon Sep 17 00:00:00 2001
> From: Mike Rapoport <[email protected]>
> Date: Sat, 18 Jul 2020 19:13:02 +0300
> Subject: [PATCH] kprobes: introduce text_alloc_kprobes() and
> text_free_kprobes()
>
> Signed-off-by: Mike Rapoport <[email protected]>
> ---
> arch/Kconfig | 5 ++++-
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/probes/kprobes.c | 10 ++++++++--
> arch/x86/Kconfig | 3 ++-
> arch/x86/kernel/kprobes/core.c | 9 ++++-----
> include/linux/kprobes.h | 17 +++++++++++++++--
> kernel/kprobes.c | 8 ++++----
> 7 files changed, 38 insertions(+), 15 deletions(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 8cc35dc556c7..c0589b3b3225 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -61,7 +61,7 @@ config OPROFILE_NMI_TIMER
>
> config KPROBES
> bool "Kprobes"
> - depends on MODULES
> + depends on MODULES || HAVE_KPROBES_TEXT_ALLOC
> depends on HAVE_KPROBES
> select KALLSYMS
> help
> @@ -186,6 +186,9 @@ config HAVE_IOREMAP_PROT
> config HAVE_KPROBES
> bool
>
> +config HAVE_KPROBES_TEXT_ALLOC
> + bool
> +
> config HAVE_KRETPROBES
> bool
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 66dc41fd49f2..abc0538b13ef 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -177,6 +177,7 @@ config ARM64
> select HAVE_STACKPROTECTOR
> select HAVE_SYSCALL_TRACEPOINTS
> select HAVE_KPROBES
> + select HAVE_KPROBES_TEXT_ALLOC
> select HAVE_KRETPROBES
> select HAVE_GENERIC_VDSO
> select IOMMU_DMA if IOMMU_SUPPORT
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index 5290f17a4d80..fac8c6020040 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -118,13 +118,19 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
> return 0;
> }
>
> -void *alloc_insn_page(void)
> +void *text_alloc_kprobes(unsigned long size)
> {
> - return __vmalloc_node_range(PAGE_SIZE, 1, VMALLOC_START, VMALLOC_END,
> + return __vmalloc_node_range(size, 1, VMALLOC_START, VMALLOC_END,
> GFP_KERNEL, PAGE_KERNEL_ROX, VM_FLUSH_RESET_PERMS,
> NUMA_NO_NODE, __builtin_return_address(0));
> }
>
> +void text_free_kprobes(void *mem)
> +{
> + lockdep_assert_irqs_enabled();
> + vfree(mem);
> +}
> +
> /* arm kprobe: install breakpoint in text */
> void __kprobes arch_arm_kprobe(struct kprobe *p)
> {
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 883da0abf779..d08b92f91531 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -190,6 +190,7 @@ config X86
> select HAVE_KERNEL_XZ
> select HAVE_KPROBES
> select HAVE_KPROBES_ON_FTRACE
> + select HAVE_KPROBES_TEXT_ALLOC
> select HAVE_FUNCTION_ERROR_INJECTION
> select HAVE_KRETPROBES
> select HAVE_KVM
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index ada39ddbc922..a3e8f01c62d4 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -418,12 +418,11 @@ static int prepare_boost(kprobe_opcode_t *buf, struct kprobe *p,
> return len;
> }
>
> -/* Make page to RO mode when allocate it */
> -void *alloc_insn_page(void)
> +void *text_alloc_kprobes(unsigned long size)
> {
> void *page;
>
> - page = module_alloc(PAGE_SIZE);
> + page = text_alloc(size);
> if (!page)
> return NULL;
>
> @@ -444,9 +443,9 @@ void *alloc_insn_page(void)
> }
>
> /* Recover page to RW mode before releasing it */
> -void free_insn_page(void *page)
> +void text_free_kprobes(void *page)
> {
> - module_memfree(page);
> + text_free(page);
> }
>
> static int arch_copy_kprobe(struct kprobe *p)
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 6adf90f248d7..dbc9c71d4ec4 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -27,6 +27,7 @@
> #include <linux/rcupdate.h>
> #include <linux/mutex.h>
> #include <linux/ftrace.h>
> +#include <linux/moduleloader.h>
> #include <asm/kprobes.h>
>
> #ifdef CONFIG_KPROBES
> @@ -374,8 +375,20 @@ int enable_kprobe(struct kprobe *kp);
>
> void dump_kprobe(struct kprobe *kp);
>
> -void *alloc_insn_page(void);
> -void free_insn_page(void *page);
> +#ifdef CONFIG_HAVE_KPROBES_TEXT_ALLOC
> +void *text_alloc_kprobes(unsigned long size);
> +void text_free_kprobes(void *page);
> +#else
> +static inline void *text_alloc_kprobes(unsigned long size)
> +{
> + return module_alloc(size);
> +}
> +
> +static inline void text_free_kprobes(void *page)
> +{
> + module_memfree(page);
> +}
> +#endif
>
> #else /* !CONFIG_KPROBES: */
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 2e97febeef77..c4f107682250 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -109,14 +109,14 @@ enum kprobe_slot_state {
> SLOT_USED = 2,
> };
>
> -void __weak *alloc_insn_page(void)
> +static void *alloc_insn_page(void)
> {
> - return module_alloc(PAGE_SIZE);
> + return text_alloc_kprobes(PAGE_SIZE);
> }
>
> -void __weak free_insn_page(void *page)
> +static void free_insn_page(void *page)
> {
> - module_memfree(page);
> + text_free_kprobes(page);
> }
>
> struct kprobe_insn_cache kprobe_insn_slots = {
> --
> 2.26.2
>
>
> > +#endif
> > +
> > #endif /* _LINUX_VMALLOC_H */
> > --
> > 2.25.1
> >
>
> --
> Sincerely yours,
> Mike.


--
Masami Hiramatsu <[email protected]>

2020-07-23 22:17:41

by Jarkko Sakkinen

Subject: Re: [PATCH v4 5/7] arch/x86: kprobes: Use text_alloc() in alloc_insn_page()

On Fri, Jul 17, 2020 at 06:04:19AM +0300, Jarkko Sakkinen wrote:
> Use text_alloc() as part of the arch specific implementation for
> alloc_insn_page().
>
> Cc: Andi Kleen <[email protected]>
> Cc: Masami Hiramatsu <[email protected]>
> Signed-off-by: Jarkko Sakkinen <[email protected]>Im
> ---
> arch/x86/kernel/kprobes/core.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index ada39ddbc922..0f20a3e52a06 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -423,7 +423,7 @@ void *alloc_insn_page(void)
> {
> void *page;
>
> - page = module_alloc(PAGE_SIZE);
> + page = text_alloc(PAGE_SIZE);
> if (!page)
> return NULL;
>
> @@ -443,12 +443,6 @@ void *alloc_insn_page(void)
> return page;
> }
>
> -/* Recover page to RW mode before releasing it */
> -void free_insn_page(void *page)
> -{
> - module_memfree(page);

This must be a mistake. Should be just changed to call text_memfree().

Probably just my clumsiness when refactoring the series.

> -}
> -
> static int arch_copy_kprobe(struct kprobe *p)
> {
> struct insn insn;
> --
> 2.25.1
>

/Jarkko

2020-07-23 22:25:32

by Jarkko Sakkinen

Subject: Re: [PATCH v4 3/7] vmalloc: Add text_alloc() and text_free()

On Fri, Jul 17, 2020 at 05:52:45PM +0900, Masami Hiramatsu wrote:
> On Fri, 17 Jul 2020 06:04:17 +0300
> Jarkko Sakkinen <[email protected]> wrote:
>
> > Introduce functions for allocating memory for dynamic trampolines, such
> > as kprobes. An arch can promote the availability of these functions with
> > CONFIG_ARCH_HAS_TEXT_ALLOC. Provide default/fallback implementation
> > wrapping module_alloc() and module_memfree().
>
> Doesn't it depend on CONFIG_MODULE?

The idea would be that arch specifically promotes that it has text
allocator that is not dependent on module subsystem.

E.g. like the patch set does for x86.

/Jarkko

2020-07-23 22:29:33

by Jarkko Sakkinen

Subject: Re: [PATCH v4 3/7] vmalloc: Add text_alloc() and text_free()

On Sat, Jul 18, 2020 at 07:23:59PM +0300, Mike Rapoport wrote:
> On Fri, Jul 17, 2020 at 06:04:17AM +0300, Jarkko Sakkinen wrote:
> > Introduce functions for allocating memory for dynamic trampolines, such
> > as kprobes. An arch can promote the availability of these functions with
> > CONFIG_ARCH_HAS_TEXT_ALLOC. Provide default/fallback implementation
> > wrapping module_alloc() and module_memfree().
> >
> > Cc: Andi Kleen <[email protected]>
> > Cc: Masami Hiramatsu <[email protected]>
> > Suggested-by: Peter Zijlstra <[email protected]>
> > Signed-off-by: Jarkko Sakkinen <[email protected]>
> > ---
> > include/linux/vmalloc.h | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> > index 0221f852a7e1..e981436e30b6 100644
> > --- a/include/linux/vmalloc.h
> > +++ b/include/linux/vmalloc.h
> > @@ -9,6 +9,7 @@
> > #include <asm/page.h> /* pgprot_t */
> > #include <linux/rbtree.h>
> > #include <linux/overflow.h>
> > +#include <linux/moduleloader.h>
> >
> > #include <asm/vmalloc.h>
> >
> > @@ -249,4 +250,26 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
> > int register_vmap_purge_notifier(struct notifier_block *nb);
> > int unregister_vmap_purge_notifier(struct notifier_block *nb);
> >
> > +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> > +/*
> > + * Allocate memory to be used for dynamic trampoline code.
> > + */
> > +void *text_alloc(unsigned long size);
> > +
> > +/*
> > + * Free memory returned from text_alloc().
> > + */
> > +void text_free(void *region);
> > +#else
> > +static inline void *text_alloc(unsigned long size)
> > +{
> > + return module_alloc(size);
> > +}
> > +
> > +static inline void text_free(void *region)
> > +{
> > + module_memfree(region);
> > +}
>
> Using module_alloc() as the default implementation of generic
> text_alloc() does not sound right to me.
>
> I would suggest rename module_alloc() to text_alloc() on x86, as Peter
> proposed and then add text_alloc_kprobes() that can be overridden by the
> architectures. x86 could use text_alloc(), arm64 vmalloc() with options
> of their choice and the fallback would remain module_alloc(). Something
> like (untested) patch below:

I'm not exactly sure which of the below is relevant as the patch set
includes the exact same changes with maybe different phrasing:

https://lore.kernel.org/lkml/[email protected]/

If there is something that these patches are missing, please do remark
but these seven patches have been at least tested and split in
reasonable manner.

/Jarkko

2020-07-24 10:16:49

by Mike Rapoport

Subject: Re: [PATCH v4 3/7] vmalloc: Add text_alloc() and text_free()

On Fri, Jul 24, 2020 at 01:28:35AM +0300, Jarkko Sakkinen wrote:
> On Sat, Jul 18, 2020 at 07:23:59PM +0300, Mike Rapoport wrote:
> > On Fri, Jul 17, 2020 at 06:04:17AM +0300, Jarkko Sakkinen wrote:
> > > Introduce functions for allocating memory for dynamic trampolines, such
> > > as kprobes. An arch can promote the availability of these functions with
> > > CONFIG_ARCH_HAS_TEXT_ALLOC. Provide default/fallback implementation
> > > wrapping module_alloc() and module_memfree().
> > >
> > > Cc: Andi Kleen <[email protected]>
> > > Cc: Masami Hiramatsu <[email protected]>
> > > Suggested-by: Peter Zijlstra <[email protected]>
> > > Signed-off-by: Jarkko Sakkinen <[email protected]>
> > > ---
> > > include/linux/vmalloc.h | 23 +++++++++++++++++++++++
> > > 1 file changed, 23 insertions(+)
> > >
> > > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> > > index 0221f852a7e1..e981436e30b6 100644
> > > --- a/include/linux/vmalloc.h
> > > +++ b/include/linux/vmalloc.h
> > > @@ -9,6 +9,7 @@
> > > #include <asm/page.h> /* pgprot_t */
> > > #include <linux/rbtree.h>
> > > #include <linux/overflow.h>
> > > +#include <linux/moduleloader.h>
> > >
> > > #include <asm/vmalloc.h>
> > >
> > > @@ -249,4 +250,26 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
> > > int register_vmap_purge_notifier(struct notifier_block *nb);
> > > int unregister_vmap_purge_notifier(struct notifier_block *nb);
> > >
> > > +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> > > +/*
> > > + * Allocate memory to be used for dynamic trampoline code.
> > > + */
> > > +void *text_alloc(unsigned long size);
> > > +
> > > +/*
> > > + * Free memory returned from text_alloc().
> > > + */
> > > +void text_free(void *region);
> > > +#else
> > > +static inline void *text_alloc(unsigned long size)
> > > +{
> > > + return module_alloc(size);
> > > +}
> > > +
> > > +static inline void text_free(void *region)
> > > +{
> > > + module_memfree(region);
> > > +}
> >
> > Using module_alloc() as the default implementation of generic
> > text_alloc() does not sound right to me.
> >
> > I would suggest rename module_alloc() to text_alloc() on x86, as Peter
> > proposed and then add text_alloc_kprobes() that can be overridden by the
> > architectures. x86 could use text_alloc(), arm64 vmalloc() with options
> > of their choice and the fallback would remain module_alloc(). Something
> > like (untested) patch below:
>
> I'm not exactly sure which of the below is relevant as the patch set
> includes the exact same changes with maybe different phrasing:

The difference in parsing is what differentiates semantically clean code
from duct tape.

As several people pointed out, a single text_alloc(), and apprently a
single ARCH_HAS_TEXT_ALLOC, would not fit all architectures and some
ground work required to implement a generic text allocation.

Your patch works aroung this for x86 with broken semantics of
text_alloc() when ARCH_HAS_TEXT_ALLOC is not defined.

My suggestion does not make text_alloc() a special case of
module_alloc() but rather makes text_alloc_kprobes() to fallback to
module_alloc() when architecture does not provide its implementation.

> https://lore.kernel.org/lkml/[email protected]/
>
> If there is something that these patches are missing, please do remark
> but these seven patches have been at least tested and split in
> reasonable manner.

My patch is not tested because I only wanted to help with
transiontion from module_alloc() in kprobes to a new clean interface, I
don't really care if kprobes will depend on MODULES...

> /Jarkko

--
Sincerely yours,
Mike.

2020-07-24 23:32:07

by Jarkko Sakkinen

Subject: Re: [PATCH v4 3/7] vmalloc: Add text_alloc() and text_free()

On Fri, Jul 24, 2020 at 01:13:02PM +0300, Mike Rapoport wrote:
> On Fri, Jul 24, 2020 at 01:28:35AM +0300, Jarkko Sakkinen wrote:
> > On Sat, Jul 18, 2020 at 07:23:59PM +0300, Mike Rapoport wrote:
> > > On Fri, Jul 17, 2020 at 06:04:17AM +0300, Jarkko Sakkinen wrote:
> > > > Introduce functions for allocating memory for dynamic trampolines, such
> > > > as kprobes. An arch can promote the availability of these functions with
> > > > CONFIG_ARCH_HAS_TEXT_ALLOC. Provide default/fallback implementation
> > > > wrapping module_alloc() and module_memfree().
> > > >
> > > > Cc: Andi Kleen <[email protected]>
> > > > Cc: Masami Hiramatsu <[email protected]>
> > > > Suggested-by: Peter Zijlstra <[email protected]>
> > > > Signed-off-by: Jarkko Sakkinen <[email protected]>
> > > > ---
> > > > include/linux/vmalloc.h | 23 +++++++++++++++++++++++
> > > > 1 file changed, 23 insertions(+)
> > > >
> > > > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> > > > index 0221f852a7e1..e981436e30b6 100644
> > > > --- a/include/linux/vmalloc.h
> > > > +++ b/include/linux/vmalloc.h
> > > > @@ -9,6 +9,7 @@
> > > > #include <asm/page.h> /* pgprot_t */
> > > > #include <linux/rbtree.h>
> > > > #include <linux/overflow.h>
> > > > +#include <linux/moduleloader.h>
> > > >
> > > > #include <asm/vmalloc.h>
> > > >
> > > > @@ -249,4 +250,26 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
> > > > int register_vmap_purge_notifier(struct notifier_block *nb);
> > > > int unregister_vmap_purge_notifier(struct notifier_block *nb);
> > > >
> > > > +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> > > > +/*
> > > > + * Allocate memory to be used for dynamic trampoline code.
> > > > + */
> > > > +void *text_alloc(unsigned long size);
> > > > +
> > > > +/*
> > > > + * Free memory returned from text_alloc().
> > > > + */
> > > > +void text_free(void *region);
> > > > +#else
> > > > +static inline void *text_alloc(unsigned long size)
> > > > +{
> > > > + return module_alloc(size);
> > > > +}
> > > > +
> > > > +static inline void text_free(void *region)
> > > > +{
> > > > + module_memfree(region);
> > > > +}
> > >
> > > Using module_alloc() as the default implementation of generic
> > > text_alloc() does not sound right to me.
> > >
> > > I would suggest rename module_alloc() to text_alloc() on x86, as Peter
> > > proposed and then add text_alloc_kprobes() that can be overridden by the
> > > architectures. x86 could use text_alloc(), arm64 vmalloc() with options
> > > of their choice and the fallback would remain module_alloc(). Something
> > > like (untested) patch below:
> >
> > I'm not exactly sure which of the below is relevant as the patch set
> > includes the exact same changes with maybe different phrasing:
>
> The difference in parsing is what differentiates semantically clean code
> from duct tape.
>
> As several people pointed out, a single text_alloc(), and apprently a
> single ARCH_HAS_TEXT_ALLOC, would not fit all architectures and some
> ground work required to implement a generic text allocation.
>
> Your patch works aroung this for x86 with broken semantics of
> text_alloc() when ARCH_HAS_TEXT_ALLOC is not defined.
>
> My suggestion does not make text_alloc() a special case of
> module_alloc() but rather makes text_alloc_kprobes() to fallback to
> module_alloc() when architecture does not provide its implementation.

OK, I see your point now. I'll response in detail to v5 comments.

Thank you.

/Jarkko

2020-07-24 23:43:45

by Jarkko Sakkinen

Subject: Re: [PATCH v4 3/7] vmalloc: Add text_alloc() and text_free()

On Sat, Jul 25, 2020 at 02:31:16AM +0300, Jarkko Sakkinen wrote:
> On Fri, Jul 24, 2020 at 01:13:02PM +0300, Mike Rapoport wrote:
> > On Fri, Jul 24, 2020 at 01:28:35AM +0300, Jarkko Sakkinen wrote:
> > > On Sat, Jul 18, 2020 at 07:23:59PM +0300, Mike Rapoport wrote:
> > > > On Fri, Jul 17, 2020 at 06:04:17AM +0300, Jarkko Sakkinen wrote:
> > > > > Introduce functions for allocating memory for dynamic trampolines, such
> > > > > as kprobes. An arch can promote the availability of these functions with
> > > > > CONFIG_ARCH_HAS_TEXT_ALLOC. Provide default/fallback implementation
> > > > > wrapping module_alloc() and module_memfree().
> > > > >
> > > > > Cc: Andi Kleen <[email protected]>
> > > > > Cc: Masami Hiramatsu <[email protected]>
> > > > > Suggested-by: Peter Zijlstra <[email protected]>
> > > > > Signed-off-by: Jarkko Sakkinen <[email protected]>
> > > > > ---
> > > > > include/linux/vmalloc.h | 23 +++++++++++++++++++++++
> > > > > 1 file changed, 23 insertions(+)
> > > > >
> > > > > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> > > > > index 0221f852a7e1..e981436e30b6 100644
> > > > > --- a/include/linux/vmalloc.h
> > > > > +++ b/include/linux/vmalloc.h
> > > > > @@ -9,6 +9,7 @@
> > > > > #include <asm/page.h> /* pgprot_t */
> > > > > #include <linux/rbtree.h>
> > > > > #include <linux/overflow.h>
> > > > > +#include <linux/moduleloader.h>
> > > > >
> > > > > #include <asm/vmalloc.h>
> > > > >
> > > > > @@ -249,4 +250,26 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
> > > > > int register_vmap_purge_notifier(struct notifier_block *nb);
> > > > > int unregister_vmap_purge_notifier(struct notifier_block *nb);
> > > > >
> > > > > +#ifdef CONFIG_ARCH_HAS_TEXT_ALLOC
> > > > > +/*
> > > > > + * Allocate memory to be used for dynamic trampoline code.
> > > > > + */
> > > > > +void *text_alloc(unsigned long size);
> > > > > +
> > > > > +/*
> > > > > + * Free memory returned from text_alloc().
> > > > > + */
> > > > > +void text_free(void *region);
> > > > > +#else
> > > > > +static inline void *text_alloc(unsigned long size)
> > > > > +{
> > > > > + return module_alloc(size);
> > > > > +}
> > > > > +
> > > > > +static inline void text_free(void *region)
> > > > > +{
> > > > > + module_memfree(region);
> > > > > +}
> > > >
> > > > Using module_alloc() as the default implementation of generic
> > > > text_alloc() does not sound right to me.
> > > >
> > > > I would suggest rename module_alloc() to text_alloc() on x86, as Peter
> > > > proposed and then add text_alloc_kprobes() that can be overridden by the
> > > > architectures. x86 could use text_alloc(), arm64 vmalloc() with options
> > > > of their choice and the fallback would remain module_alloc(). Something
> > > > like (untested) patch below:
> > >
> > > I'm not exactly sure which of the below is relevant as the patch set
> > > includes the exact same changes with maybe different phrasing:
> >
> > The difference in parsing is what differentiates semantically clean code
> > from duct tape.
> >
> > As several people pointed out, a single text_alloc(), and apprently a
> > single ARCH_HAS_TEXT_ALLOC, would not fit all architectures and some
> > ground work required to implement a generic text allocation.
> >
> > Your patch works aroung this for x86 with broken semantics of
> > text_alloc() when ARCH_HAS_TEXT_ALLOC is not defined.
> >
> > My suggestion does not make text_alloc() a special case of
> > module_alloc() but rather makes text_alloc_kprobes() to fallback to
> > module_alloc() when architecture does not provide its implementation.
>
> OK, I see your point now. I'll response in detail to v5 comments.
>
> Thank you.

I also extended the explicit CC list heavily for follow up version.
Apologies if this have been somewhat confusing so far (e.g. getting
only a subportion of patches).

/Jarkko