Re: [PATCH 0/2] kexec: Remove unnecessary arch hook
On 12/15/22 at 12:23pm, Bjorn Helgaas wrote: > From: Bjorn Helgaas > > There are no arch-specific things in arch_kexec_kernel_image_load(), so > remove it and just use the generic version. This patchset looks good to me, thx. Acked-by: Baoquan He Since it cleans up the last arch specific version of arch_kexec_kernel_image_load in x86, maybe this patchset can go into x86 branch? Thanks Baoquan ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 0/2] kexec: Remove unnecessary arch hook
On 12/17/22 at 09:58am, Bjorn Helgaas wrote: > On Sat, Dec 17, 2022 at 05:48:51PM +0800, Baoquan He wrote: > > On 12/15/22 at 12:23pm, Bjorn Helgaas wrote: > > > From: Bjorn Helgaas > > > > > > There are no arch-specific things in arch_kexec_kernel_image_load(), so > > > remove it and just use the generic version. > > > > I ever posted below patch to do the same thing, Andrew only picked the > > memory leak fixing patch. > > > > [PATCH v2 2/2] kexec_file: clean up arch_kexec_kernel_image_load > > https://lore.kernel.org/all/20220223113225.63106-3-...@redhat.com/T/#u > > Indeed! Sorry, I wasn't aware of your previous work. If you repost > it, cc me and I'll be glad to help review it. When tried to rebase my old patch to the latest kernel, I realized this patchset is what I can end up with. I would like to ack this patchset to make it merged. Thanks a lot for the effort. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 3/3] kexec: Introduce parameters load_limit_reboot and load_limit_panic
On Wed, 21 Dec 2022 02:22:57 +0100 Ricardo Ribalda wrote: > > > + kexec_core.load_limit_reboot= > > > + kexec_core.load_limit_panic= > > > + [KNL] > > > + This parameter specifies a limit to the number of > > > times > > > + a kexec kernel can be loaded. > > > + Format: > > > + -1 = Unlimited. > > > + int = Number of times kexec can be called. > > > + > > > + During runtime, this parameter can be modified with > > > a > > > > > + value smaller than the current one (but not -1). > > > > Perhaps state: > > smaller positive value than the current one or if > > current is currently -1. > > I find it a bit complicated.. > What about: > > During runtime this parameter can be modified with a more restrictive value Sure. That's better than the original. > > > +module_param_cb(load_limit_reboot, _limit_ops, _limit_reboot, > > > 0644); > > > +MODULE_PARM_DESC(load_limit_reboot, "Maximum attempts to load a kexec > > > reboot kernel"); > > > + > > > +module_param_cb(load_limit_panic, _limit_ops, _limit_panic, > > > 0644); > > > +MODULE_PARM_DESC(load_limit_reboot, "Maximum attempts to load a kexec > > > panic kernel"); > > > > Wait, why the module params if this can not be a module? > > > > The kernel/kexec.c is decided via CONFIG_KEXEC_CORE which is bool. Either > > builtin or not at all. No module selection possible. > > > > For kernel parameters, we should just use __setup(), right? > > Isn't __setup() only kernel parameter and then it cannot be updated on > runtime? Yes, but then we use sysctl. > > What about using late_param_cb? and remove MODULE_PARAM_DESC ? > > I think this is how these parameters work > > $ ls /sys/module/kernel/parameters/ > consoleblank crash_kexec_post_notifiers ignore_rlimit_data > initcall_debug module_blacklist panic panic_on_warn panic_print > pause_on_oops Well, I think those are more leftovers and not something we want to add to. I believe sysctl is the better option, and a more common one: $ ls /proc/sys/kernel/ acct perf_event_max_contexts_per_stack acpi_video_flags perf_event_max_sample_rate auto_msgmniperf_event_max_stack bootloader_typeperf_event_mlock_kb bootloader_version perf_event_paranoid bpf_stats_enabled pid_max cad_pidpoweroff_cmd cap_last_cap print-fatal-signals core_pattern printk core_pipe_limitprintk_delay core_uses_pid printk_devkmsg ctrl-alt-del printk_ratelimit dmesg_restrict printk_ratelimit_burst domainname pty firmware_configrandom ftrace_dump_on_oopsrandomize_va_space ftrace_enabled real-root-dev hardlockup_all_cpu_backtrace sched_autogroup_enabled hardlockup_panic sched_child_runs_first hostname sched_deadline_period_max_us hotplugsched_deadline_period_min_us hung_task_all_cpu_backtracesched_energy_aware hung_task_check_count sched_rr_timeslice_ms hung_task_check_interval_secs sched_rt_period_us hung_task_panicsched_rt_runtime_us hung_task_timeout_secs sched_util_clamp_max hung_task_warnings sched_util_clamp_min io_delay_type sched_util_clamp_min_rt_default kexec_load_disabledseccomp keys sem kptr_restrict sem_next_id max_lock_depth shmall max_rcu_stall_to_panic shmmax modprobe shmmni modules_disabled shm_next_id msgmax shm_rmid_forced msgmnb softlockup_all_cpu_backtrace msgmni softlockup_panic msg_next_idsoft_watchdog ngroups_maxstack_tracer_enabled nmi_watchdog sysctl_writes_strict ns_last_pidsysrq numa_balancing tainted oops_all_cpu_backtrace task_delayacct osrelease threads-max ostype timer_migration overflowgidtraceoff_on_warning overflowuidtracepoint_printk panic unknown_nmi_panic panic_on_io_nmiunprivileged_bpf_disabled panic_on_oops usermodehelper panic_on_rcu_stall version panic_on_unrecovered_nmi watchdog panic_on_warn watchdog_cpumask panic_printwatchdog_thresh perf_cpu_time_max_percent yama > I could pass the flags and then check for flags & (KEXEC_ON_CRASH | > KEXEC_FILE_ON_CRASH)... but not
[PATCH makedumpfile] Fix incorrect exclusion of slab pages on Linux 6.2-rc1
From: Kazuhito Hagio
* Required for kernel 6.2
Kernel commit 130d4df57390 ("mm/sl[au]b: rearrange struct slab fields to
allow larger rcu_head"), which is contained in Linux 6.2-rc1 and later,
made the offset of slab.slabs equal to page.mapping's one. As a result,
"makedumpfile -d 8", which should exclude user data, excludes some slab
pages incorrectly because isAnon() returns true when slab.slabs is an
odd number. With such dumpfiles, crash can fail to start session with
an error like this:
# crash vmlinux dumpfile
...
crash: page excluded: kernel virtual address: 8fa047ac2fe8 type: "xa_node
shift"
Make isAnon() check that the page is not slab to fix this.
Signed-off-by: Kazuhito Hagio
---
makedumpfile.c | 6 +++---
makedumpfile.h | 9 +++--
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/makedumpfile.c b/makedumpfile.c
index ff821ebd3eb0..f40368364cf3 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -6502,7 +6502,7 @@ __exclude_unnecessary_pages(unsigned long mem_map,
*/
else if ((info->dump_level & DL_EXCLUDE_CACHE)
&& is_cache_page(flags)
- && !isPrivate(flags) && !isAnon(mapping)) {
+ && !isPrivate(flags) && !isAnon(mapping, flags)) {
pfn_counter = _cache;
}
/*
@@ -6510,7 +6510,7 @@ __exclude_unnecessary_pages(unsigned long mem_map,
*/
else if ((info->dump_level & DL_EXCLUDE_CACHE_PRI)
&& is_cache_page(flags)
- && !isAnon(mapping)) {
+ && !isAnon(mapping, flags)) {
if (isPrivate(flags))
pfn_counter = _cache_private;
else
@@ -6522,7 +6522,7 @@ __exclude_unnecessary_pages(unsigned long mem_map,
* - hugetlbfs pages
*/
else if ((info->dump_level & DL_EXCLUDE_USER_DATA)
-&& (isAnon(mapping) || isHugetlb(compound_dtor))) {
+&& (isAnon(mapping, flags) ||
isHugetlb(compound_dtor))) {
pfn_counter = _user;
}
/*
diff --git a/makedumpfile.h b/makedumpfile.h
index 70a1a91d66be..21dec7d1145c 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -161,12 +161,9 @@ test_bit(int nr, unsigned long addr)
#define isSwapBacked(flags)test_bit(NUMBER(PG_swapbacked), flags)
#define isHWPOISON(flags) (test_bit(NUMBER(PG_hwpoison), flags) \
&& (NUMBER(PG_hwpoison) != NOT_FOUND_NUMBER))
-
-static inline int
-isAnon(unsigned long mapping)
-{
- return ((unsigned long)mapping & PAGE_MAPPING_ANON) != 0;
-}
+#define isSlab(flags) test_bit(NUMBER(PG_slab), flags)
+#define isAnon(mapping, flags) (((unsigned long)mapping & PAGE_MAPPING_ANON)
!= 0 \
+ && !isSlab(flags))
#define PTOB(X)(((unsigned long long)(X)) <<
PAGESHIFT())
#define BTOP(X)(((unsigned long long)(X)) >>
PAGESHIFT())
--
2.31.1
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 3/3] kexec: Introduce parameters load_limit_reboot and load_limit_panic
Hi Steven
Thanks for your review!!! Will send a new version.
After giving it a thought... you are right :). setting the current
value should return -EINVAL. We should only return OK if we actually
do something.
On Wed, 21 Dec 2022 at 01:22, Steven Rostedt wrote:
>
> On Tue, 20 Dec 2022 23:05:45 +0100
> Ricardo Ribalda wrote:
>
> I hate to be the grammar police, but..
>
> > Add two parameter to specify how many times a kexec kernel can be loaded.
>
>"parameters"
>
> >
> > The sysadmin can set different limits for kexec panic and kexec reboot
> > kernels.
> >
> > The value can be modified at runtime via sysfs, but only with a value
> > smaller than the current one (except -1).
> >
> > Signed-off-by: Ricardo Ribalda
> > ---
> > Documentation/admin-guide/kernel-parameters.txt | 14
> > include/linux/kexec.h | 2 +-
> > kernel/kexec.c | 2 +-
> > kernel/kexec_core.c | 91
> > -
> > kernel/kexec_file.c | 2 +-
> > 5 files changed, 106 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt
> > b/Documentation/admin-guide/kernel-parameters.txt
> > index 42af9ca0127e..2b37d6a20747 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -2374,6 +2374,20 @@
> > for Movable pages. "nn[KMGTPE]", "nn%", and "mirror"
> > are exclusive, so you cannot specify multiple forms.
> >
> > + kexec_core.load_limit_reboot=
> > + kexec_core.load_limit_panic=
> > + [KNL]
> > + This parameter specifies a limit to the number of
> > times
> > + a kexec kernel can be loaded.
> > + Format:
> > + -1 = Unlimited.
> > + int = Number of times kexec can be called.
> > +
> > + During runtime, this parameter can be modified with a
>
> > + value smaller than the current one (but not -1).
>
> Perhaps state:
> smaller positive value than the current one or if
> current is currently -1.
I find it a bit complicated..
What about:
During runtime this parameter can be modified with a more restrictive value
>
> > +
> > + Default: -1
> > +
> > kgdbdbgp= [KGDB,HW] kgdb over EHCI usb debug port.
> > Format: [,poll interval]
> > The controller # is the number of the ehci usb debug
> > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > index 182e0c11b87b..5daf9990d5b8 100644
> > --- a/include/linux/kexec.h
> > +++ b/include/linux/kexec.h
> > @@ -407,7 +407,7 @@ extern int kimage_crash_copy_vmcoreinfo(struct kimage
> > *image);
> > extern struct kimage *kexec_image;
> > extern struct kimage *kexec_crash_image;
> >
> > -bool kexec_load_permitted(void);
> > +bool kexec_load_permitted(bool crash_image);
> >
> > #ifndef kexec_flush_icache_page
> > #define kexec_flush_icache_page(page)
> > diff --git a/kernel/kexec.c b/kernel/kexec.c
> > index ce1bca874a8d..7aefd134e319 100644
> > --- a/kernel/kexec.c
> > +++ b/kernel/kexec.c
> > @@ -193,7 +193,7 @@ static inline int kexec_load_check(unsigned long
> > nr_segments,
> > int result;
> >
> > /* We only trust the superuser with rebooting the system. */
> > - if (!kexec_load_permitted())
> > + if (!kexec_load_permitted(flags & KEXEC_ON_CRASH))
>
> Note, here we have KEXEC_ON_CRASH (see bottom).
>
> > return -EPERM;
> >
> > /* Permit LSMs and IMA to fail the kexec */
> > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> > index a1efc70f4158..adf71f2be3ff 100644
> > --- a/kernel/kexec_core.c
> > +++ b/kernel/kexec_core.c
> > @@ -952,13 +952,100 @@ static int __init kexec_core_sysctl_init(void)
> > late_initcall(kexec_core_sysctl_init);
> > #endif
> >
> > -bool kexec_load_permitted(void)
> > +struct kexec_load_limit {
> > + /* Mutex protects the limit count. */
> > + struct mutex mutex;
> > + int limit;
> > +};
> > +
> > +struct kexec_load_limit load_limit_reboot = {
>
> Perhaps make the above static?
>
> > + .mutex = __MUTEX_INITIALIZER(load_limit_reboot.mutex),
> > + .limit = -1,
> > +};
> > +
> > +struct kexec_load_limit load_limit_panic = {
>
> static?
>
> > + .mutex = __MUTEX_INITIALIZER(load_limit_panic.mutex),
> > + .limit = -1,
> > +};
> > +
> > +static int param_get_limit(char *buffer, const struct kernel_param *kp)
> > {
> > + int ret;
> > + struct kexec_load_limit *limit = kp->arg;
>
> Looks better if "int ret;" is after the "limit".
>
> > +
> > + mutex_lock(>mutex);
> > + ret = scnprintf(buffer, PAGE_SIZE, "%i\n", limit->limit);
>
> The above string can be at most "-2147483648\n\0"
>
>
Re: [PATCH v3 3/3] kexec: Introduce parameters load_limit_reboot and load_limit_panic
On Tue, 20 Dec 2022 23:05:45 +0100
Ricardo Ribalda wrote:
I hate to be the grammar police, but..
> Add two parameter to specify how many times a kexec kernel can be loaded.
"parameters"
>
> The sysadmin can set different limits for kexec panic and kexec reboot
> kernels.
>
> The value can be modified at runtime via sysfs, but only with a value
> smaller than the current one (except -1).
>
> Signed-off-by: Ricardo Ribalda
> ---
> Documentation/admin-guide/kernel-parameters.txt | 14
> include/linux/kexec.h | 2 +-
> kernel/kexec.c | 2 +-
> kernel/kexec_core.c | 91
> -
> kernel/kexec_file.c | 2 +-
> 5 files changed, 106 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt
> b/Documentation/admin-guide/kernel-parameters.txt
> index 42af9ca0127e..2b37d6a20747 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2374,6 +2374,20 @@
> for Movable pages. "nn[KMGTPE]", "nn%", and "mirror"
> are exclusive, so you cannot specify multiple forms.
>
> + kexec_core.load_limit_reboot=
> + kexec_core.load_limit_panic=
> + [KNL]
> + This parameter specifies a limit to the number of times
> + a kexec kernel can be loaded.
> + Format:
> + -1 = Unlimited.
> + int = Number of times kexec can be called.
> +
> + During runtime, this parameter can be modified with a
> + value smaller than the current one (but not -1).
Perhaps state:
smaller positive value than the current one or if
current is currently -1.
> +
> + Default: -1
> +
> kgdbdbgp= [KGDB,HW] kgdb over EHCI usb debug port.
> Format: [,poll interval]
> The controller # is the number of the ehci usb debug
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 182e0c11b87b..5daf9990d5b8 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -407,7 +407,7 @@ extern int kimage_crash_copy_vmcoreinfo(struct kimage
> *image);
> extern struct kimage *kexec_image;
> extern struct kimage *kexec_crash_image;
>
> -bool kexec_load_permitted(void);
> +bool kexec_load_permitted(bool crash_image);
>
> #ifndef kexec_flush_icache_page
> #define kexec_flush_icache_page(page)
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index ce1bca874a8d..7aefd134e319 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -193,7 +193,7 @@ static inline int kexec_load_check(unsigned long
> nr_segments,
> int result;
>
> /* We only trust the superuser with rebooting the system. */
> - if (!kexec_load_permitted())
> + if (!kexec_load_permitted(flags & KEXEC_ON_CRASH))
Note, here we have KEXEC_ON_CRASH (see bottom).
> return -EPERM;
>
> /* Permit LSMs and IMA to fail the kexec */
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index a1efc70f4158..adf71f2be3ff 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -952,13 +952,100 @@ static int __init kexec_core_sysctl_init(void)
> late_initcall(kexec_core_sysctl_init);
> #endif
>
> -bool kexec_load_permitted(void)
> +struct kexec_load_limit {
> + /* Mutex protects the limit count. */
> + struct mutex mutex;
> + int limit;
> +};
> +
> +struct kexec_load_limit load_limit_reboot = {
Perhaps make the above static?
> + .mutex = __MUTEX_INITIALIZER(load_limit_reboot.mutex),
> + .limit = -1,
> +};
> +
> +struct kexec_load_limit load_limit_panic = {
static?
> + .mutex = __MUTEX_INITIALIZER(load_limit_panic.mutex),
> + .limit = -1,
> +};
> +
> +static int param_get_limit(char *buffer, const struct kernel_param *kp)
> {
> + int ret;
> + struct kexec_load_limit *limit = kp->arg;
Looks better if "int ret;" is after the "limit".
> +
> + mutex_lock(>mutex);
> + ret = scnprintf(buffer, PAGE_SIZE, "%i\n", limit->limit);
The above string can be at most "-2147483648\n\0"
Which is 13 characters. Why use PAGE_SIZE. Or scnprintf(), and not just
state:
/* buffer is PAGE_SIZE, much larger than what %i can be */
ret = sprintf(buffer, "%i\n", limit->limit);
> + mutex_unlock(>mutex);
> +
> + return ret;
> +}
> +
> +static int param_set_limit(const char *buffer, const struct kernel_param *kp)
> +{
> + int ret;
> + struct kexec_load_limit *limit = kp->arg;
> + int new_val;
> +
> + ret = kstrtoint(buffer, 0, _val);
> + if (ret)
> + return ret;
> +
> + new_val = max(-1, new_val);
I wonder if anything less than -1 should be invalid.
> +
> +
[PATCH v3 2/3] kexec: Factor out kexec_load_permitted
Both syscalls (kexec and kexec_file) do the same check, lets factor it
out.
Signed-off-by: Ricardo Ribalda
---
include/linux/kexec.h | 3 ++-
kernel/kexec.c| 2 +-
kernel/kexec_core.c | 11 ++-
kernel/kexec_file.c | 2 +-
4 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 41a686996aaa..182e0c11b87b 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -406,7 +406,8 @@ extern int kimage_crash_copy_vmcoreinfo(struct kimage
*image);
extern struct kimage *kexec_image;
extern struct kimage *kexec_crash_image;
-extern int kexec_load_disabled;
+
+bool kexec_load_permitted(void);
#ifndef kexec_flush_icache_page
#define kexec_flush_icache_page(page)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index cb8e6e6f983c..ce1bca874a8d 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -193,7 +193,7 @@ static inline int kexec_load_check(unsigned long
nr_segments,
int result;
/* We only trust the superuser with rebooting the system. */
- if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
+ if (!kexec_load_permitted())
return -EPERM;
/* Permit LSMs and IMA to fail the kexec */
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index ca2743f9c634..a1efc70f4158 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -928,7 +928,7 @@ int kimage_load_segment(struct kimage *image,
struct kimage *kexec_image;
struct kimage *kexec_crash_image;
-int kexec_load_disabled;
+static int kexec_load_disabled;
#ifdef CONFIG_SYSCTL
static struct ctl_table kexec_core_sysctls[] = {
{
@@ -952,6 +952,15 @@ static int __init kexec_core_sysctl_init(void)
late_initcall(kexec_core_sysctl_init);
#endif
+bool kexec_load_permitted(void)
+{
+ /*
+* Only the superuser can use the kexec syscall and if it has not
+* been disabled.
+*/
+ return capable(CAP_SYS_BOOT) && !kexec_load_disabled;
+}
+
/*
* No panic_cpu check version of crash_kexec(). This function is called
* only when panic_cpu holds the current CPU number; this is the only CPU
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 45637511e0de..29efa43ea951 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -330,7 +330,7 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int,
initrd_fd,
struct kimage **dest_image, *image;
/* We only trust the superuser with rebooting the system. */
- if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
+ if (!kexec_load_permitted())
return -EPERM;
/* Make sure we have a legal set of flags */
--
2.39.0.314.g84b9a713c41-goog-b4-0.11.0-dev-696ae
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v3 1/3] Documentation: sysctl: Correct kexec_load_disabled
kexec_load_disabled affects both ``kexec_load`` and ``kexec_file_load`` syscalls. Make it explicit. Signed-off-by: Ricardo Ribalda --- Documentation/admin-guide/sysctl/kernel.rst | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst index 98d1b198b2b4..97394bd9d065 100644 --- a/Documentation/admin-guide/sysctl/kernel.rst +++ b/Documentation/admin-guide/sysctl/kernel.rst @@ -450,9 +450,10 @@ this allows system administrators to override the kexec_load_disabled === -A toggle indicating if the ``kexec_load`` syscall has been disabled. -This value defaults to 0 (false: ``kexec_load`` enabled), but can be -set to 1 (true: ``kexec_load`` disabled). +A toggle indicating if the syscalls ``kexec_load`` and +``kexec_file_load`` have been disabled. +This value defaults to 0 (false: ``kexec_*load`` enabled), but can be +set to 1 (true: ``kexec_*load`` disabled). Once true, kexec can no longer be used, and the toggle cannot be set back to false. This allows a kexec image to be loaded before disabling the syscall, -- 2.39.0.314.g84b9a713c41-goog-b4-0.11.0-dev-696ae ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v3 3/3] kexec: Introduce parameters load_limit_reboot and load_limit_panic
Add two parameter to specify how many times a kexec kernel can be loaded.
The sysadmin can set different limits for kexec panic and kexec reboot
kernels.
The value can be modified at runtime via sysfs, but only with a value
smaller than the current one (except -1).
Signed-off-by: Ricardo Ribalda
---
Documentation/admin-guide/kernel-parameters.txt | 14
include/linux/kexec.h | 2 +-
kernel/kexec.c | 2 +-
kernel/kexec_core.c | 91 -
kernel/kexec_file.c | 2 +-
5 files changed, 106 insertions(+), 5 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index 42af9ca0127e..2b37d6a20747 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2374,6 +2374,20 @@
for Movable pages. "nn[KMGTPE]", "nn%", and "mirror"
are exclusive, so you cannot specify multiple forms.
+ kexec_core.load_limit_reboot=
+ kexec_core.load_limit_panic=
+ [KNL]
+ This parameter specifies a limit to the number of times
+ a kexec kernel can be loaded.
+ Format:
+ -1 = Unlimited.
+ int = Number of times kexec can be called.
+
+ During runtime, this parameter can be modified with a
+ value smaller than the current one (but not -1).
+
+ Default: -1
+
kgdbdbgp= [KGDB,HW] kgdb over EHCI usb debug port.
Format: [,poll interval]
The controller # is the number of the ehci usb debug
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 182e0c11b87b..5daf9990d5b8 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -407,7 +407,7 @@ extern int kimage_crash_copy_vmcoreinfo(struct kimage
*image);
extern struct kimage *kexec_image;
extern struct kimage *kexec_crash_image;
-bool kexec_load_permitted(void);
+bool kexec_load_permitted(bool crash_image);
#ifndef kexec_flush_icache_page
#define kexec_flush_icache_page(page)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index ce1bca874a8d..7aefd134e319 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -193,7 +193,7 @@ static inline int kexec_load_check(unsigned long
nr_segments,
int result;
/* We only trust the superuser with rebooting the system. */
- if (!kexec_load_permitted())
+ if (!kexec_load_permitted(flags & KEXEC_ON_CRASH))
return -EPERM;
/* Permit LSMs and IMA to fail the kexec */
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index a1efc70f4158..adf71f2be3ff 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -952,13 +952,100 @@ static int __init kexec_core_sysctl_init(void)
late_initcall(kexec_core_sysctl_init);
#endif
-bool kexec_load_permitted(void)
+struct kexec_load_limit {
+ /* Mutex protects the limit count. */
+ struct mutex mutex;
+ int limit;
+};
+
+struct kexec_load_limit load_limit_reboot = {
+ .mutex = __MUTEX_INITIALIZER(load_limit_reboot.mutex),
+ .limit = -1,
+};
+
+struct kexec_load_limit load_limit_panic = {
+ .mutex = __MUTEX_INITIALIZER(load_limit_panic.mutex),
+ .limit = -1,
+};
+
+static int param_get_limit(char *buffer, const struct kernel_param *kp)
{
+ int ret;
+ struct kexec_load_limit *limit = kp->arg;
+
+ mutex_lock(>mutex);
+ ret = scnprintf(buffer, PAGE_SIZE, "%i\n", limit->limit);
+ mutex_unlock(>mutex);
+
+ return ret;
+}
+
+static int param_set_limit(const char *buffer, const struct kernel_param *kp)
+{
+ int ret;
+ struct kexec_load_limit *limit = kp->arg;
+ int new_val;
+
+ ret = kstrtoint(buffer, 0, _val);
+ if (ret)
+ return ret;
+
+ new_val = max(-1, new_val);
+
+ mutex_lock(>mutex);
+
+ if (new_val == -1 && limit->limit != -1) {
+ ret = -EINVAL;
+ goto done;
+ }
+
+ if (limit->limit != -1 && new_val > limit->limit) {
+ ret = -EINVAL;
+ goto done;
+ }
+
+ limit->limit = new_val;
+
+done:
+ mutex_unlock(>mutex);
+
+ return ret;
+}
+
+static const struct kernel_param_ops load_limit_ops = {
+ .get = param_get_limit,
+ .set = param_set_limit,
+};
+
+module_param_cb(load_limit_reboot, _limit_ops, _limit_reboot, 0644);
+MODULE_PARM_DESC(load_limit_reboot, "Maximum attempts to load a kexec reboot
kernel");
+
+module_param_cb(load_limit_panic, _limit_ops, _limit_panic, 0644);
+MODULE_PARM_DESC(load_limit_reboot, "Maximum attempts to load a kexec panic
kernel");
+
+bool kexec_load_permitted(bool crash_image)
+{
+
Re: [PATCH v2 3/3] kexec: Introduce paramters load_limit_reboot and load_limit_panic
Hi Joel
Thanks for looking into this
On Thu, 15 Dec 2022 at 20:16, Joel Fernandes wrote:
>
> Hi Ricardo,
>
> On Thu, Dec 08, 2022 at 05:38:02PM +0100, Ricardo Ribalda wrote:
> > Add two parameter to specify how many times a kexec kernel can be loaded.
> >
> > The sysadmin can set different limits for kexec panic and kexec reboot
> > kernels.
> >
> > The value can be modified at runtime via sysfs, but only with a value
> > smaller than the current one (except -1).
> >
> > Signed-off-by: Ricardo Ribalda
> > ---
> > Documentation/admin-guide/kernel-parameters.txt | 14
> > include/linux/kexec.h | 2 +-
> > kernel/kexec.c | 2 +-
> > kernel/kexec_core.c | 91
> > -
> > kernel/kexec_file.c | 2 +-
> > 5 files changed, 106 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt
> > b/Documentation/admin-guide/kernel-parameters.txt
> > index 42af9ca0127e..2b37d6a20747 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -2374,6 +2374,20 @@
> > for Movable pages. "nn[KMGTPE]", "nn%", and "mirror"
> > are exclusive, so you cannot specify multiple forms.
> >
> > + kexec_core.load_limit_reboot=
> > + kexec_core.load_limit_panic=
> > + [KNL]
> > + This parameter specifies a limit to the number of
> > times
> > + a kexec kernel can be loaded.
> > + Format:
> > + -1 = Unlimited.
> > + int = Number of times kexec can be called.
> > +
> > + During runtime, this parameter can be modified with a
> > + value smaller than the current one (but not -1).
> > +
> > + Default: -1
> > +
> > kgdbdbgp= [KGDB,HW] kgdb over EHCI usb debug port.
> > Format: [,poll interval]
> > The controller # is the number of the ehci usb debug
> > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > index e9e1ab5e8006..3d7d10f7187a 100644
> > --- a/include/linux/kexec.h
> > +++ b/include/linux/kexec.h
> > @@ -407,7 +407,7 @@ extern int kimage_crash_copy_vmcoreinfo(struct kimage
> > *image);
> > extern struct kimage *kexec_image;
> > extern struct kimage *kexec_crash_image;
> >
> > -bool kexec_load_permited(void);
> > +bool kexec_load_permited(bool crash_image);
> >
> > #ifndef kexec_flush_icache_page
> > #define kexec_flush_icache_page(page)
> > diff --git a/kernel/kexec.c b/kernel/kexec.c
> > index d83fc9093aff..2b0856e83fe1 100644
> > --- a/kernel/kexec.c
> > +++ b/kernel/kexec.c
> > @@ -193,7 +193,7 @@ static inline int kexec_load_check(unsigned long
> > nr_segments,
> > int result;
> >
> > /* We only trust the superuser with rebooting the system. */
> > - if (!kexec_load_permited())
> > + if (!kexec_load_permited(flags & KEXEC_ON_CRASH))
>
> nit: permitted.
>
> > return -EPERM;
> >
> > /* Permit LSMs and IMA to fail the kexec */
> > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> > index 18bd90ca9c99..7f9d5288b24b 100644
> > --- a/kernel/kexec_core.c
> > +++ b/kernel/kexec_core.c
> > @@ -952,13 +952,100 @@ static int __init kexec_core_sysctl_init(void)
> > late_initcall(kexec_core_sysctl_init);
> > #endif
> >
> > -bool kexec_load_permited(void)
> > +struct kexec_load_limit {
> > + /* Mutex protects the limit count. */
> > + struct mutex mutex;
> > + int limit;
>
> Can you not just use atomic ops for limit, and get rid of the mutex?
>
> That will simplify the code as well.
I could not find a way to use atomic_t. The operations are not just
counters, but maybe I am missing a better way to do it :)
The current operations:
- permitted():
if (param==-1) {
return -1;
}
if (param =0)
return -1;
param = param -1;
- paramter_set()
new_param = min(-1, new_param);
if (param == -1) {
param = new_param;
return
}
if (new_param == -1) {
return -EINVAL;
}
param = min(new_param, param);
>
> > +};
> > +
> > +struct kexec_load_limit load_limit_reboot = {
> > + .mutex = __MUTEX_INITIALIZER(load_limit_reboot.mutex),
> > + .limit = -1,
> > +};
> > +
> > +struct kexec_load_limit load_limit_panic = {
> > + .mutex = __MUTEX_INITIALIZER(load_limit_panic.mutex),
> > + .limit = -1,
> > +};
> > +
> > +static int param_get_limit(char *buffer, const struct kernel_param *kp)
> > {
> > + int ret;
> > + struct kexec_load_limit *limit = kp->arg;
> > +
> > + mutex_lock(>mutex);
> > + ret = scnprintf(buffer, PAGE_SIZE, "%i\n", limit->limit);
> > + mutex_unlock(>mutex);
> > +
> > + return ret;
> > +}
> > +
> > +static int param_set_limit(const char *buffer, const struct kernel_param
>
[PATCH v3 0/3] kexec: Add new parameter to limit the access to kexec
Add two parameter to specify how many times a kexec kernel can be loaded. These parameter allow hardening the system. While we are at it, fix a documentation issue and refactor some code. To: Jonathan Corbet To: Eric Biederman Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: kexec@lists.infradead.org Cc: Joel Fernandes (Google) Cc: Sergey Senozhatsky Cc: Steven Rostedt Cc: Ross Zwisler To: Philipp Rudo To: Guilherme G. Piccoli Signed-off-by: Ricardo Ribalda --- Changes in v3: - s/paramter/parameter/ Thanks Ghilherme! - s/permited/permitted/ Thanks Joel! - Link to v2: https://lore.kernel.org/r/20221114-disable-kexec-reset-v2-0-c498313c1...@chromium.org Changes in v2: - Instead of kexec_reboot_disabled, add two new counters (Thanks Philipp!) - Link to v1: https://lore.kernel.org/r/20221114-disable-kexec-reset-v1-0-fb51d20cf...@chromium.org --- Ricardo Ribalda (3): Documentation: sysctl: Correct kexec_load_disabled kexec: Factor out kexec_load_permitted kexec: Introduce parameters load_limit_reboot and load_limit_panic Documentation/admin-guide/kernel-parameters.txt | 14 Documentation/admin-guide/sysctl/kernel.rst | 7 +- include/linux/kexec.h | 3 +- kernel/kexec.c | 2 +- kernel/kexec_core.c | 98 - kernel/kexec_file.c | 2 +- 6 files changed, 119 insertions(+), 7 deletions(-) --- base-commit: 479174d402bcf60789106eedc4def3957c060bad change-id: 20221114-disable-kexec-reset-19b7e117338f Best regards, -- Ricardo Ribalda ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] Revert "x86/apic/x2apic: Implement IPI shorthands support"
On 12/20/22 at 12:38pm, Peter Zijlstra wrote:
> On Tue, Dec 20, 2022 at 01:34:58PM +0800, Baoquan He wrote:
> > This reverts commit 43931d350f30c6cd8c2f498d54ef7d65750abc92.
> >
> > On kvm guest with 4 cpus deployed, when adding 'nr_cpus=2' to normal
> > kernel's cmdline, and triggering crash to jump to kdump kernel, kdump
> > kernel will stably hang. Reverting commit 43931d350f30 ("x86/apic/x2apic:
> > Implement IPI shorthands support") can fix it.
> >
> > The problem will disappear if removing 'nr_cpus=2' from normal kerne's
> > cmdline.
>
> And the root cause for this is... ? Does the kvm x2apic emulation
> somehow get upset when we shorthand CPUs that haven't been initialized?
Thanks for checking.
I haven't figure out the root cause. I haven't read the apic code for
long time, and not familiar with the kvm code. So raise the issue to
upstream.
I can do testing if any suggestion.
Add our virt dev Dr. David Alan Gilbert to CC.
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] Revert "x86/apic/x2apic: Implement IPI shorthands support"
On Tue, Dec 20, 2022 at 01:34:58PM +0800, Baoquan He wrote:
> This reverts commit 43931d350f30c6cd8c2f498d54ef7d65750abc92.
>
> On kvm guest with 4 cpus deployed, when adding 'nr_cpus=2' to normal
> kernel's cmdline, and triggering crash to jump to kdump kernel, kdump
> kernel will stably hang. Reverting commit 43931d350f30 ("x86/apic/x2apic:
> Implement IPI shorthands support") can fix it.
>
> The problem will disappear if removing 'nr_cpus=2' from normal kerne's
> cmdline.
And the root cause for this is... ? Does the kvm x2apic emulation
somehow get upset when we shorthand CPUs that haven't been initialized?
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
