2018-02-26 15:37:18

by Chris Wilson

[permalink] [raw]
Subject: [PATCH] kernel: Downgrade warning for unsafe parameters

As using an unsafe module parameter is, by its very definition, an
expected user action, emitting a warning is overkill. Nothing has yet
gone wrong, and we add a taint flag for any future oops should
something actually go wrong. So instead of having a user controllable
pr_warn, downgrade it to a pr_notice for "a normal, but significant
condition".

We make use of unsafe kernel parameters in igt (we have not yet
succeeded in removing all such debugging options), which generates a
warning and taints the kernel. The warning is unhelpful as we then need
to filter it out again as we check that every test themselves do not
provoke any kernel warnings.

References: 91f9d330cc14 ("module: make it possible to have unsafe, tainting module params")
Signed-off-by: Chris Wilson <[email protected]>
Cc: Jani Nikula <[email protected]>
Cc: Rusty Russell <[email protected]>
Cc: Jean Delvare <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Li Zhong <[email protected]>
Cc: Petri Latvala <[email protected]>
Cc: Daniel Vetter <[email protected]>
---
kernel/params.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/params.c b/kernel/params.c
index cc9108c2a1fd..ce89f757e6da 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -111,8 +111,8 @@ bool parameq(const char *a, const char *b)
static void param_check_unsafe(const struct kernel_param *kp)
{
if (kp->flags & KERNEL_PARAM_FL_UNSAFE) {
- pr_warn("Setting dangerous option %s - tainting kernel\n",
- kp->name);
+ pr_notice("Setting dangerous option %s - tainting kernel\n",
+ kp->name);
add_taint(TAINT_USER, LOCKDEP_STILL_OK);
}
}
--
2.16.2



2018-02-26 15:32:36

by Jani Nikula

[permalink] [raw]
Subject: Re: [PATCH] kernel: Downgrade warning for unsafe parameters

On Mon, 26 Feb 2018, Chris Wilson <[email protected]> wrote:
> As using an unsafe module parameter is, by its very definition, an
> expected user action, emitting a warning is overkill. Nothing has yet
> gone wrong, and we add a taint flag for any future oops should
> something actually go wrong. So instead of having a user controllable
> pr_warn, downgrade it to a pr_notice for "a normal, but significant
> condition".
>
> We make use of unsafe kernel parameters in igt (we have not yet
> succeeded in removing all such debugging options), which generates a
> warning and taints the kernel. The warning is unhelpful as we then need
> to filter it out again as we check that every test themselves do not
> provoke any kernel warnings.

IGT being https://cgit.freedesktop.org/drm/igt-gpu-tools/ for those not
in the DRM/KMS circles.

> References: 91f9d330cc14 ("module: make it possible to have unsafe, tainting module params")
> Signed-off-by: Chris Wilson <[email protected]>
> Cc: Jani Nikula <[email protected]>
> Cc: Rusty Russell <[email protected]>
> Cc: Jean Delvare <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Li Zhong <[email protected]>
> Cc: Petri Latvala <[email protected]>
> Cc: Daniel Vetter <[email protected]>

Acked-by: Jani Nikula <[email protected]>

When I added the unsafe module params, I erred on the side of making
more noise about tainting, but there was really no discussion about
it. FWIW, I don't mind the change, as long as it's not debug level.

BR,
Jani.

> ---
> kernel/params.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/params.c b/kernel/params.c
> index cc9108c2a1fd..ce89f757e6da 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -111,8 +111,8 @@ bool parameq(const char *a, const char *b)
> static void param_check_unsafe(const struct kernel_param *kp)
> {
> if (kp->flags & KERNEL_PARAM_FL_UNSAFE) {
> - pr_warn("Setting dangerous option %s - tainting kernel\n",
> - kp->name);
> + pr_notice("Setting dangerous option %s - tainting kernel\n",
> + kp->name);
> add_taint(TAINT_USER, LOCKDEP_STILL_OK);
> }
> }

--
Jani Nikula, Intel Open Source Technology Center