| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <201911300820.C587C6AC41@keescook>
Date: Sat, 30 Nov 2019 08:22:57 -0800
From: Kees Cook <keescook@...omium.org>
To: Konstantin Khlebnikov <khlebnikov@...dex-team.ru>
Cc: linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
Sasha Levin <sashal@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
rcu@...r.kernel.org, Tejun Heo <tj@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 1/2] kernel: set taint flag 'L' at any kind of lockup
On Fri, Nov 29, 2019 at 04:21:46PM +0300, Konstantin Khlebnikov wrote:
> Any lockup or stall notifies about unexpected lack of progress.
> It's better to know about them for further problem investigations.
>
> Right now only softlockup has own taint flag. Let's generalize it.
>
> This patch renames TAINT_SOFTLOCKUP into TAINT_LOCKUP at sets it for:
> - softlockup
> - hardlockup
> - RCU stalls
> - stuck in workqueues
> - detected task hung
This seems reasonable to me. The only nit I have is that it might be
nicer to separate the rename from the new call sites, but if there's no
v2, I'm fine with it this way.
Reviewed-by: Kees Cook <keescook@...omium.org>
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@...dex-team.ru>
> ---
> Documentation/admin-guide/sysctl/kernel.rst | 2 +-
> Documentation/admin-guide/tainted-kernels.rst | 8 ++++++--
> include/linux/kernel.h | 2 +-
> kernel/hung_task.c | 2 ++
> kernel/panic.c | 2 +-
> kernel/rcu/tree_stall.h | 1 +
> kernel/watchdog.c | 2 +-
> kernel/watchdog_hld.c | 1 +
> kernel/workqueue.c | 1 +
> tools/debugging/kernel-chktaint | 2 +-
> 10 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
> index 032c7cd3cede..60867ec525a4 100644
> --- a/Documentation/admin-guide/sysctl/kernel.rst
> +++ b/Documentation/admin-guide/sysctl/kernel.rst
> @@ -1082,7 +1082,7 @@ ORed together. The letters are seen in "Tainted" line of Oops reports.
> 2048 `(I)` workaround for bug in platform firmware applied
> 4096 `(O)` externally-built ("out-of-tree") module was loaded
> 8192 `(E)` unsigned module was loaded
> - 16384 `(L)` soft lockup occurred
> + 16384 `(L)` lockup occurred
> 32768 `(K)` kernel has been live patched
> 65536 `(X)` Auxiliary taint, defined and used by for distros
> 131072 `(T)` The kernel was built with the struct randomization plugin
> diff --git a/Documentation/admin-guide/tainted-kernels.rst b/Documentation/admin-guide/tainted-kernels.rst
> index 71e9184a9079..fc76d0aad9f5 100644
> --- a/Documentation/admin-guide/tainted-kernels.rst
> +++ b/Documentation/admin-guide/tainted-kernels.rst
> @@ -96,7 +96,7 @@ Bit Log Number Reason that got the kernel tainted
> 11 _/I 2048 workaround for bug in platform firmware applied
> 12 _/O 4096 externally-built ("out-of-tree") module was loaded
> 13 _/E 8192 unsigned module was loaded
> - 14 _/L 16384 soft lockup occurred
> + 14 _/L 16384 lockup occurred
> 15 _/K 32768 kernel has been live patched
> 16 _/X 65536 auxiliary taint, defined for and used by distros
> 17 _/T 131072 kernel was built with the struct randomization plugin
> @@ -152,7 +152,11 @@ More detailed explanation for tainting
> 13) ``E`` if an unsigned module has been loaded in a kernel supporting
> module signature.
>
> - 14) ``L`` if a soft lockup has previously occurred on the system.
> + 14) ``L`` if some kind of lockup has previously occurred on the system:
> + - soft/hardlockup, see Documentation/admin-guide/lockup-watchdogs.rst
> + - RCU stall, see Documentation/RCU/stallwarn.txt
> + - hung task detected, see CONFIG_DETECT_HUNG_TASK
> + - kernel workqueue lockup, see CONFIG_WQ_WATCHDOG
>
> 15) ``K`` if the kernel has been live patched.
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index d83d403dac2e..e8a6808e4f2f 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -591,7 +591,7 @@ extern enum system_states {
> #define TAINT_FIRMWARE_WORKAROUND 11
> #define TAINT_OOT_MODULE 12
> #define TAINT_UNSIGNED_MODULE 13
> -#define TAINT_SOFTLOCKUP 14
> +#define TAINT_LOCKUP 14
> #define TAINT_LIVEPATCH 15
> #define TAINT_AUX 16
> #define TAINT_RANDSTRUCT 17
> diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> index 14a625c16cb3..521eb2fbf5fc 100644
> --- a/kernel/hung_task.c
> +++ b/kernel/hung_task.c
> @@ -139,6 +139,8 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout)
> hung_task_show_lock = true;
> }
>
> + add_taint(TAINT_LOCKUP, LOCKDEP_STILL_OK);
> +
> touch_nmi_watchdog();
> }
>
> diff --git a/kernel/panic.c b/kernel/panic.c
> index f470a038b05b..d7750a45ca8d 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -372,7 +372,7 @@ const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = {
> [ TAINT_FIRMWARE_WORKAROUND ] = { 'I', ' ', false },
> [ TAINT_OOT_MODULE ] = { 'O', ' ', true },
> [ TAINT_UNSIGNED_MODULE ] = { 'E', ' ', true },
> - [ TAINT_SOFTLOCKUP ] = { 'L', ' ', false },
> + [ TAINT_LOCKUP ] = { 'L', ' ', false },
> [ TAINT_LIVEPATCH ] = { 'K', ' ', true },
> [ TAINT_AUX ] = { 'X', ' ', true },
> [ TAINT_RANDSTRUCT ] = { 'T', ' ', true },
> diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
> index c0b8c458d8a6..181495efff80 100644
> --- a/kernel/rcu/tree_stall.h
> +++ b/kernel/rcu/tree_stall.h
> @@ -74,6 +74,7 @@ early_initcall(check_cpu_stall_init);
> /* If so specified via sysctl, panic, yielding cleaner stall-warning output. */
> static void panic_on_rcu_stall(void)
> {
> + add_taint(TAINT_LOCKUP, LOCKDEP_STILL_OK);
> if (sysctl_panic_on_rcu_stall)
> panic("RCU Stall\n");
> }
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index f41334ef0971..d60b195708f7 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -466,7 +466,7 @@ static enum hrtimer_restart watchdog_timer_fn(struct hrtimer *hrtimer)
> smp_mb__after_atomic();
> }
>
> - add_taint(TAINT_SOFTLOCKUP, LOCKDEP_STILL_OK);
> + add_taint(TAINT_LOCKUP, LOCKDEP_STILL_OK);
> if (softlockup_panic)
> panic("softlockup: hung tasks");
> __this_cpu_write(soft_watchdog_warn, true);
> diff --git a/kernel/watchdog_hld.c b/kernel/watchdog_hld.c
> index 247bf0b1582c..f77256f47422 100644
> --- a/kernel/watchdog_hld.c
> +++ b/kernel/watchdog_hld.c
> @@ -152,6 +152,7 @@ static void watchdog_overflow_callback(struct perf_event *event,
> !test_and_set_bit(0, &hardlockup_allcpu_dumped))
> trigger_allbutself_cpu_backtrace();
>
> + add_taint(TAINT_LOCKUP, LOCKDEP_STILL_OK);
> if (hardlockup_panic)
> nmi_panic(regs, "Hard LOCKUP");
>
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index bc2e09a8ea61..825a92893882 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -5741,6 +5741,7 @@ static void wq_watchdog_timer_fn(struct timer_list *unused)
> pr_cont_pool_info(pool);
> pr_cont(" stuck for %us!\n",
> jiffies_to_msecs(jiffies - pool_ts) / 1000);
> + add_taint(TAINT_LOCKUP, LOCKDEP_STILL_OK);
> }
> }
>
> diff --git a/tools/debugging/kernel-chktaint b/tools/debugging/kernel-chktaint
> index 2240cb56e6e5..9f24719d8c80 100755
> --- a/tools/debugging/kernel-chktaint
> +++ b/tools/debugging/kernel-chktaint
> @@ -168,7 +168,7 @@ if [ `expr $T % 2` -eq 0 ]; then
> addout " "
> else
> addout "L"
> - echo " * soft lockup occurred (#14)"
> + echo " * lockup occurred (#14)"
> fi
>
> T=`expr $T / 2`
>
--
Kees Cook
Powered by blists - more mailing lists