linux-kernel - Re: [PATCH v3] kernel: add panic_on

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]

Date:   Mon, 11 May 2020 18:24:55 +0000
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     Rafael Aquini <aquini@...hat.com>
Cc:     linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
        kexec@...ts.infradead.org, linux-fsdevel@...r.kernel.org,
        dyoung@...hat.com, bhe@...hat.com, corbet@....net,
        keescook@...omium.org, akpm@...ux-foundation.org, cai@....pw,
        rdunlap@...radead.org, tytso@....edu, bunk@...nel.org,
        torvalds@...ux-foundation.org, gregkh@...uxfoundation.org,
        labbott@...hat.com, jeffm@...e.com, jikos@...nel.org, jeyu@...e.de,
        tiwai@...e.de, AnDavis@...e.com, rpalethorpe@...e.de
Subject: Re: [PATCH v3] kernel: add panic_on_taint

On Sat, May 09, 2020 at 09:57:37AM -0400, Rafael Aquini wrote:
> +Trigger Kdump on add_taint()
> +============================
> +
> +The kernel parameter, panic_on_taint, calls panic() from within add_taint(),
> +whenever the value set in this bitmask matches with the bit flag being set
> +by add_taint(). This will cause a kdump to occur at the panic() call.
> +In cases where a user wants to specify this during runtime,
> +/proc/sys/kernel/panic_on_taint can be set to a respective bitmask value
> +to achieve the same behaviour.
> +
>  Contact
>  =======
>  
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 7bc83f3d9bdf..4a69fe49a70d 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3404,6 +3404,21 @@
>  	panic_on_warn	panic() instead of WARN().  Useful to cause kdump
>  			on a WARN().
>  
> +	panic_on_taint=	[KNL] conditionally panic() in add_taint()
> +			Format: <str>
> +			Specifies, as a string, the TAINT flag set that will
> +			compose a bitmask for calling panic() when the kernel
> +			gets tainted.
> +			See Documentation/admin-guide/tainted-kernels.rst for
> +			details on the taint flags that users can pick to
> +			compose the bitmask to assign to panic_on_taint.
> +			When the string is prefixed with a '-' the bitmask
> +			set in panic_on_taint will be mutually exclusive
> +			with the sysctl knob kernel.tainted, and any attempt
> +			to write to that sysctl will fail with -EINVAL for
> +			any taint value that masks with the flags set for
> +			this option.

This talks about using a string, but that it sets a bitmask. Its not
very clear that one must use the string representation from each taint
flag. Also, I don't think to use the character representation as we
limit ourselves to the alphabet and quirky what-should-be-arbitrary
characters that represent the taint flags. The taint flag character
representation is juse useful for human reading of a panic, but I think
because of the limitation of the mask with the alphabet this was not
such a great idea long term.

So, I don't think we should keep on extending the alphabet use case, a
simple digit representation would suffice. I think this means we'd need
two params one for exclusive and one for the value of the taint.

Using a hex value or number also lets us make the input value shorter.

If a kernel boots with panic-on-taint flag not yet supported, we don't
complain, therefore getting a false sense of security that we will panic
with a not yet supported taint flag. I think we should pr_warn() or
fail to boot when that happens.

  Luis