[PATCH v3 6/6] module: add install target for modules.builtin.ranges
When CONFIG_BUILTIN_MODULE_RANGES is enabled, the modules.builtin.ranges file should be installed in the module install location. Signed-off-by: Kris Van Hees Reviewed-by: Nick Alcock --- Changes since v2: - Include modules.builtin.ranges in modules install target --- scripts/Makefile.modinst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst index 0afd75472679f..f5160ddd74239 100644 --- a/scripts/Makefile.modinst +++ b/scripts/Makefile.modinst @@ -30,10 +30,10 @@ $(MODLIB)/modules.order: modules.order FORCE quiet_cmd_install_modorder = INSTALL $@ cmd_install_modorder = sed 's:^\(.*\)\.o$$:kernel/\1.ko:' $< > $@ -# Install modules.builtin(.modinfo) even when CONFIG_MODULES is disabled. -install-y += $(addprefix $(MODLIB)/, modules.builtin modules.builtin.modinfo) +# Install modules.builtin(.modinfo,.ranges) even when CONFIG_MODULES is disabled. +install-y += $(addprefix $(MODLIB)/, modules.builtin modules.builtin.modinfo modules.builtin.ranges) -$(addprefix $(MODLIB)/, modules.builtin modules.builtin.modinfo): $(MODLIB)/%: % FORCE +$(addprefix $(MODLIB)/, modules.builtin modules.builtin.modinfo modules.builtin.ranges): $(MODLIB)/%: % FORCE $(call cmd,install) endif -- 2.43.0
[PATCH v3 5/6] kbuild: generate modules.builtin.ranges when linking the kernel
Signed-off-by: Kris Van Hees Reviewed-by: Nick Alcock --- Changes since v2: - 1st arg to generate_builtin_ranges.awk is now modules.builtin.modinfo - Use $(real-prereqs) rather than $(filter-out ...) --- scripts/Makefile.vmlinux | 16 1 file changed, 16 insertions(+) diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux index c9f3e03124d7f..afe8287e8dda0 100644 --- a/scripts/Makefile.vmlinux +++ b/scripts/Makefile.vmlinux @@ -36,6 +36,22 @@ targets += vmlinux vmlinux: scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE +$(call if_changed_dep,link_vmlinux) +# module.builtin.ranges +# --- +ifdef CONFIG_BUILTIN_MODULE_RANGES +__default: modules.builtin.ranges + +quiet_cmd_modules_builtin_ranges = GEN $@ + cmd_modules_builtin_ranges = \ + $(srctree)/scripts/generate_builtin_ranges.awk $(real-prereqs) > $@ + +vmlinux.map: vmlinux + +targets += modules.builtin.ranges +modules.builtin.ranges: modules.builtin.modinfo vmlinux.map vmlinux.o.map FORCE + $(call if_changed,modules_builtin_ranges) +endif + # Add FORCE to the prequisites of a target to force it to be always rebuilt. # --- -- 2.43.0
[PATCH v3 4/6] module: script to generate offset ranges for builtin modules
The offset range data for builtin modules is generated using:
- modules.builtin.modinfo: associates object files with module names
- vmlinux.map: provides load order of sections and offset of first member
per section
- vmlinux.o.map: provides offset of object file content per section
- .*.cmd: build cmd file with KBUILD_MODFILE and KBUILD_MODNAME
The generated data will look like:
.text - = _text
.text baf0-cb10 amd_uncore
.text 0009bd10-0009c8e0 iosf_mbi
...
.text 008e6660-008e9630 snd_soc_wcd_mbhc
.text 008e9630-008ea610 snd_soc_wcd9335 snd_soc_wcd934x snd_soc_wcd938x
.text 008ea610-008ea780 snd_soc_wcd9335
...
.data - = _sdata
.data f020-f680 amd_uncore
For each ELF section, it lists the offset of the first symbol. This can
be used to determine the base address of the section at runtime.
Next, it lists (in strict ascending order) offset ranges in that section
that cover the symbols of one or more builtin modules. Multiple ranges
can apply to a single module, and ranges can be shared between modules.
Signed-off-by: Kris Van Hees
Reviewed-by: Nick Alcock
---
Changes since v2:
- 1st arg to generate_builtin_ranges.awk is now modules.builtin.modinfo
- Switched from using modules.builtin.objs to parsing .*.cmd files
- Parse data from .*.cmd in generate_builtin_ranges.awk
---
scripts/generate_builtin_ranges.awk | 232
1 file changed, 232 insertions(+)
create mode 100755 scripts/generate_builtin_ranges.awk
diff --git a/scripts/generate_builtin_ranges.awk
b/scripts/generate_builtin_ranges.awk
new file mode 100755
index 0..6975a9c7266d9
--- /dev/null
+++ b/scripts/generate_builtin_ranges.awk
@@ -0,0 +1,232 @@
+#!/usr/bin/gawk -f
+# SPDX-License-Identifier: GPL-2.0
+# generate_builtin_ranges.awk: Generate address range data for builtin modules
+# Written by Kris Van Hees
+#
+# Usage: generate_builtin_ranges.awk modules.builtin.modinfo vmlinux.map \
+# vmlinux.o.map > modules.builtin.ranges
+#
+
+BEGIN {
+ # modules.builtin.modinfo uses \0 as record separator
+ # All other files use \n.
+ RS = "[\n\0]";
+}
+
+# Return the module name(s) (if any) associated with the given object.
+#
+# If we have seen this object before, return information from the cache.
+# Otherwise, retrieve it from the corresponding .cmd file.
+#
+function get_module_info(fn, mod, obj, mfn, s) {
+ if (fn in omod)
+ return omod[fn];
+
+ if (match(fn, /\/[^/]+$/) == 0)
+ return "";
+
+ obj = fn;
+ mod = "";
+ mfn = "";
+ fn = substr(fn, 1, RSTART) "." substr(fn, RSTART + 1) ".cmd";
+ if (getline s 0) {
+ mod = substr(s, RSTART + 17, RLENGTH - 17);
+ gsub(/['"]/, "", mod);
+ gsub(/:/, " ", mod);
+ }
+
+ if (match(s, /DKBUILD_MODFILE=[^ ]+/) > 0) {
+ mfn = substr(s, RSTART + 17, RLENGTH - 17);
+ gsub(/['"]/, "", mfn);
+ gsub(/:/, " ", mfn);
+ }
+ }
+ close(fn);
+
+# tmp = $0;
+# $0 = s;
+# print mod " " mfn " " obj " " $NF;
+# $0 = tmp;
+
+ # A single module (common case) also reflects objects that are not part
+ # of a module. Some of those objects have names that are also a module
+ # name (e.g. core). We check the associated module file name, and if
+ # they do not match, the object is not part of a module.
+ if (mod !~ / /) {
+ if (!(mod in mods))
+ return "";
+ if (mods[mod] != mfn)
+ return "";
+ }
+
+ # At this point, mod is a single (valid) module name, or a list of
+ # module names (that do not need validation).
+ omod[obj] = mod;
+ close(fn);
+
+ return mod;
+}
+
+FNR == 1 {
+ FC++;
+}
+
+# (1-old) Build a mapping to associate object files with built-in module names.
+#
+# The first file argument is used as input (modules.builtin.objs).
+#
+FC == 1 && old_behaviour {
+ sub(/:/, "");
+ mod = $1;
+ sub(/([^/]*\/)+/, "", mod);
+ sub(/\.o$/, "", mod);
+ gsub(/-/, "_", mod);
+
+ if (NF > 1) {
+ for (i = 2; i <= NF; i++) {
+ if ($i in mods)
+ mods[$i] = mods[$i] " " mod;
+ else
+ mods[$i] = mod;
+ }
+ } else
+ mods[$1] = mod;
+
+ next;
+}
+
+# (1) Build a lookup map of built-in module names.
+#
+# The first file argument is used as input (modules.builtin.modinfo).
+#
+# We are interested in lines that follow the format
+# .file=
+# and use them to record
+#
+FC == 1 && /^[^\.]+.file=/ {
+ gsub(/[\.=]/, " ");
+# print $1 " -> " $3;
+ mods[$1] = $3;
+ next;
+}
+
+# (2) Determine the load address
[PATCH v3 3/6] kbuild: generate a linker map for vmlinux.o
When CONFIG_BUILTIN_MODULE_RANGES is set, a linker map for vmlinux.o needs
to be generated. The generation of offset range data for builtin modules
depends on that linker map to know what offsets in an ELF section belong
to an object file for a particular builtin module.
Signed-off-by: Kris Van Hees
Reviewed-by: Nick Alcock
---
scripts/Makefile.vmlinux_o | 3 +++
1 file changed, 3 insertions(+)
diff --git a/scripts/Makefile.vmlinux_o b/scripts/Makefile.vmlinux_o
index 6de297916ce68..252505505e0e3 100644
--- a/scripts/Makefile.vmlinux_o
+++ b/scripts/Makefile.vmlinux_o
@@ -45,9 +45,12 @@ objtool-args = $(vmlinux-objtool-args-y) --link
# Link of vmlinux.o used for section mismatch analysis
# ---
+vmlinux-o-ld-args-$(CONFIG_BUILTIN_MODULE_RANGES) += -Map=$@.map
+
quiet_cmd_ld_vmlinux.o = LD $@
cmd_ld_vmlinux.o = \
$(LD) ${KBUILD_LDFLAGS} -r -o $@ \
+ $(vmlinux-o-ld-args-y) \
$(addprefix -T , $(initcalls-lds)) \
--whole-archive vmlinux.a --no-whole-archive \
--start-group $(KBUILD_VMLINUX_LIBS) --end-group \
--
2.43.0
[PATCH v3 2/6] trace: add CONFIG_BUILTIN_MODULE_RANGES option
The CONFIG_BUILTIN_MODULE_RANGES option controls whether offset range data is generated for kernel modules that are built into the kernel image. Signed-off-by: Kris Van Hees Reviewed-by: Nick Alcock Reviewed-by: Alan Maguire --- Changes since v2: - Add explicit dependency on FTRACE for CONFIG_BUILTIN_MODULE_RANGES --- kernel/trace/Kconfig | 18 ++ 1 file changed, 18 insertions(+) diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig index 47345bf1d4a9f..d0c82b4b3a61e 100644 --- a/kernel/trace/Kconfig +++ b/kernel/trace/Kconfig @@ -188,6 +188,24 @@ menuconfig FTRACE if FTRACE +config BUILTIN_MODULE_RANGES + bool "Generate address range information for builtin modules" + depends on FTRACE + select VMLINUX_MAP + help + When modules are built into the kernel, there will be no module name + associated with its symbols in /proc/kallsyms. Tracers may want to + identify symbols by module name and symbol name regardless of whether + the module is configured as loadable or not. + + This option generates modules.builtin.ranges in the build tree with + offset ranges (per ELF section) for the module(s) they belong to. + It also records an anchor symbol to determine the load address of the + section. + + It is fully compatible with CONFIG_RANDOMIZE_BASE and similar late- + address-modification options. + config BOOTTIME_TRACING bool "Boot-time Tracing support" depends on TRACING -- 2.43.0
[PATCH v3 1/6] kbuild: add mod(name,file)_flags to assembler flags for module objects
Module objects compiled from C source can be identified by the presence of -DKBUILD_MODFILE and -DKBUILD_MODNAME on their compile command lines. However, module objects from assembler source do not have this defines. Add $(modfile_flags) to modkern_aflags (similar to modkern_cflahs), and add $(modname_flags) to a_flags (similar to c_flags). Signed-off-by: Kris Van Hees --- scripts/Makefile.lib | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 3179747cbd2cc..a2524ffd046f4 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -234,7 +234,7 @@ modkern_rustflags = \ modkern_aflags = $(if $(part-of-module), \ $(KBUILD_AFLAGS_MODULE) $(AFLAGS_MODULE), \ - $(KBUILD_AFLAGS_KERNEL) $(AFLAGS_KERNEL)) + $(KBUILD_AFLAGS_KERNEL) $(AFLAGS_KERNEL) $(modfile_flags)) c_flags= -Wp,-MMD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \ -include $(srctree)/include/linux/compiler_types.h \ @@ -244,7 +244,7 @@ c_flags= -Wp,-MMD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \ rust_flags = $(_rust_flags) $(modkern_rustflags) @$(objtree)/include/generated/rustc_cfg a_flags= -Wp,-MMD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \ -$(_a_flags) $(modkern_aflags) +$(_a_flags) $(modkern_aflags) $(modname_flags) cpp_flags = -Wp,-MMD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \ $(_cpp_flags) -- 2.43.0
[PATCH v3 0/6] Generate address range data for built-in modules
Especially for tracing applications, it is convenient to be able to refer to a symbol using a pair and to be able to translate an address into a pair. But that does not work if the module is built into the kernel because the object files that comprise the built-in module implementation are simply linked into the kernel image along with all other kernel object files. This is especially visible when providing tracing scripts for support purposes, where the developer of the script targets a particular kernel version, but does not have control over whether the target system has a particular module as loadable module or built-in module. When tracing symbols within a module, referring them by pairs is both convenient and aids symbol lookup. But that naming will not work if the module name information is lost if the module is built into the kernel on the target system. Earlier work addressing this loss of information for built-in modules involved adding module name information to the kallsyms data, but that required more invasive code in the kernel proper. This work never did get merged into the kernel tree. All that is really needed is knowing whether a given address belongs to a particular module (or multiple modules if they share an object file). Or in other words, whether that address falls within an address range that is associated with one or more modules. Objects can be identified as belonging to a particular module (or modules) based on defines that are passed as flags to their respective compilation commands. The data found in modules.builtin.modinfo is used to determine what modules are built into the kernel proper. Then, vmlinux.o.map and vmlinux.map can be parsed in a single pass to generate a modules.buitin.ranges file with offset range information (relative to the base address of the associated section) for built-in modules. This file gets installed along with the other modules.builtin.* files. The impact on the kernel build is minimal because everything is done using a single-pass AWK script. The generated data size is minimal as well, (depending on the exact kernel configuration) usually in the range of 500-700 lines, with a file size of 20-40KB (if all modules are built in, the file contains about 8000 lines, with a file size of about 285KB). Changes since v2: - Switched from using modules.builtin.objs to parsing .*.cmd files - Add explicit dependency on FTRACE for CONFIG_BUILTIN_MODULE_RANGES - 1st arg to generate_builtin_ranges.awk is now modules.builtin.modinfo - Parse data from .*.cmd in generate_builtin_ranges.awk - Use $(real-prereqs) rather than $(filter-out ...) - Include modules.builtin.ranges in modules install target Changes since v1: - Renamed CONFIG_BUILTIN_RANGES to CONFIG_BUILTIN_MODULE_RANGES - Moved the config option to the tracers section - 2nd arg to generate_builtin_ranges.awk should be vmlinux.map Kris Van Hees (5): trace: add CONFIG_BUILTIN_MODULE_RANGES option kbuild: generate a linker map for vmlinux.o module: script to generate offset ranges for builtin modules kbuild: generate modules.builtin.ranges when linking the kernel module: add install target for modules.builtin.ranges Luis Chamberlain (1): kbuild: add modules.builtin.objs .gitignore | 2 +- Documentation/dontdiff | 2 +- Documentation/kbuild/kbuild.rst | 5 ++ Makefile| 8 +- include/linux/module.h | 4 +- kernel/trace/Kconfig| 17 scripts/Makefile.lib| 5 +- scripts/Makefile.modinst| 11 ++- scripts/Makefile.vmlinux| 17 scripts/Makefile.vmlinux_o | 18 - scripts/generate_builtin_ranges.awk | 149 11 files changed, 228 insertions(+), 10 deletions(-) create mode 100755 scripts/generate_builtin_ranges.awk base-commit: dd5a440a31fae6e459c0d627162825505361 -- 2.42.0
Re: [PATCH] vhost/vsock: always initialize seqpacket_allow
On Wed, 15 May 2024 11:05:43 -0400 Michael S. Tsirkin wrote: > There are two issues around seqpacket_allow: > 1. seqpacket_allow is not initialized when socket is >created. Thus if features are never set, it will be >read uninitialized. > 2. if VIRTIO_VSOCK_F_SEQPACKET is set and then cleared, >then seqpacket_allow will not be cleared appropriately >(existing apps I know about don't usually do this but > it's legal and there's no way to be sure no one relies > on this). Acked-by: Jakub Kicinski -- pw-bot: nap
[PATCH] vringh: add MODULE_DESCRIPTION()
Fix the allmodconfig 'make w=1' issue:
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/vhost/vringh.o
Signed-off-by: Jeff Johnson
---
drivers/vhost/vringh.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 7b8fd977f71c..73e153f9b449 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -1614,4 +1614,5 @@ EXPORT_SYMBOL(vringh_need_notify_iotlb);
#endif
+MODULE_DESCRIPTION("host side of a virtio ring");
MODULE_LICENSE("GPL");
---
base-commit: 7f094f0e3866f83ca705519b1e8f5a7d6ecce232
change-id: 20240516-md-vringh-c43803ae0ba4
Re: [PATCH RESEND v8 16/16] bpf: remove CONFIG_BPF_JIT dependency on CONFIG_MODULES of
3: 800081223dd0 x22: 3a198a40 x21: [ 44.430908] x20: d4202000 x19: 8000814dd880 x18: 0006 [ 44.439122] x17: x16: 0020 x15: 0002 [ 44.447338] x14: 8000811a6370 x13: 2000 x12: [ 44.43] x11: 8000811a6370 x10: 0166 x9 : 8000811fe370 [ 44.463771] x8 : 00017fe8 x7 : f000 x6 : 8000811fe370 [ 44.471989] x5 : x4 : x3 : [ 44.480208] x2 : x1 : x0 : 02203240 [ 44.488420] Call trace: [ 44.491847] vfree (mm/vmalloc.c:3324 (discriminator 1)) [ 44.495900] execmem_free (mm/execmem.c:70) [ 44.500394] bpf_jit_free_exec+0x10/0x1c [ 44.505329] bpf_prog_pack_free (kernel/bpf/core.c:1006) [ 44.510507] bpf_jit_binary_pack_free (kernel/bpf/core.c:1195) [ 44.516017] bpf_jit_free (include/linux/filter.h:1083 arch/arm64/net/bpf_jit_comp.c:2474) [ 44.520424] bpf_prog_free_deferred (kernel/bpf/core.c:2785) [ 44.525864] process_one_work (kernel/workqueue.c:3273) [ 44.530754] worker_thread (kernel/workqueue.c:3342 (discriminator 2) kernel/workqueue.c:3429 (discriminator 2)) [ 44.535364] kthread (kernel/kthread.c:388) [ 44.539417] ret_from_fork (arch/arm64/kernel/entry.S:861) [ 44.543791] ---[ end trace ]--- # bad: [dbd9e2e056d8577375ae4b31ada94f8aa3769e8a] Add linux-next specific files for 20240516 git bisect start 'next/master' # status: waiting for good commit(s), bad commit known # good: [8c06da67d0bd3139a97f301b4aa9c482b9d4f29e] Merge tag 'livepatching-for-6.10' of git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching git bisect good 8c06da67d0bd3139a97f301b4aa9c482b9d4f29e # good: [147d3734724040bb0aff1252299e48947a6c8858] Merge branch 'master' of git://linuxtv.org/mchehab/media-next.git git bisect good 147d3734724040bb0aff1252299e48947a6c8858 # bad: [729cf96da8de5e7ae70fef40a1b864bc00c2dca1] Merge branch 'next' of git://git.kernel.org/pub/scm/virt/kvm/kvm.git git bisect bad 729cf96da8de5e7ae70fef40a1b864bc00c2dca1 # good: [4364438497c638785b1394aab764a15b6baefaf3] Merge branch 'drm-xe-next' of https://gitlab.freedesktop.org/drm/xe/kernel git bisect good 4364438497c638785b1394aab764a15b6baefaf3 # bad: [b3ead6c10eccbfa446ce30927f94472c278cd3d7] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git git bisect bad b3ead6c10eccbfa446ce30927f94472c278cd3d7 # bad: [d83384f475a4cfa0e9bda1cab538d99360fa2c48] Merge branch 'for-mfd-next' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git git bisect bad d83384f475a4cfa0e9bda1cab538d99360fa2c48 # bad: [9564f97e8e3ec6bdbf0c105b45fa2516d64c4685] Merge branch 'for-next' of git://git.kernel.dk/linux-block.git git bisect bad 9564f97e8e3ec6bdbf0c105b45fa2516d64c4685 # bad: [0e6c77dedcb11f510c0dbdaf6455b918b28f1b62] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git git bisect bad 0e6c77dedcb11f510c0dbdaf6455b918b28f1b62 # good: [5852f2afcdd9b7c9dedec4fdf14b8b079349828f] Input: drop explicit initialization of struct i2c_device_id::driver_data to 0 git bisect good 5852f2afcdd9b7c9dedec4fdf14b8b079349828f # good: [223b5e57d0d50b0c07b933350dbcde92018d3080] mm/execmem, arch: convert remaining overrides of module_alloc to execmem git bisect good 223b5e57d0d50b0c07b933350dbcde92018d3080 # good: [14e56fb2ed1dbc3c3171d12ab435b0f691f6f215] x86/ftrace: enable dynamic ftrace without CONFIG_MODULES git bisect good 14e56fb2ed1dbc3c3171d12ab435b0f691f6f215 # good: [7582b7be16d0ba90e3dbd9575a730cabd9eb852a] kprobes: remove dependency on CONFIG_MODULES git bisect good 7582b7be16d0ba90e3dbd9575a730cabd9eb852a # bad: [86d899efdd58c98a0d196e31945009fc47a56264] Merge branch 'modules-next' of git://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git git bisect bad 86d899efdd58c98a0d196e31945009fc47a56264 # bad: [2c9e5d4a008293407836d29d35dfd4353615bd2f] bpf: remove CONFIG_BPF_JIT dependency on CONFIG_MODULES of git bisect bad 2c9e5d4a008293407836d29d35dfd4353615bd2f # first bad commit: [2c9e5d4a008293407836d29d35dfd4353615bd2f] bpf: remove CONFIG_BPF_JIT dependency on CONFIG_MODULES of config.gz Description: application/gzip
Re: [RFC][PATCH] uprobe: support for private hugetlb mappings
On 16.05.24 19:44, Guillaume Morin wrote: On 02 May 5:59, Guillaume Morin wrote: On 30 Apr 21:25, David Hildenbrand wrote: I tried to get the hugepd stuff right but this was the first I heard about it :-) Afaict follow_huge_pmd and friends were already DTRT I'll have to have a closer look at some details (the hugepd writability check looks a bit odd), but it's mostly what I would have expected! Ok in the meantime, here is the uprobe change on your current uprobes_cow trying to address the comments you made in your previous message. Some of them were not 100% clear to me, so it's a best effort patch :-) Again lightly tested David, have you had a chance to take a look at both patches? Not in detail, last weeks were busy (currently traveling back home from LSF/MM). I'll try to find time within the next two weeks to polish my changes and send them out. It would be great if you could send your stuff based on top of that then. (the merge window just opened on Saturday, so we have plenty of time to make it to the next one :) ) -- Cheers, David / dhildenb
Re: [RFC][PATCH] uprobe: support for private hugetlb mappings
On 02 May 5:59, Guillaume Morin wrote: > > On 30 Apr 21:25, David Hildenbrand wrote: > > > I tried to get the hugepd stuff right but this was the first I heard > > > about it :-) Afaict follow_huge_pmd and friends were already DTRT > > > > I'll have to have a closer look at some details (the hugepd writability > > check looks a bit odd), but it's mostly what I would have expected! > > Ok in the meantime, here is the uprobe change on your current > uprobes_cow trying to address the comments you made in your previous > message. Some of them were not 100% clear to me, so it's a best effort > patch :-) Again lightly tested David, have you had a chance to take a look at both patches? -- Guillaume Morin
Re: [PATCH] vhost/vsock: always initialize seqpacket_allow
On Wed, May 15, 2024 at 5:05 PM Michael S. Tsirkin wrote:
>
> There are two issues around seqpacket_allow:
> 1. seqpacket_allow is not initialized when socket is
>created. Thus if features are never set, it will be
>read uninitialized.
> 2. if VIRTIO_VSOCK_F_SEQPACKET is set and then cleared,
>then seqpacket_allow will not be cleared appropriately
>(existing apps I know about don't usually do this but
> it's legal and there's no way to be sure no one relies
> on this).
>
> To fix:
> - initialize seqpacket_allow after allocation
> - set it unconditionally in set_features
>
> Reported-by: syzbot+6c21aeb59d0e82eb2...@syzkaller.appspotmail.com
> Reported-by: Jeongjun Park
> Fixes: ced7b713711f ("vhost/vsock: support SEQPACKET for transport").
> Cc: Arseny Krasnov
> Cc: David S. Miller
> Cc: Stefan Hajnoczi
> Signed-off-by: Michael S. Tsirkin
> Acked-by: Arseniy Krasnov
> Tested-by: Arseniy Krasnov
>
Reviewed-by: Eugenio Pérez
Thanks!
> ---
>
>
> Reposting now it's been tested.
>
> drivers/vhost/vsock.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index ec20ecff85c7..bf664ec9341b 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -667,6 +667,7 @@ static int vhost_vsock_dev_open(struct inode *inode,
> struct file *file)
> }
>
> vsock->guest_cid = 0; /* no CID assigned yet */
> + vsock->seqpacket_allow = false;
>
> atomic_set(>queued_replies, 0);
>
> @@ -810,8 +811,7 @@ static int vhost_vsock_set_features(struct vhost_vsock
> *vsock, u64 features)
> goto err;
> }
>
> - if (features & (1ULL << VIRTIO_VSOCK_F_SEQPACKET))
> - vsock->seqpacket_allow = true;
> + vsock->seqpacket_allow = features & (1ULL <<
> VIRTIO_VSOCK_F_SEQPACKET);
>
> for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
> vq = >vqs[i];
> --
> MST
>
[PATCH v2 6/6] selftests: livepatch: Test livepatching function using an external symbol
The test proves that klp-convert works as intended and it is possible to
livepatch a function that use an external symbol.
Signed-off-by: Lukas Hruska
---
.../testing/selftests/livepatch/functions.sh | 16 +-
.../selftests/livepatch/test-extern.sh| 57 +++
.../selftests/livepatch/test_modules/Makefile | 2 +
.../livepatch/test_modules/test_klp_extern.c | 51 +
.../test_modules/test_klp_extern_hello.c | 36
5 files changed, 161 insertions(+), 1 deletion(-)
create mode 100755 tools/testing/selftests/livepatch/test-extern.sh
create mode 100644
tools/testing/selftests/livepatch/test_modules/test_klp_extern.c
create mode 100644
tools/testing/selftests/livepatch/test_modules/test_klp_extern_hello.c
diff --git a/tools/testing/selftests/livepatch/functions.sh
b/tools/testing/selftests/livepatch/functions.sh
index fc4c6a016d38..801d55dc06ac 100644
--- a/tools/testing/selftests/livepatch/functions.sh
+++ b/tools/testing/selftests/livepatch/functions.sh
@@ -7,6 +7,7 @@
MAX_RETRIES=600
RETRY_INTERVAL=".1"# seconds
KLP_SYSFS_DIR="/sys/kernel/livepatch"
+MODULE_SYSFS_DIR="/sys/module"
# Kselftest framework requirement - SKIP code is 4
ksft_skip=4
@@ -299,7 +300,7 @@ function check_result {
result=$(dmesg | awk -v last_dmesg="$LAST_DMESG" 'p; $0 == last_dmesg {
p=1 }' | \
grep -e 'livepatch:' -e 'test_klp' | \
grep -v '\(tainting\|taints\) kernel' | \
-sed 's/^\[[ 0-9.]*\] //')
+sed 's/^\[[ 0-9.]*\] //' | sed 's/^test_klp_log: //')
if [[ "$expect" == "$result" ]] ; then
echo "ok"
@@ -344,3 +345,16 @@ function check_sysfs_value() {
die "Unexpected value in $path: $expected_value vs. $value"
fi
}
+
+# read_module_param_value(modname, param) - read module parameter value
+# modname - livepatch module creating the sysfs interface
+# param - parameter name
+function read_module_param() {
+ local mod="$1"; shift
+ local param="$1"; shift
+
+ local path="$MODULE_SYSFS_DIR/$mod/parameters/$param"
+
+ log "% echo \"$mod/parameters/$param: \$(cat $path)\""
+ log "$mod/parameters/$param: $(cat $path)"
+}
diff --git a/tools/testing/selftests/livepatch/test-extern.sh
b/tools/testing/selftests/livepatch/test-extern.sh
new file mode 100755
index ..3dde6cabb07c
--- /dev/null
+++ b/tools/testing/selftests/livepatch/test-extern.sh
@@ -0,0 +1,57 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2024 Lukas Hruska
+
+. $(dirname $0)/functions.sh
+
+MOD_LIVEPATCH=test_klp_extern
+MOD_HELLO=test_klp_extern_hello
+PARAM_HELLO=hello
+
+setup_config
+
+# - load a module to be livepatched
+# - load a livepatch that modifies the output from 'hello' parameter
+# of the previously loaded module and verify correct behaviour
+# - unload the livepatch and make sure the patch was removed
+# - unload the module that was livepatched
+
+start_test "livepatch with external symbol"
+
+load_mod $MOD_HELLO
+
+read_module_param $MOD_HELLO $PARAM_HELLO
+
+load_lp $MOD_LIVEPATCH
+
+read_module_param $MOD_HELLO $PARAM_HELLO
+
+disable_lp $MOD_LIVEPATCH
+unload_lp $MOD_LIVEPATCH
+
+read_module_param $MOD_HELLO $PARAM_HELLO
+
+unload_mod $MOD_HELLO
+
+check_result "% insmod test_modules/$MOD_HELLO.ko
+% echo \"$MOD_HELLO/parameters/$PARAM_HELLO: \$(cat
/sys/module/$MOD_HELLO/parameters/$PARAM_HELLO)\"
+$MOD_HELLO/parameters/$PARAM_HELLO: Hello from kernel module.
+% insmod test_modules/$MOD_LIVEPATCH.ko
+livepatch: enabling patch '$MOD_LIVEPATCH'
+livepatch: '$MOD_LIVEPATCH': initializing patching transition
+livepatch: '$MOD_LIVEPATCH': starting patching transition
+livepatch: '$MOD_LIVEPATCH': completing patching transition
+livepatch: '$MOD_LIVEPATCH': patching complete
+% echo \"$MOD_HELLO/parameters/$PARAM_HELLO: \$(cat
/sys/module/$MOD_HELLO/parameters/$PARAM_HELLO)\"
+$MOD_HELLO/parameters/$PARAM_HELLO: Hello from livepatched module.
+% echo 0 > /sys/kernel/livepatch/$MOD_LIVEPATCH/enabled
+livepatch: '$MOD_LIVEPATCH': initializing unpatching transition
+livepatch: '$MOD_LIVEPATCH': starting unpatching transition
+livepatch: '$MOD_LIVEPATCH': completing unpatching transition
+livepatch: '$MOD_LIVEPATCH': unpatching complete
+% rmmod $MOD_LIVEPATCH
+% echo \"$MOD_HELLO/parameters/$PARAM_HELLO: \$(cat
/sys/module/$MOD_HELLO/parameters/$PARAM_HELLO)\"
+$MOD_HELLO/parameters/$PARAM_HELLO: Hello from kernel module.
+% rmmod $MOD_HELLO"
+
+exit 0
diff --git a/tools/testing/selftests/livepatch/test_modules/Makefile
b/tools/testing/selftests/livepatch/test_modules/Makefile
index e6e638c4bcba..0d6df14787da 100644
--- a/tools/testing/selftests/livepatch/test_modules/Makefile
+++ b/tools/testing/selftests/livepatch/test_modules/Makefile
@@ -6,6 +6,8 @@ obj-m += test_klp_atomic_replace.o \
test_klp_callbacks_demo.o \
test_klp_callbacks_demo2.o \
test_klp_callbacks_mod.o \
+
[PATCH v2 5/6] documentation: Update on livepatch elf format
Add a section to Documentation/livepatch/module-elf-format.rst describing how klp-convert works for fixing relocations. Signed-off-by: Lukas Hruska Reviewed-by: Petr Mladek Reviewed-by: Marcos Paulo de Souza --- Documentation/livepatch/module-elf-format.rst | 67 +++ 1 file changed, 67 insertions(+) diff --git a/Documentation/livepatch/module-elf-format.rst b/Documentation/livepatch/module-elf-format.rst index a03ed02ec57e..2aa9b11cd806 100644 --- a/Documentation/livepatch/module-elf-format.rst +++ b/Documentation/livepatch/module-elf-format.rst @@ -300,3 +300,70 @@ symbol table, and relocation section indices, ELF information is preserved for livepatch modules and is made accessible by the module loader through module->klp_info, which is a :c:type:`klp_modinfo` struct. When a livepatch module loads, this struct is filled in by the module loader. + +6. klp-convert tool +=== +The livepatch relocation sections might be created using +scripts/livepatch/klp-convert. It is called automatically during +the build as part of a module post processing. + +The tool is not able to find the symbols and all the metadata +automatically. Instead, all needed information must already be +part of rela entry for the given symbol. Such a rela can +be created easily by using KLP_RELOC_SYMBOL() macro after +the symbol declaration. + +KLP_RELOC_SYMBOL causes that the relocation entries for +the given symbol will be created in the following format:: + + .klp.sym.rela.lp_object.sym_object.sym_name,sympos + ^ ^ ^ ^ ^^ ^ ^ ^ + |___| |___| || |__| | + [A] [B][C] [D][E] + +[A] + The symbol name is prefixed with the string ".klp.sym.rela." + +[B] + The name of the object (i.e. "vmlinux" or name of module) which + is livepatched. + +[C] + The name of the object (i.e. "vmlinux" or name of module) to + which the symbol belongs follows immediately after the prefix. + +[D] + The actual name of the symbol. + +[E] + The position of the symbol in the object (as according to kallsyms) + This is used to differentiate duplicate symbols within the same + object. The symbol position is expressed numerically (0, 1, 2...). + The symbol position of a unique symbol is 0. + +Example: + +**Livepatch source code:** + +:: + + extern char *saved_command_line \ + KLP_RELOC_SYMBOL(vmlinux, vmlinux, saved_command_line, 0); + +**`readelf -r -W` output of compiled module:** + +:: + + Relocation section '.rela.text' at offset 0x32e60 contains 10 entries: + Offset Info Type Symbol's Value Symbol's Name + Addend + ... + 0068 003c0002 R_X86_64_PC32 .klp.sym.rela.vmlinux.vmlinux.saved_command_line,0 - 4 + ... + +**`readelf -r -W` output of transformed module by klp-convert:** + +:: + + Relocation section '.klp.rela.vmlinux.text' at offset 0x5cb60 contains 1 entry: + Offset Info Type Symbol's Value Symbol's Name + Addend + 0068 003c0002 R_X86_64_PC32 .klp.sym.vmlinux.saved_command_line,0 - 4 -- 2.45.0
[PATCH v2 4/6] livepatch: Add sample livepatch module
From: Josh Poimboeuf
Add a new livepatch sample in samples/livepatch/ to make use of symbols
that must be post-processed to enable load-time relocation resolution.
As the new sample is to be used as an example, it is annotated with
KLP_RELOC_SYMBOL macro.
The livepatch sample updates the function cmdline_proc_show to print the
string referenced by the symbol saved_command_line appended by the
string "livepatch=1".
Signed-off-by: Josh Poimboeuf
Signed-off-by: Lukas Hruska
Reviewed-by: Petr Mladek
---
samples/livepatch/Makefile | 1 +
samples/livepatch/livepatch-extern-symbol.c | 84 +
2 files changed, 85 insertions(+)
create mode 100644 samples/livepatch/livepatch-extern-symbol.c
diff --git a/samples/livepatch/Makefile b/samples/livepatch/Makefile
index 9f853eeb6140..f2b41f4d6c16 100644
--- a/samples/livepatch/Makefile
+++ b/samples/livepatch/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-fix2.o
obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-callbacks-demo.o
obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-callbacks-mod.o
obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-callbacks-busymod.o
+obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-annotated-sample.o
diff --git a/samples/livepatch/livepatch-extern-symbol.c
b/samples/livepatch/livepatch-extern-symbol.c
new file mode 100644
index ..276a43d157b4
--- /dev/null
+++ b/samples/livepatch/livepatch-extern-symbol.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2014 Seth Jennings
+ */
+
+/*
+ * livepatch-extern-symbol.c - Kernel Live Patching Sample Module
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include
+#include
+#include
+
+/*
+ * This (dumb) live patch overrides the function that prints the
+ * kernel boot cmdline when /proc/cmdline is read.
+ *
+ * This livepatch uses the symbol saved_command_line whose relocation
+ * must be resolved during load time. To enable that, this module
+ * must be post-processed by a tool called klp-convert, which embeds
+ * information to be used by the loader to solve the relocation.
+ *
+ * The module is annotated with KLP_RELOC_SYMBOL macros.
+ * These annotations are used by klp-convert to infer that the symbol
+ * saved_command_line is in the object vmlinux.
+ *
+ * Example:
+ *
+ * $ cat /proc/cmdline
+ *
+ *
+ * $ insmod livepatch-sample.ko
+ * $ cat /proc/cmdline
+ * livepatch=1
+ *
+ * $ echo 0 > /sys/kernel/livepatch/livepatch_sample/enabled
+ * $ cat /proc/cmdline
+ *
+ */
+
+extern char *saved_command_line \
+ KLP_RELOC_SYMBOL(vmlinux, vmlinux, saved_command_line);
+
+#include
+static int livepatch_cmdline_proc_show(struct seq_file *m, void *v)
+{
+ seq_printf(m, "%s livepatch=1\n", saved_command_line);
+ return 0;
+}
+
+static struct klp_func funcs[] = {
+ {
+ .old_name = "cmdline_proc_show",
+ .new_func = livepatch_cmdline_proc_show,
+ }, { }
+};
+
+static struct klp_object objs[] = {
+ {
+ /* name being NULL means vmlinux */
+ .funcs = funcs,
+ }, { }
+};
+
+static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+};
+
+static int livepatch_init(void)
+{
+ return klp_enable_patch();
+}
+
+static void livepatch_exit(void)
+{
+}
+
+module_init(livepatch_init);
+module_exit(livepatch_exit);
+MODULE_LICENSE("GPL");
+MODULE_INFO(livepatch, "Y");
--
2.45.0
[PATCH v2 2/6] livepatch: Add klp-convert tool
Livepatches need to access external symbols which can't be handled by the normal relocation mechanism. It is needed for two types of symbols: + Symbols which can be local for the original livepatched function. The alternative implementation in the livepatch sees them as external symbols. + Symbols in modules which are exported via EXPORT_SYMBOL*(). They must be handled special way otherwise the livepatch module would depend on the livepatched one. Loading such livepatch would cause loading the other module as well. The address of these symbols can be found via kallsyms. Or they can be relocated using livepatch specific relocation sections as specified in Documentation/livepatch/module-elf-format.txt. Currently, there is no trivial way to embed the required information as requested in the final livepatch elf object. klp-convert solves this problem by using annotations in the elf object to convert the relocation accordingly to the specification, enabling it to be handled by the livepatch loader. Given the above, create scripts/livepatch to hold tools developed for livepatches and add source files for klp-convert there. Allow to annotate such external symbols in the livepatch by a macro KLP_RELOC_SYMBOL(). It will create symbol with all needed metadata. For example: extern char *saved_command_line \ KLP_RELOC_SYMBOL(vmlinux, vmlinux, saved_command_line, 0); would create symbol $>readelf -r -W : Relocation section '.rela.text' at offset 0x32e60 contains 10 entries: Offset Info Type Symbol's Value Symbol's Name + Addend [...] 0068 003c0002 R_X86_64_PC32 .klp.sym.rela.vmlinux.vmlinux.saved_command_line,0 - 4 [...] Also add scripts/livepatch/klp-convert. The tool transforms symbols created by KLP_RELOC_SYMBOL() to object specific rela sections and rela entries which would later be proceed when the livepatch or the livepatched object is loaded. For example, klp-convert would replace the above symbol with: $> readelf -r -W Relocation section '.klp.rela.vmlinux.text' at offset 0x5cb60 contains 1 entry: Offset Info Type Symbol's Value Symbol's Name + Addend 0068 003c0002 R_X86_64_PC32 .klp.sym.vmlinux.saved_command_line,0 - 4 klp-convert relies on libelf and on a list implementation. Add files scripts/livepatch/elf.c and scripts/livepatch/elf.h, which are a libelf interfacing layer and scripts/livepatch/list.h, which is a list implementation. Update Makefiles to correctly support the compilation of the new tool, update MAINTAINERS file and add a .gitignore file. [jpoim...@redhat.com: initial version] Signed-off-by: Josh Poimboeuf [joe.lawre...@redhat.com: clean-up and fixes] Signed-off-by: Joe Lawrence [lhru...@suse.cz: klp-convert code, minimal approach] Signed-off-by: Lukas Hruska Reviewed-by: Marcos Paulo de Souza --- MAINTAINERS | 1 + include/linux/livepatch.h | 19 + scripts/Makefile| 1 + scripts/livepatch/.gitignore| 1 + scripts/livepatch/Makefile | 5 + scripts/livepatch/elf.c | 817 scripts/livepatch/elf.h | 73 +++ scripts/livepatch/klp-convert.c | 284 +++ scripts/livepatch/klp-convert.h | 23 + scripts/livepatch/list.h| 391 +++ 10 files changed, 1615 insertions(+) create mode 100644 scripts/livepatch/.gitignore create mode 100644 scripts/livepatch/Makefile create mode 100644 scripts/livepatch/elf.c create mode 100644 scripts/livepatch/elf.h create mode 100644 scripts/livepatch/klp-convert.c create mode 100644 scripts/livepatch/klp-convert.h create mode 100644 scripts/livepatch/list.h diff --git a/MAINTAINERS b/MAINTAINERS index 130b8b0bd4f7..d2facc1f4e15 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12618,6 +12618,7 @@ F: include/uapi/linux/livepatch.h F: kernel/livepatch/ F: kernel/module/livepatch.c F: samples/livepatch/ +F: scripts/livepatch/ F: tools/testing/selftests/livepatch/ LLC (802.2) diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h index 9b9b38e89563..83bbcd1c43fd 100644 --- a/include/linux/livepatch.h +++ b/include/linux/livepatch.h @@ -235,6 +235,25 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs, unsigned int symindex, unsigned int secindex, const char *objname); +/** + * KLP_RELOC_SYMBOL_POS - define relocation for external symbols + * + * @LP_OBJ_NAME: name of the livepatched object where the symbol is needed + * @SYM_OBJ_NAME: name of the object where the symbol exists + * @SYM_NAME: symbol name + * @SYM_POS: position of the symbol in SYM_OBJ when there are more + * symbols of the same name. + * + * Use for annotating external symbols used in livepatches
[PATCH v2 3/6] kbuild/modpost: integrate klp-convert
From: Josh Poimboeuf
Call klp-convert for the livepatch modules after the final linking.
Also update the modpost tool so that it does not warn about unresolved
symbols matching the expected format which will be then resolved by
klp-convert.
Signed-off-by: Josh Poimboeuf
Signed-off-by: Lukas Hruska
Reviewed-by: Petr Mladek
Reviewed-by: Marcos Paulo de Souza
---
.gitignore| 1 +
Makefile | 10 ++
scripts/Makefile.modfinal | 15 +++
scripts/Makefile.modpost | 5 +
scripts/mod/modpost.c | 36 ++--
scripts/mod/modpost.h | 3 +++
6 files changed, 64 insertions(+), 6 deletions(-)
diff --git a/.gitignore b/.gitignore
index c59dc60ba62e..9b7d492cac99 100644
--- a/.gitignore
+++ b/.gitignore
@@ -70,6 +70,7 @@ modules.order
/Module.markers
/modules.builtin
/modules.builtin.modinfo
+/modules.livepatch
/modules.nsdeps
#
diff --git a/Makefile b/Makefile
index 967e97878ecd..db06e5db21af 100644
--- a/Makefile
+++ b/Makefile
@@ -1095,6 +1095,7 @@ PHONY += prepare0
export extmod_prefix = $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)
export MODORDER := $(extmod_prefix)modules.order
export MODULES_NSDEPS := $(extmod_prefix)modules.nsdeps
+export MODULES_LIVEPATCH := $(extmod_prefix)modules.livepatch
ifeq ($(KBUILD_EXTMOD),)
@@ -1453,8 +1454,8 @@ endif
#
# *.ko are usually independent of vmlinux, but CONFIG_DEBUG_INFO_BTF_MODULES
-# is an exception.
-ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+# and CONFIG_LIVEPATCH are exceptions.
+ifneq ($(or $(CONFIG_DEBUG_INFO_BTF_MODULES),$(CONFIG_LIVEPATCH)),)
KBUILD_BUILTIN := 1
modules: vmlinux
endif
@@ -1477,8 +1478,9 @@ endif # CONFIG_MODULES
# Directories & files removed with 'make clean'
CLEAN_FILES += vmlinux.symvers modules-only.symvers \
modules.builtin modules.builtin.modinfo modules.nsdeps \
- compile_commands.json .thinlto-cache rust/test \
- rust-project.json .vmlinux.objs .vmlinux.export.c
+ modules.livepatch compile_commands.json .thinlto-cache \
+ rust/test rust-project.json .vmlinux.objs \
+ .vmlinux.export.c
# Directories & files removed with 'make mrproper'
MRPROPER_FILES += include/config include/generated \
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 79fcf2731686..a2a162d204f6 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -14,6 +14,7 @@ include $(srctree)/scripts/Makefile.lib
# find all modules listed in modules.order
modules := $(call read-file, $(MODORDER))
+modules-klp := $(call read-file, $(MODULES_LIVEPATCH))
__modfinal: $(modules:%.o=%.ko)
@:
@@ -60,6 +61,20 @@ endif
targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o)
+# Livepatch
+# ---
+
+%.tmp.ko: %.o %.mod.o FORCE
+ +$(call if_changed,ld_ko_o)
+
+quiet_cmd_klp_convert = KLP $@
+ cmd_klp_convert = scripts/livepatch/klp-convert $< $@
+
+$(modules-klp:%.o=%.ko): %.ko: %.tmp.ko FORCE
+ $(call if_changed,klp_convert)
+
+targets += $(modules-klp:.ko=.tmp.ko)
+
# Add FORCE to the prequisites of a target to force it to be always rebuilt.
# ---
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 739402f45509..3c765c568e34 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -48,6 +48,7 @@ modpost-args =
\
$(if $(KBUILD_MODPOST_WARN),-w)
\
$(if $(KBUILD_NSDEPS),-d $(MODULES_NSDEPS))
\
$(if
$(CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS)$(KBUILD_NSDEPS),-N) \
+ $(if $(CONFIG_LIVEPATCH),-l $(MODULES_LIVEPATCH))
\
$(if $(findstring 1, $(KBUILD_EXTRA_WARN)),-W)
\
-o $@
@@ -145,6 +146,10 @@ $(output-symdump): $(modpost-deps) FORCE
$(call if_changed,modpost)
__modpost: $(output-symdump)
+ifndef CONFIG_LIVEPATCH
+ $(Q)rm -f $(MODULES_LIVEPATCH)
+ $(Q)touch $(MODULES_LIVEPATCH)
+endif
PHONY += FORCE
FORCE:
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 2f5b91da5afa..15d7d2de653e 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1654,6 +1654,10 @@ static void read_symbols(const char *modname)
}
}
+ /* Livepatch modules have unresolved symbols resolved by klp-convert */
+ if (get_modinfo(, "livepatch"))
+ mod->is_livepatch = true;
+
if (extra_warn && !get_modinfo(, "description"))
warn("missing MODULE_DESCRIPTION() in %s\n", modname);
for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
@@ -1742,10 +1746,18
[PATCH v2 1/6] livepatch: Create and include UAPI headers
From: Josh Poimboeuf
Define klp prefixes in include/uapi/linux/livepatch.h, and use them for
replacing hard-coded values in kernel/livepatch/core.c.
Signed-off-by: Josh Poimboeuf
Signed-off-by: Lukas Hruska
Reviewed-by: Petr Mladek
Reviewed-by: Marcos Paulo de Souza
---
MAINTAINERS| 1 +
include/uapi/linux/livepatch.h | 15 +++
kernel/livepatch/core.c| 5 +++--
3 files changed, 19 insertions(+), 2 deletions(-)
create mode 100644 include/uapi/linux/livepatch.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 28e20975c26f..130b8b0bd4f7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12614,6 +12614,7 @@ F: Documentation/ABI/testing/sysfs-kernel-livepatch
F: Documentation/livepatch/
F: arch/powerpc/include/asm/livepatch.h
F: include/linux/livepatch.h
+F: include/uapi/linux/livepatch.h
F: kernel/livepatch/
F: kernel/module/livepatch.c
F: samples/livepatch/
diff --git a/include/uapi/linux/livepatch.h b/include/uapi/linux/livepatch.h
new file mode 100644
index ..e19430918a07
--- /dev/null
+++ b/include/uapi/linux/livepatch.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+/*
+ * livepatch.h - Kernel Live Patching Core
+ *
+ * Copyright (C) 2016 Josh Poimboeuf
+ */
+
+#ifndef _UAPI_LIVEPATCH_H
+#define _UAPI_LIVEPATCH_H
+
+#define KLP_RELA_PREFIX".klp.rela."
+#define KLP_SYM_PREFIX ".klp.sym."
+
+#endif /* _UAPI_LIVEPATCH_H */
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index ecbc9b6aba3a..8bde84d85263 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include "core.h"
#include "patch.h"
@@ -226,7 +227,7 @@ static int klp_resolve_symbols(Elf_Shdr *sechdrs, const
char *strtab,
/* Format: .klp.sym.sym_objname.sym_name,sympos */
cnt = sscanf(strtab + sym->st_name,
-".klp.sym.%55[^.].%511[^,],%lu",
+KLP_SYM_PREFIX "%55[^.].%511[^,],%lu",
sym_objname, sym_name, );
if (cnt != 3) {
pr_err("symbol %s has an incorrectly formatted name\n",
@@ -305,7 +306,7 @@ static int klp_write_section_relocs(struct module *pmod,
Elf_Shdr *sechdrs,
* See comment in klp_resolve_symbols() for an explanation
* of the selected field width value.
*/
- cnt = sscanf(shstrtab + sec->sh_name, ".klp.rela.%55[^.]",
+ cnt = sscanf(shstrtab + sec->sh_name, KLP_RELA_PREFIX "%55[^.]",
sec_objname);
if (cnt != 1) {
pr_err("section %s has an incorrectly formatted name\n",
--
2.45.0
[PATCH v2 0/5] livepatch: klp-convert tool - Minimal version
Summary --- This is a significantly simplified version of the original klp-convert tool. The klp-convert code has never got a proper review and also clean ups were not easy. The last version was v7, see https://lore.kernel.org/r/20230306140824.3858543-1-joe.lawre...@redhat.com The main change is that the tool does not longer search for the symbols which would need the livepatch specific relocation entry. Also klp.symbols file is not longer needed. Instead, the needed information is appended to the symbol declaration via a new macro KLP_RELOC_SYMBOL(). It creates symbol with all needed metadata. For example: extern char *saved_command_line \ KLP_RELOC_SYMBOL(vmlinux, vmlinux, saved_command_line, 0); would create symbol $>readelf -r -W : Relocation section '.rela.text' at offset 0x32e60 contains 10 entries: Offset Info Type Symbol's Value Symbol's Name + Addend [...] 0068 003c0002 R_X86_64_PC32 .klp.sym.rela.vmlinux.vmlinux.saved_command_line,0 - 4 [...] The simplified klp-convert tool just transforms symbols created by KLP_RELOC_SYMBOL() to object specific rela sections and rela entries which would later be proceed when the livepatch or the livepatched object is loaded. For example, klp-convert would replace the above symbols with: $> readelf -r -W Relocation section '.klp.rela.vmlinux.text' at offset 0x5cb60 contains 1 entry: Offset Info Type Symbol's Value Symbol's Name + Addend 0068 003c0002 R_X86_64_PC32 .klp.sym.vmlinux.saved_command_line,0 - 4 Note that similar macro was needed also in the original version to handle more symbols of the same name (sympos). Given the above, add klp-convert tool; integrate klp-convert tool into kbuild; add data-structure and macros to enable users to annotate livepatch source code; make modpost stage compatible with livepatches; update livepatch-sample and update documentation. Testing --- The patchset selftests build and execute on x86_64, s390x, and ppc64le for both default config (with added livepatch dependencies) and a larger SLE-15-ish config. Summary of changes in this minimal version v2 - rebase for v6.9 - cleaned-up SoB chains (suggested by pmladek) - klp-convert: remove the symbol map auto-resolving solution - klp-convert: add macro for flagging variables inside a LP src to be resolved by this tool - klp-convert: code simplification - selftests: add selftest livepatching function using an external symbol Previous versions - RFC: https://lore.kernel.org/r/cover.1477578530.git.jpoim...@redhat.com/ v2: https://lore.kernel.org/r/f52d29f7-7d1b-ad3d-050b-a9fa8878f...@redhat.com/ v3: https://lore.kernel.org/r/20190410155058.9437-1-joe.lawre...@redhat.com/ v4: https://lore.kernel.org/r/20190509143859.9050-1-joe.lawre...@redhat.com/ v5: (not posted) https://github.com/joe-lawrence/klp-convert-tree/tree/klp-convert-v5-devel v6: https://lore.kernel.org/r/20220216163940.228309-1-joe.lawre...@redhat.com/ v7: https://lore.kernel.org/r/20230306140824.3858543-1-joe.lawre...@redhat.com/ v1 minimal: https://lore.kernel.org/r/20231106162513.17556-1-lhru...@suse.cz/
[PATCH] x86/paravirt: Disable virt spinlock when CONFIG_PARAVIRT_SPINLOCKS disabled
Performance drop is reported when running encode/decode workload and
BenchSEE cache sub-workload.
Bisect points to commit ce0a1b608bfc ("x86/paravirt: Silence unused
native_pv_lock_init() function warning"). When CONFIG_PARAVIRT_SPINLOCKS
is disabled the virt_spin_lock_key is set to true on bare-metal.
The qspinlock degenerates to test-and-set spinlock, which decrease the
performance on bare-metal.
Fix this by disabling virt_spin_lock_key if CONFIG_PARAVIRT_SPINLOCKS
is not set, or it is on bare-metal.
Fixes: ce0a1b608bfc ("x86/paravirt: Silence unused native_pv_lock_init()
function warning")
Suggested-by: Qiuxu Zhuo
Reported-by: Prem Nath Dey
Reported-by: Xiaoping Zhou
Signed-off-by: Chen Yu
---
arch/x86/kernel/paravirt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 5358d43886ad..ee51c0949ed8 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -55,7 +55,7 @@ DEFINE_STATIC_KEY_TRUE(virt_spin_lock_key);
void __init native_pv_lock_init(void)
{
- if (IS_ENABLED(CONFIG_PARAVIRT_SPINLOCKS) &&
+ if (!IS_ENABLED(CONFIG_PARAVIRT_SPINLOCKS) ||
!boot_cpu_has(X86_FEATURE_HYPERVISOR))
static_branch_disable(_spin_lock_key);
}
--
2.25.1
RE: [PATCH] vhost: use pr_err for vq_err
> Subject: Re: [PATCH] vhost: use pr_err for vq_err
>
> On Thu, May 16, 2024 at 03:46:29PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan
> >
> > Use pr_err to print out error message without enabling DEBUG. This
> > could make people catch error easier.
> >
> > Signed-off-by: Peng Fan
>
> This isn't appropriate: pr_err must not be triggerable by userspace. If you
> are
> debugging userspace, use a debugging kernel, it's that simple.
I see, then drop this patch.
Thanks,
Peng.
>
>
> > ---
> > drivers/vhost/vhost.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index
> > bb75a292d50c..0bff436d1ce9 100644
> > --- a/drivers/vhost/vhost.h
> > +++ b/drivers/vhost/vhost.h
> > @@ -248,7 +248,7 @@ void vhost_iotlb_map_free(struct vhost_iotlb *iotlb,
> > struct vhost_iotlb_map *map);
> >
> > #define vq_err(vq, fmt, ...) do { \
> > - pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
> > + pr_err(pr_fmt(fmt), ##__VA_ARGS__); \
> > if ((vq)->error_ctx) \
> > eventfd_signal((vq)->error_ctx);\
> > } while (0)
> > --
> > 2.37.1
Re: [PATCH] vhost: use pr_err for vq_err
On Thu, May 16, 2024 at 03:46:29PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan
>
> Use pr_err to print out error message without enabling DEBUG. This could
> make people catch error easier.
>
> Signed-off-by: Peng Fan
This isn't appropriate: pr_err must not be triggerable
by userspace. If you are debugging userspace, use a debugging
kernel, it's that simple.
> ---
> drivers/vhost/vhost.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index bb75a292d50c..0bff436d1ce9 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -248,7 +248,7 @@ void vhost_iotlb_map_free(struct vhost_iotlb *iotlb,
> struct vhost_iotlb_map *map);
>
> #define vq_err(vq, fmt, ...) do { \
> - pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
> + pr_err(pr_fmt(fmt), ##__VA_ARGS__); \
> if ((vq)->error_ctx) \
> eventfd_signal((vq)->error_ctx);\
> } while (0)
> --
> 2.37.1
Re: [REGRESSION][v6.8-rc1] virtio-pci: Introduce admin virtqueue
On Thu, May 16, 2024 at 5:46 PM Catherine Redfield
wrote:
>
> Feng,
>
> Thank you for providing your debugging steps; I used them on a gce image
> locally and was not able to replicate the issue. I also attempted to
> replicate in qemu/virsh using qemu-guest-agent to enable the S3 suspend
> state, also without success (that is S3 suspend state worked without any
> problems). I have brought this back to the cloud for further debugging of
> their config and guest agent to try and determine what the issue is.
>
> Thank you very much for all your help on this issue and time looking into it!
> Catherine
Does this fix the issue? I guess the reason is that GCE is using legacy virtio.
https://lore.kernel.org/kvm/cacgkmeth_9baewekq862ygzwuozwg96z3g6oyqhzycj2jpu...@mail.gmail.com/T/
Thanks
>
> On Thu, May 9, 2024 at 5:03 AM Feng Liu wrote:
>>
>>
>> On 2024-05-08 a.m.7:18, Catherine Redfield wrote:
>> > *External email: Use caution opening links or attachments*
>> >
>> >
>> > On a VM with the GCP kernel (where we first identified the problem), I see:
>> >
>> > 1. The full kernel log from `journalctl --system > kernlog` attached.
>> > The specific suspend section is here:
>> >
>> > May 08 11:08:42 kernel-test-202405080702.c.ubuntu-catred.internal
>> > systemd[1]: Reached target sleep.target - Sleep.
>> > May 08 11:08:42 kernel-test-202405080702.c.ubuntu-catred.internal
>> > systemd[1]: Starting systemd-suspend.service - System Suspend...
>> > May 08 11:08:42 kernel-test-202405080702.c.ubuntu-catred.internal
>> > systemd-sleep[1413]: Performing sleep operation 'suspend'...
>> > May 08 11:08:42 kernel-test-202405080702.c.ubuntu-catred.internal
>> > kernel: PM: suspend entry (deep)
>> > May 08 11:08:42 kernel-test-202405080702.c.ubuntu-catred.internal
>> > kernel: Filesystems sync: 0.008 seconds
>> > May 08 11:08:42 kernel-test-202405080702.c.ubuntu-catred.internal
>> > kernel: Freezing user space processes
>> > May 08 11:08:42 kernel-test-202405080702.c.ubuntu-catred.internal
>> > kernel: Freezing user space processes completed (elapsed 0.001 seconds)
>> > May 08 11:08:42 kernel-test-202405080702.c.ubuntu-catred.internal
>> > kernel: OOM killer disabled.
>> > May 08 11:08:42 kernel-test-202405080702.c.ubuntu-catred.internal
>> > kernel: Freezing remaining freezable tasks
>> > May 08 11:08:42 kernel-test-202405080702.c.ubuntu-catred.internal
>> > kernel: Freezing remaining freezable tasks completed (elapsed 0.000
>> > seconds)
>> > May 08 11:08:42 kernel-test-202405080702.c.ubuntu-catred.internal
>> > kernel: printk: Suspending console(s) (use no_console_suspend to debug)
>> > May 08 11:08:42 kernel-test-202405080702.c.ubuntu-catred.internal
>> > kernel: port 00:03:0.0: PM: dpm_run_callback():
>> > pm_runtime_force_suspend+0x0/0x130 returns -16
>> > May 08 11:08:42 kernel-test-202405080702.c.ubuntu-catred.internal
>> > kernel: port 00:03:0.0: PM: failed to suspend: error -16
>>
>> Thanks Joesph and Catherine's help.
>>
>> Hi,
>>
>> I have alreay synced up with Cananical guys offline about this issue.
>>
>> I can run "suspend/resume" sucessfully on my local server and VM.
>> And "PM: failed to suspend: error -16" looks like not cause by my
>> previous virtio patch ( fd27ef6b44be ("virtio-pci: Introduce admin
>> virtqueue")) which only modified "virtio_device_freeze" about "suspend"
>> action.
>>
>> So I have provide the my steps and debug patch to Joesph and Catherine.
>> I will also sync up the information here, as follow:
>>
>> I have read the qemu code and find a way to trigger "suspend/resume" on
>> my setup, and add some debug message in the latest kerenel
>>
>> My setps are:
>> 1. QEMU cmdline add following
>>
>> -global PIIX4_PM.disable_s3=0 \
>> -global PIIX4_PM.disable_s4=1 \
>>
>> -netdev type=tap,ifname=tap0,id=hostnet0,script=no,downscript=no \
>> -device
>> virtio-net-pci,netdev=hostnet0,id=net0,mac=$SSH_MAC,bus=pci.0,addr=0x3 \
>> ..
>>
>> 2. In the VM, run "systemctl suspend" to PM suspend the VM into memory
>> 3. In qemu hmp shell, run "system_wakeup" to resume the VM again
>>
>> My VM configuration:
>> NIC: 1 virtio nic emulated by QEMU
>> OS: Ubuntu 22.04.4 LTS
>> kernel: latest kernel, 6.9-rc7: ee5b455b0ada (kernel2/net-next-virito,
>> kernel2/master, master) Merge tag 'slab-for-6.9-rc7-fixes' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab)
>>
>>
>> I add some debug message on the latest kernel, and do above steps to
>> trigger "suspen/resume". Everything of VM is OK, VM could suspend/resume
>> successfully.
>> Follwing is the kernel log:
>>
>>
>> May 6 15:59:52 feliu-vm kernel: [ 43.446737] PM: suspend entry (deep)
>> May 6 16:00:04 feliu-vm kernel: [ 43.467640] Filesystems sync: 0.020
>> seconds
>> May 6 16:00:04 feliu-vm kernel: [ 43.467923] Freezing user space
>> processes
>> May 6 16:00:04 feliu-vm kernel: [ 43.470294] Freezing user space
>>
Re: [RFC PATCH v3 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup
On Thu May 16, 2024 at 1:29 AM EEST, Haitao Huang wrote:
> On Wed, 15 May 2024 16:55:59 -0500, Haitao Huang
> wrote:
>
> > On Wed, 15 May 2024 01:55:21 -0500, Bojun Zhu
> > wrote:
> >
> >> EDMM's ioctl()s support batch operations, which may be
> >> time-consuming. Try to explicitly give up the CPU as the prefix
> >> operation at the every begin of "for loop" in
> >> sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
> >> to give other tasks a chance to run, and avoid softlockup warning.
> >>
> >> Additionally perform pending signals check as the prefix operation,
> >> and introduce sgx_check_signal_and_resched(),
> >> which wraps all the checks.
> >>
> >> The following has been observed on Linux v6.9-rc5 with kernel
> >> preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
> >> is requested to restrict page permissions of a large number of EPC
> >> pages.
> >>
> >> [ cut here ]
> >> watchdog: BUG: soft lockup - CPU#45 stuck for 22s!
> >> ...
> >> RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
> >> ...
> >> Call Trace:
> >> sgx_ioctl
> >> __x64_sys_ioctl
> >> x64_sys_call
> >> do_syscall_64
> >> entry_SYSCALL_64_after_hwframe
> >> [ end trace ]
> >>
> >> Signed-off-by: Bojun Zhu
> >> ---
> >> arch/x86/kernel/cpu/sgx/ioctl.c | 40 +++--
> >> 1 file changed, 28 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c
> >> b/arch/x86/kernel/cpu/sgx/ioctl.c
> >> index b65ab214bdf5..6199f483143e 100644
> >> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> >> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> >> @@ -365,6 +365,20 @@ static int sgx_validate_offset_length(struct
> >> sgx_encl *encl,
> >>return 0;
> >> }
> >> +/*
> >> + * Check signals and invoke scheduler. Return true for a pending
> >> signal.
> >> + */
> >> +static bool sgx_check_signal_and_resched(void)
> >> +{
> >> + if (signal_pending(current))
> >> + return true;
> >> +
> >> + if (need_resched())
> >> + cond_resched();
> >> +
> >> + return false;
> >> +}
> >> +
> >> /**
> >> * sgx_ioc_enclave_add_pages() - The handler for
> >> %SGX_IOC_ENCLAVE_ADD_PAGES
> >> * @encl: an enclave pointer
> >> @@ -409,7 +423,7 @@ static long sgx_ioc_enclave_add_pages(struct
> >> sgx_encl *encl, void __user *arg)
> >>struct sgx_enclave_add_pages add_arg;
> >>struct sgx_secinfo secinfo;
> >>unsigned long c;
> >> - int ret;
> >> + int ret = -ERESTARTSYS;
> >>if (!test_bit(SGX_ENCL_CREATED, >flags) ||
> >>test_bit(SGX_ENCL_INITIALIZED, >flags))
> >> @@ -432,15 +446,8 @@ static long sgx_ioc_enclave_add_pages(struct
> >> sgx_encl *encl, void __user *arg)
> >>return -EINVAL;
> >>for (c = 0 ; c < add_arg.length; c += PAGE_SIZE) {
> >> - if (signal_pending(current)) {
> >> - if (!c)
> >> - ret = -ERESTARTSYS;
> >> -
> >> + if (sgx_check_signal_and_resched())
> >>break;
> >> - }
> >
> > ERESTARTSYS is only appropriate if we have not EADDed any pages yet.
> > If we got interrupted in the middle, we should return 0. User space
> > would check the 'count' returned and decide to recall this ioctl() with
> > 'offset' reset to the next page, and adjust length.
>
> NVM, I misread it. ret will be changed to zero in subsequent iteration.
>
> Reviewed-by: Haitao Huang
Duh, and I responded too quickly. OK, I revisited the original
patch and yes ret gets reseted. Ignore my previous response ;-)
My tags still hold, sorry.
BR, Jarkko
Re: [RFC PATCH v3 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup
On Thu May 16, 2024 at 12:55 AM EEST, Haitao Huang wrote:
> On Wed, 15 May 2024 01:55:21 -0500, Bojun Zhu
> wrote:
>
> > EDMM's ioctl()s support batch operations, which may be
> > time-consuming. Try to explicitly give up the CPU as the prefix
> > operation at the every begin of "for loop" in
> > sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
> > to give other tasks a chance to run, and avoid softlockup warning.
> >
> > Additionally perform pending signals check as the prefix operation,
> > and introduce sgx_check_signal_and_resched(),
> > which wraps all the checks.
> >
> > The following has been observed on Linux v6.9-rc5 with kernel
> > preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
> > is requested to restrict page permissions of a large number of EPC pages.
> >
> > [ cut here ]
> > watchdog: BUG: soft lockup - CPU#45 stuck for 22s!
> > ...
> > RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
> > ...
> > Call Trace:
> > sgx_ioctl
> > __x64_sys_ioctl
> > x64_sys_call
> > do_syscall_64
> > entry_SYSCALL_64_after_hwframe
> > [ end trace ]
> >
> > Signed-off-by: Bojun Zhu
> > ---
> > arch/x86/kernel/cpu/sgx/ioctl.c | 40 +++--
> > 1 file changed, 28 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c
> > b/arch/x86/kernel/cpu/sgx/ioctl.c
> > index b65ab214bdf5..6199f483143e 100644
> > --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> > +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> > @@ -365,6 +365,20 @@ static int sgx_validate_offset_length(struct
> > sgx_encl *encl,
> > return 0;
> > }
> > +/*
> > + * Check signals and invoke scheduler. Return true for a pending signal.
> > + */
> > +static bool sgx_check_signal_and_resched(void)
> > +{
> > + if (signal_pending(current))
> > + return true;
> > +
> > + if (need_resched())
> > + cond_resched();
> > +
> > + return false;
> > +}
> > +
> > /**
> > * sgx_ioc_enclave_add_pages() - The handler for
> > %SGX_IOC_ENCLAVE_ADD_PAGES
> > * @encl: an enclave pointer
> > @@ -409,7 +423,7 @@ static long sgx_ioc_enclave_add_pages(struct
> > sgx_encl *encl, void __user *arg)
> > struct sgx_enclave_add_pages add_arg;
> > struct sgx_secinfo secinfo;
> > unsigned long c;
> > - int ret;
> > + int ret = -ERESTARTSYS;
> > if (!test_bit(SGX_ENCL_CREATED, >flags) ||
> > test_bit(SGX_ENCL_INITIALIZED, >flags))
> > @@ -432,15 +446,8 @@ static long sgx_ioc_enclave_add_pages(struct
> > sgx_encl *encl, void __user *arg)
> > return -EINVAL;
> > for (c = 0 ; c < add_arg.length; c += PAGE_SIZE) {
> > - if (signal_pending(current)) {
> > - if (!c)
> > - ret = -ERESTARTSYS;
> > -
> > + if (sgx_check_signal_and_resched())
> > break;
> > - }
>
> ERESTARTSYS is only appropriate if we have not EADDed any pages yet.
> If we got interrupted in the middle, we should return 0. User space would
> check the 'count' returned and decide to recall this ioctl() with
> 'offset' reset to the next page, and adjust length.
Good catch! Thanks Haitao for the remark.
BR, Jarkko
Re: [PATCH] vhost/vsock: always initialize seqpacket_allow
On Wed, May 15, 2024 at 11:05:43AM GMT, Michael S. Tsirkin wrote:
There are two issues around seqpacket_allow:
1. seqpacket_allow is not initialized when socket is
created. Thus if features are never set, it will be
read uninitialized.
2. if VIRTIO_VSOCK_F_SEQPACKET is set and then cleared,
then seqpacket_allow will not be cleared appropriately
(existing apps I know about don't usually do this but
it's legal and there's no way to be sure no one relies
on this).
To fix:
- initialize seqpacket_allow after allocation
- set it unconditionally in set_features
Reported-by: syzbot+6c21aeb59d0e82eb2...@syzkaller.appspotmail.com
Reported-by: Jeongjun Park
Fixes: ced7b713711f ("vhost/vsock: support SEQPACKET for transport").
Cc: Arseny Krasnov
Cc: David S. Miller
Cc: Stefan Hajnoczi
Signed-off-by: Michael S. Tsirkin
Acked-by: Arseniy Krasnov
Tested-by: Arseniy Krasnov
---
Reposting now it's been tested.
drivers/vhost/vsock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Thanks for fixing this issue!
Reviewed-by: Stefano Garzarella
[PATCH] vhost: use pr_err for vq_err
From: Peng Fan
Use pr_err to print out error message without enabling DEBUG. This could
make people catch error easier.
Signed-off-by: Peng Fan
---
drivers/vhost/vhost.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index bb75a292d50c..0bff436d1ce9 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -248,7 +248,7 @@ void vhost_iotlb_map_free(struct vhost_iotlb *iotlb,
struct vhost_iotlb_map *map);
#define vq_err(vq, fmt, ...) do { \
- pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
+ pr_err(pr_fmt(fmt), ##__VA_ARGS__); \
if ((vq)->error_ctx) \
eventfd_signal((vq)->error_ctx);\
} while (0)
--
2.37.1
