linux-kernel - Re: [PATCH V4] notifier/panic: Introduce panic_notifier

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <fda509a5-ea0d-4d1d-a1c1-ca5e80010fc0@igalia.com>
Date:   Tue, 25 Jan 2022 09:34:23 -0300
From:   "Guilherme G. Piccoli" <gpiccoli@...lia.com>
To:     "d.hatayama@...itsu.com" <d.hatayama@...itsu.com>,
        "kexec@...ts.infradead.org" <kexec@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dyoung@...hat.com" <dyoung@...hat.com>
Cc:     "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "bhe@...hat.com" <bhe@...hat.com>,
        "vgoyal@...hat.com" <vgoyal@...hat.com>,
        "stern@...land.harvard.edu" <stern@...land.harvard.edu>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "andriy.shevchenko@...ux.intel.com" 
        <andriy.shevchenko@...ux.intel.com>,
        "corbet@....net" <corbet@....net>,
        "halves@...onical.com" <halves@...onical.com>,
        "kernel@...ccoli.net" <kernel@...ccoli.net>
Subject: Re: [PATCH V4] notifier/panic: Introduce panic_notifier_filter

On 25/01/2022 08:50, d.hatayama@...itsu.com wrote:
>> +       while ((func = strsep(&buf, ","))) {
>> +               addr = kallsyms_lookup_name(func);
>> +               if (!addr) {
>> +                       pr_warn("panic_notifier_filter: invalid symbol %s\n", func);
>> +                       continue;
>> +               }
> 
> Could you remove this check?
> 
> panic_notifier_list is exported to kernel modules and this check
> prevents such users from using this feature.
> 
> Thanks.
> HATAYAMA, Daisuke

Hi, thanks for the review. First of all, notice that it's very likely
this patch isn't gonna get merged this way, we are considering a
refactor that included 2 panic notifiers: one a bit earlier (pre_dump),
that includes functions less risky, as watchdog unloaders, kernel offset
dump, etc, and the second panic notifier (post_dump) will keep the
majority of callbacks, and can be conditionally executed on kdump
through the usage of "crash_kexec_post_notifiers".

Anyway, I'm curious with your code review - how can we use this filter
with modules, if the filter setup is invoked as early_param(), before
modules load? In that case, module functions won't have a valid address,
correct? So, in that moment, this lookup fails, we cannot record an
unloaded module address in such list. Please, correct me if I'm wrong.

Cheers,

Guilherme