All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luis Chamberlain <mcgrof@kernel.org>
To: Rafael Aquini <aquini@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	kexec@lists.infradead.org, linux-fsdevel@vger.kernel.org,
	dyoung@redhat.com, bhe@redhat.com, corbet@lwn.net,
	keescook@chromium.org, akpm@linux-foundation.org, cai@lca.pw,
	rdunlap@infradead.org, tytso@mit.edu, bunk@kernel.org,
	torvalds@linux-foundation.org, gregkh@linuxfoundation.org,
	labbott@redhat.com, jeffm@suse.com, jikos@kernel.org,
	jeyu@suse.de, tiwai@suse.de, AnDavis@suse.com,
	rpalethorpe@suse.de
Subject: Re: [PATCH v3] kernel: add panic_on_taint
Date: Mon, 11 May 2020 18:24:55 +0000	[thread overview]
Message-ID: <20200511182455.GR11244@42.do-not-panic.com> (raw)
In-Reply-To: <20200509135737.622299-1-aquini@redhat.com>

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

WARNING: multiple messages have this Message-ID
From: Luis Chamberlain <mcgrof@kernel.org>
To: Rafael Aquini <aquini@redhat.com>
Cc: linux-doc@vger.kernel.org, tiwai@suse.de, jeffm@suse.com,
	bhe@redhat.com, corbet@lwn.net, labbott@redhat.com,
	dyoung@redhat.com, AnDavis@suse.com, rpalethorpe@suse.de,
	keescook@chromium.org, jikos@kernel.org, cai@lca.pw,
	bunk@kernel.org, tytso@mit.edu, jeyu@suse.de,
	gregkh@linuxfoundation.org, rdunlap@infradead.org,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, akpm@linux-foundation.org,
	torvalds@linux-foundation.org
Subject: Re: [PATCH v3] kernel: add panic_on_taint
Date: Mon, 11 May 2020 18:24:55 +0000	[thread overview]
Message-ID: <20200511182455.GR11244@42.do-not-panic.com> (raw)
In-Reply-To: <20200509135737.622299-1-aquini@redhat.com>

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

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

  parent reply	other threads:[~2020-05-11 18:24 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-09 13:57 Rafael Aquini
2020-05-09 13:57 ` Rafael Aquini
2020-05-09 18:59 ` Kees Cook
2020-05-09 18:59   ` Kees Cook
2020-05-10  2:59 ` Baoquan He
2020-05-10  2:59   ` Baoquan He
2020-05-10  4:10   ` Randy Dunlap
2020-05-10  4:10     ` Randy Dunlap
2020-05-10  5:16     ` Baoquan He
2020-05-10  5:16       ` Baoquan He
2020-05-10 18:22   ` Rafael Aquini
2020-05-10 18:22     ` Rafael Aquini
2020-05-11  1:11     ` Baoquan He
2020-05-11  1:11       ` Baoquan He
2020-05-11 18:24 ` Luis Chamberlain [this message]
2020-05-11 18:24   ` Luis Chamberlain
2020-05-11 20:03   ` Rafael Aquini
2020-05-11 20:03     ` Rafael Aquini
2020-05-11 21:05     ` Luis Chamberlain
2020-05-11 21:05       ` Luis Chamberlain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200511182455.GR11244@42.do-not-panic.com \
    --to=mcgrof@kernel.org \
    --cc=AnDavis@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=aquini@redhat.com \
    --cc=bhe@redhat.com \
    --cc=bunk@kernel.org \
    --cc=cai@lca.pw \
    --cc=corbet@lwn.net \
    --cc=dyoung@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jeffm@suse.com \
    --cc=jeyu@suse.de \
    --cc=jikos@kernel.org \
    --cc=keescook@chromium.org \
    --cc=kexec@lists.infradead.org \
    --cc=labbott@redhat.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=rpalethorpe@suse.de \
    --cc=tiwai@suse.de \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --subject='Re: [PATCH v3] kernel: add panic_on_taint' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.