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 <[email protected]>
To: Eric Biederman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Joel Fernandes (Google) <[email protected]>
Cc: Sergey Senozhatsky <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Ross Zwisler <[email protected]>
To: Philipp Rudo <[email protected]>
To: Guilherme G. Piccoli <[email protected]>
Signed-off-by: Ricardo Ribalda <[email protected]>
---
Changes in v5:
- Remove maxlen from ctl_table. Thanks Steven!
- Link to v4: https://lore.kernel.org/r/[email protected]
Changes in v4 (Thanks Steven!):
- Uses sysctl instead or module_parameters
- Pass image type instead of boolean to permitted
- Fix typo on flag handling
- Return -EINVAL for values that does not change the current value.
- Link to v3: https://lore.kernel.org/r/[email protected]
Changes in v3:
- s/paramter/parameter/ Thanks Ghilherme!
- s/permited/permitted/ Thanks Joel!
- Link to v2: https://lore.kernel.org/r/[email protected]
Changes in v2:
- Instead of kexec_reboot_disabled, add two new counters (Thanks Philipp!)
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Ricardo Ribalda (3):
Documentation: sysctl: Correct kexec_load_disabled
kexec: Factor out kexec_load_permitted
kexec: Introduce sysctl parameters kexec_load_limit_*
Documentation/admin-guide/sysctl/kernel.rst | 25 +++++++-
include/linux/kexec.h | 3 +-
kernel/kexec.c | 4 +-
kernel/kexec_core.c | 94 ++++++++++++++++++++++++++++-
kernel/kexec_file.c | 11 ++--
5 files changed, 127 insertions(+), 10 deletions(-)
---
base-commit: 479174d402bcf60789106eedc4def3957c060bad
change-id: 20221114-disable-kexec-reset-19b7e117338f
Best regards,
--
Ricardo Ribalda <[email protected]>
Add two parameters 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 sysctl, but only with a value
smaller than the current one (except -1).
Signed-off-by: Ricardo Ribalda <[email protected]>
---
Documentation/admin-guide/sysctl/kernel.rst | 18 ++++++
include/linux/kexec.h | 2 +-
kernel/kexec.c | 4 +-
kernel/kexec_core.c | 87 ++++++++++++++++++++++++++++-
kernel/kexec_file.c | 11 ++--
5 files changed, 114 insertions(+), 8 deletions(-)
diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index 97394bd9d065..a3922dffbd47 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -461,6 +461,24 @@ allowing a system to set up (and later use) an image without it being
altered.
Generally used together with the `modules_disabled`_ sysctl.
+kexec_load_limit_panic
+======================
+
+This parameter specifies a limit to the number of times the syscalls
+``kexec_load`` and ``kexec_file_load`` can be called with a crash
+image. It can only be set with a more restrictive value than the
+current one.
+
+= =============================================================
+-1 Unlimited calls to kexec. This is the default setting.
+N Number of calls left.
+= =============================================================
+
+kexec_load_limit_reboot
+======================
+
+Similar functionality as ``kexec_load_limit_panic``, but for a crash
+image.
kptr_restrict
=============
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 182e0c11b87b..791e65829f86 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(int kexec_image_type);
#ifndef kexec_flush_icache_page
#define kexec_flush_icache_page(page)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index ce1bca874a8d..92d301f98776 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -190,10 +190,12 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
static inline int kexec_load_check(unsigned long nr_segments,
unsigned long flags)
{
+ int image_type = (flags & KEXEC_ON_CRASH) ?
+ KEXEC_TYPE_CRASH : KEXEC_TYPE_DEFAULT;
int result;
/* We only trust the superuser with rebooting the system. */
- if (!kexec_load_permitted())
+ if (!kexec_load_permitted(image_type))
return -EPERM;
/* Permit LSMs and IMA to fail the kexec */
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index a1efc70f4158..951541d78ca8 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -926,10 +926,64 @@ int kimage_load_segment(struct kimage *image,
return result;
}
+struct kexec_load_limit {
+ /* Mutex protects the limit count. */
+ struct mutex mutex;
+ int limit;
+};
+
+static struct kexec_load_limit load_limit_reboot = {
+ .mutex = __MUTEX_INITIALIZER(load_limit_reboot.mutex),
+ .limit = -1,
+};
+
+static struct kexec_load_limit load_limit_panic = {
+ .mutex = __MUTEX_INITIALIZER(load_limit_panic.mutex),
+ .limit = -1,
+};
+
struct kimage *kexec_image;
struct kimage *kexec_crash_image;
static int kexec_load_disabled;
+
#ifdef CONFIG_SYSCTL
+static int kexec_limit_handler(struct ctl_table *table, int write,
+ void *buffer, size_t *lenp, loff_t *ppos)
+{
+ struct kexec_load_limit *limit = table->data;
+ int val;
+ struct ctl_table tmp = {
+ .data = &val,
+ .maxlen = sizeof(val),
+ .mode = table->mode,
+ };
+ int ret;
+
+ if (write) {
+ ret = proc_dointvec(&tmp, write, buffer, lenp, ppos);
+ if (ret)
+ return ret;
+
+ if (val < 0)
+ return -EINVAL;
+
+ mutex_lock(&limit->mutex);
+ if (limit->limit != -1 && val >= limit->limit)
+ ret = -EINVAL;
+ else
+ limit->limit = val;
+ mutex_unlock(&limit->mutex);
+
+ return ret;
+ }
+
+ mutex_lock(&limit->mutex);
+ val = limit->limit;
+ mutex_unlock(&limit->mutex);
+
+ return proc_dointvec(&tmp, write, buffer, lenp, ppos);
+}
+
static struct ctl_table kexec_core_sysctls[] = {
{
.procname = "kexec_load_disabled",
@@ -941,6 +995,18 @@ static struct ctl_table kexec_core_sysctls[] = {
.extra1 = SYSCTL_ONE,
.extra2 = SYSCTL_ONE,
},
+ {
+ .procname = "kexec_load_limit_panic",
+ .data = &load_limit_panic,
+ .mode = 0644,
+ .proc_handler = kexec_limit_handler,
+ },
+ {
+ .procname = "kexec_load_limit_reboot",
+ .data = &load_limit_reboot,
+ .mode = 0644,
+ .proc_handler = kexec_limit_handler,
+ },
{ }
};
@@ -952,13 +1018,30 @@ static int __init kexec_core_sysctl_init(void)
late_initcall(kexec_core_sysctl_init);
#endif
-bool kexec_load_permitted(void)
+bool kexec_load_permitted(int kexec_image_type)
{
+ struct kexec_load_limit *limit;
+
/*
* Only the superuser can use the kexec syscall and if it has not
* been disabled.
*/
- return capable(CAP_SYS_BOOT) && !kexec_load_disabled;
+ if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
+ return false;
+
+ /* Check limit counter and decrease it.*/
+ limit = (kexec_image_type == KEXEC_TYPE_CRASH) ?
+ &load_limit_panic : &load_limit_reboot;
+ mutex_lock(&limit->mutex);
+ if (!limit->limit) {
+ mutex_unlock(&limit->mutex);
+ return false;
+ }
+ if (limit->limit != -1)
+ limit->limit--;
+ mutex_unlock(&limit->mutex);
+
+ return true;
}
/*
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 29efa43ea951..da9cfbbb0cbd 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -326,11 +326,13 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
unsigned long, cmdline_len, const char __user *, cmdline_ptr,
unsigned long, flags)
{
- int ret = 0, i;
+ int image_type = (flags & KEXEC_FILE_ON_CRASH) ?
+ KEXEC_TYPE_CRASH : KEXEC_TYPE_DEFAULT;
struct kimage **dest_image, *image;
+ int ret = 0, i;
/* We only trust the superuser with rebooting the system. */
- if (!kexec_load_permitted())
+ if (!kexec_load_permitted(flags & KEXEC_FILE_ON_CRASH))
return -EPERM;
/* Make sure we have a legal set of flags */
@@ -342,11 +344,12 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
if (!kexec_trylock())
return -EBUSY;
- dest_image = &kexec_image;
- if (flags & KEXEC_FILE_ON_CRASH) {
+ if (image_type == KEXEC_TYPE_CRASH) {
dest_image = &kexec_crash_image;
if (kexec_crash_image)
arch_kexec_unprotect_crashkres();
+ } else {
+ dest_image = &kexec_image;
}
if (flags & KEXEC_FILE_UNLOAD)
--
2.39.0.314.g84b9a713c41-goog-b4-0.11.0-dev-696ae
On Wed, 21 Dec 2022 20:45:56 +0100
Ricardo Ribalda <[email protected]> wrote:
> 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 <[email protected]>
> To: Eric Biederman <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: Joel Fernandes (Google) <[email protected]>
> Cc: Sergey Senozhatsky <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Ross Zwisler <[email protected]>
> To: Philipp Rudo <[email protected]>
> To: Guilherme G. Piccoli <[email protected]>
> Signed-off-by: Ricardo Ribalda <[email protected]>
Reviewed-by: Steven Rostedt (Google) <[email protected]>
-- Steve
Hi,
On 12/21/22 at 08:45pm, Ricardo Ribalda wrote:
> Add two parameters 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 sysctl, but only with a value
> smaller than the current one (except -1).
This patch is not complex, and doens't add code complexity. And from
code comment and document, we can easily know how the new toggles can be
used. BUT I am wondering if everybody have got why they are needed, and
where or when they are needed and will be used, except of stupid me.
We have had a toggle kexec_load_disabled which can be used to close the
door before alread loading image. Now we want another two toggles to
guard how many times we can flip the door. I am not familiar with
kernel security policy, is it not safe to provide details in log? If so,
please ignore this mail.
In fact, my concern is that in the future, if a new user of kexec/kdump
want to know why these two knobs are added, while can't get any helpful
information from commit message. If she/he asks me, I can only shrug. Up
to now, I don't know what real use case these two toggles are used for,
why they have to be added. And the default value of them are '-1', how
will we make use of them in distros? What's the suggested value? Or '-1'
is the suggested value, you just use them in some cornor case on some
mysterious system?
Just saying we want to add some new useful stuffs, that's it. Frankly
speaking, w/o special reason, this is not the way we are supposed to add
new stuffs into kernel.
Thanks
Baoquan
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
> ---
> Documentation/admin-guide/sysctl/kernel.rst | 18 ++++++
> include/linux/kexec.h | 2 +-
> kernel/kexec.c | 4 +-
> kernel/kexec_core.c | 87 ++++++++++++++++++++++++++++-
> kernel/kexec_file.c | 11 ++--
> 5 files changed, 114 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
> index 97394bd9d065..a3922dffbd47 100644
> --- a/Documentation/admin-guide/sysctl/kernel.rst
> +++ b/Documentation/admin-guide/sysctl/kernel.rst
> @@ -461,6 +461,24 @@ allowing a system to set up (and later use) an image without it being
> altered.
> Generally used together with the `modules_disabled`_ sysctl.
>
> +kexec_load_limit_panic
> +======================
> +
> +This parameter specifies a limit to the number of times the syscalls
> +``kexec_load`` and ``kexec_file_load`` can be called with a crash
> +image. It can only be set with a more restrictive value than the
> +current one.
> +
> += =============================================================
> +-1 Unlimited calls to kexec. This is the default setting.
> +N Number of calls left.
> += =============================================================
> +
> +kexec_load_limit_reboot
> +======================
> +
> +Similar functionality as ``kexec_load_limit_panic``, but for a crash
> +image.
>
> kptr_restrict
> =============
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 182e0c11b87b..791e65829f86 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(int kexec_image_type);
>
> #ifndef kexec_flush_icache_page
> #define kexec_flush_icache_page(page)
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index ce1bca874a8d..92d301f98776 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -190,10 +190,12 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
> static inline int kexec_load_check(unsigned long nr_segments,
> unsigned long flags)
> {
> + int image_type = (flags & KEXEC_ON_CRASH) ?
> + KEXEC_TYPE_CRASH : KEXEC_TYPE_DEFAULT;
> int result;
>
> /* We only trust the superuser with rebooting the system. */
> - if (!kexec_load_permitted())
> + if (!kexec_load_permitted(image_type))
> return -EPERM;
>
> /* Permit LSMs and IMA to fail the kexec */
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index a1efc70f4158..951541d78ca8 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -926,10 +926,64 @@ int kimage_load_segment(struct kimage *image,
> return result;
> }
>
> +struct kexec_load_limit {
> + /* Mutex protects the limit count. */
> + struct mutex mutex;
> + int limit;
> +};
> +
> +static struct kexec_load_limit load_limit_reboot = {
> + .mutex = __MUTEX_INITIALIZER(load_limit_reboot.mutex),
> + .limit = -1,
> +};
> +
> +static struct kexec_load_limit load_limit_panic = {
> + .mutex = __MUTEX_INITIALIZER(load_limit_panic.mutex),
> + .limit = -1,
> +};
> +
> struct kimage *kexec_image;
> struct kimage *kexec_crash_image;
> static int kexec_load_disabled;
> +
> #ifdef CONFIG_SYSCTL
> +static int kexec_limit_handler(struct ctl_table *table, int write,
> + void *buffer, size_t *lenp, loff_t *ppos)
> +{
> + struct kexec_load_limit *limit = table->data;
> + int val;
> + struct ctl_table tmp = {
> + .data = &val,
> + .maxlen = sizeof(val),
> + .mode = table->mode,
> + };
> + int ret;
> +
> + if (write) {
> + ret = proc_dointvec(&tmp, write, buffer, lenp, ppos);
> + if (ret)
> + return ret;
> +
> + if (val < 0)
> + return -EINVAL;
> +
> + mutex_lock(&limit->mutex);
> + if (limit->limit != -1 && val >= limit->limit)
> + ret = -EINVAL;
> + else
> + limit->limit = val;
> + mutex_unlock(&limit->mutex);
> +
> + return ret;
> + }
> +
> + mutex_lock(&limit->mutex);
> + val = limit->limit;
> + mutex_unlock(&limit->mutex);
> +
> + return proc_dointvec(&tmp, write, buffer, lenp, ppos);
> +}
> +
> static struct ctl_table kexec_core_sysctls[] = {
> {
> .procname = "kexec_load_disabled",
> @@ -941,6 +995,18 @@ static struct ctl_table kexec_core_sysctls[] = {
> .extra1 = SYSCTL_ONE,
> .extra2 = SYSCTL_ONE,
> },
> + {
> + .procname = "kexec_load_limit_panic",
> + .data = &load_limit_panic,
> + .mode = 0644,
> + .proc_handler = kexec_limit_handler,
> + },
> + {
> + .procname = "kexec_load_limit_reboot",
> + .data = &load_limit_reboot,
> + .mode = 0644,
> + .proc_handler = kexec_limit_handler,
> + },
> { }
> };
>
> @@ -952,13 +1018,30 @@ static int __init kexec_core_sysctl_init(void)
> late_initcall(kexec_core_sysctl_init);
> #endif
>
> -bool kexec_load_permitted(void)
> +bool kexec_load_permitted(int kexec_image_type)
> {
> + struct kexec_load_limit *limit;
> +
> /*
> * Only the superuser can use the kexec syscall and if it has not
> * been disabled.
> */
> - return capable(CAP_SYS_BOOT) && !kexec_load_disabled;
> + if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
> + return false;
> +
> + /* Check limit counter and decrease it.*/
> + limit = (kexec_image_type == KEXEC_TYPE_CRASH) ?
> + &load_limit_panic : &load_limit_reboot;
> + mutex_lock(&limit->mutex);
> + if (!limit->limit) {
> + mutex_unlock(&limit->mutex);
> + return false;
> + }
> + if (limit->limit != -1)
> + limit->limit--;
> + mutex_unlock(&limit->mutex);
> +
> + return true;
> }
>
> /*
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 29efa43ea951..da9cfbbb0cbd 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -326,11 +326,13 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> unsigned long, cmdline_len, const char __user *, cmdline_ptr,
> unsigned long, flags)
> {
> - int ret = 0, i;
> + int image_type = (flags & KEXEC_FILE_ON_CRASH) ?
> + KEXEC_TYPE_CRASH : KEXEC_TYPE_DEFAULT;
> struct kimage **dest_image, *image;
> + int ret = 0, i;
>
> /* We only trust the superuser with rebooting the system. */
> - if (!kexec_load_permitted())
> + if (!kexec_load_permitted(flags & KEXEC_FILE_ON_CRASH))
> return -EPERM;
>
> /* Make sure we have a legal set of flags */
> @@ -342,11 +344,12 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> if (!kexec_trylock())
> return -EBUSY;
>
> - dest_image = &kexec_image;
> - if (flags & KEXEC_FILE_ON_CRASH) {
> + if (image_type == KEXEC_TYPE_CRASH) {
> dest_image = &kexec_crash_image;
> if (kexec_crash_image)
> arch_kexec_unprotect_crashkres();
> + } else {
> + dest_image = &kexec_image;
> }
>
> if (flags & KEXEC_FILE_UNLOAD)
>
> --
> 2.39.0.314.g84b9a713c41-goog-b4-0.11.0-dev-696ae
>
> _______________________________________________
> kexec mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/kexec
>
Hi Ricardo,
On Wed, 21 Dec 2022 20:45:59 +0100
Ricardo Ribalda <[email protected]> wrote:
> Add two parameters 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 sysctl, but only with a value
> smaller than the current one (except -1).
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
> ---
> Documentation/admin-guide/sysctl/kernel.rst | 18 ++++++
> include/linux/kexec.h | 2 +-
> kernel/kexec.c | 4 +-
> kernel/kexec_core.c | 87 ++++++++++++++++++++++++++++-
> kernel/kexec_file.c | 11 ++--
> 5 files changed, 114 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
> index 97394bd9d065..a3922dffbd47 100644
> --- a/Documentation/admin-guide/sysctl/kernel.rst
> +++ b/Documentation/admin-guide/sysctl/kernel.rst
> @@ -461,6 +461,24 @@ allowing a system to set up (and later use) an image without it being
> altered.
> Generally used together with the `modules_disabled`_ sysctl.
>
> +kexec_load_limit_panic
> +======================
> +
> +This parameter specifies a limit to the number of times the syscalls
> +``kexec_load`` and ``kexec_file_load`` can be called with a crash
> +image. It can only be set with a more restrictive value than the
> +current one.
> +
> += =============================================================
> +-1 Unlimited calls to kexec. This is the default setting.
> +N Number of calls left.
> += =============================================================
> +
> +kexec_load_limit_reboot
> +======================
> +
> +Similar functionality as ``kexec_load_limit_panic``, but for a crash
> +image.
Is this description correct? IIUC the ``kexec_load_limit_panic`` is for
the crash image, and ``kexec_load_limit_reboot`` is for the normal
image, but the sentence above says again: "for a crash image."
Petr T
On Wed, Dec 21, 2022 at 08:45:59PM +0100, Ricardo Ribalda wrote:
> += =============================================================
> +-1 Unlimited calls to kexec. This is the default setting.
> +N Number of calls left.
> += =============================================================
> +
> +kexec_load_limit_reboot
> +======================
> +
You need to match the length of table borders and section underline:
---- >8 ----
diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index a3922dffbd474b..a64b340c3ae509 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -469,13 +469,13 @@ This parameter specifies a limit to the number of times the syscalls
image. It can only be set with a more restrictive value than the
current one.
-= =============================================================
--1 Unlimited calls to kexec. This is the default setting.
-N Number of calls left.
-= =============================================================
+== =============================================================
+-1 Unlimited calls to kexec. This is the default setting.
+N Number of calls left.
+== =============================================================
kexec_load_limit_reboot
-======================
+=======================
Similar functionality as ``kexec_load_limit_panic``, but for a crash
image.
Thanks.
--
An old man doll... just what I always wanted! - Clara