linux-kernel - Re: [PATCH 3/3] panic: Allow printing extra panic information on kdump

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Yd/qmyz+qSuoUwbs@alley>
Date:   Thu, 13 Jan 2022 10:02:19 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     "Guilherme G. Piccoli" <gpiccoli@...lia.com>
Cc:     linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-doc@...r.kernel.org, mcgrof@...nel.org,
        keescook@...omium.org, yzaikin@...gle.com,
        akpm@...ux-foundation.org, feng.tang@...el.com,
        siglesias@...lia.com, kernel@...ccoli.net
Subject: Re: [PATCH 3/3] panic: Allow printing extra panic information on
 kdump

On Tue 2021-11-09 17:28:48, Guilherme G. Piccoli wrote:
> Currently we have the "panic_print" parameter/sysctl to allow some extra
> information to be printed in a panic event. On the other hand, the kdump
> mechanism allows to kexec a new kernel to collect a memory dump for the
> running kernel in case of panic.
> Right now these options are incompatible: the user either sets the kdump
> or makes use of "panic_print". The code path of "panic_print" isn't
> reached when kdump is configured.
> 
> There are situations though in which this would be interesting: for
> example, in systems that are very memory constrained, a handcrafted
> tiny kernel/initrd for kdump might be used in order to only collect the
> dmesg in kdump kernel. Even more common, systems with no disk space for
> the full (compressed) memory dump might very well rely in this
> functionality too, dumping only the dmesg with the additional information
> provided by "panic_print".

Is anyone really using this approach? kmsg_dump() looks like a better
choice when there are memory constrains. It does not need to reserve
memory for booting the crash kernel.

I would not mind much but this change depends on a not fully reliable
assumption, see below.

Also it will also complicate the solution for the kmsg_dump() code path.
It would be better to discuss this togeter with the other patch
https://lore.kernel.org/r/20220106212835.119409-1-gpiccoli@igalia.com


> So, this is what the patch does: allows both functionality to co-exist;
> if "panic_print" is set and the system performs a kdump, the extra
> information is printed on dmesg before the kexec. Some notes about the
> design choices here:
> 
> (a) We could have introduced a sysctl or an extra bit on "panic_print"
> to allow enabling the co-existence of kdump and "panic_print", but seems
> that would be over-engineering; we have 3 cases, let's check how this
> patch change things:
> 
> - if the user have kdump set and not "panic_print", nothing changes;
> - if the user have "panic_print" set and not kdump, nothing changes;
> - if both are enabled, now we print the extra information before kdump,
> which is exactly the goal of the patch (and should be the goal of the
> user, since they enabled both options).
> 
> (b) We assume that the code path won't return from __crash_kexec()
> so we didn't guard against double execution of panic_print_sys_info().

This sounds suspiciously. There is small race window but it actually works.
__crash_kexec() really never returns when @kexec_crash_image is
loaded. Well, it might break in the future if the code is modified.

Best Regards,
Petr