Re: [Intel-gfx] [PATCH 0/4] module: add support for unsafe, tainting parameters
On Wed, 20 Aug 2014, Rusty Russell ru...@rustcorp.com.au wrote: I've applied this cleanup on top, however. Cheers, Rusty. Subject: param: check for tainting before calling set op. This means every set op doesn't need to call it, and it can move into params.c. Much better, thanks. I was looking for a way to do something like this, but obviously didn't look hard enough. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/4] module: add support for unsafe, tainting parameters
Daniel Vetter daniel.vet...@ffwll.ch writes:
On Wed, Aug 13, 2014 at 10:25 PM, Rusty Russell ru...@rustcorp.com.au wrote:
Jani Nikula jani.nik...@intel.com writes:
This is a generic version of Daniel's patch [1] letting us have unsafe
module parameters (experimental, debugging, testing, etc.) that taint
the kernel when set. Quoting Daniel,
OK, I think the idea is fine, but we'll probably only want this for
a few types (eg. int and bool). So for the moment I prefer a more
naive approach.
Does this work for you?
Can you please discuss this with yourself from a few months back?
We've done the general version since you suggested that just doing it
for int is a bit lame ;-) And I actually agreed so asked Jani to look
into that.
Don't listen to me, I'm an idiot!
Applied.
I've applied this cleanup on top, however.
Cheers,
Rusty.
Subject: param: check for tainting before calling set op.
This means every set op doesn't need to call it, and it can move into
params.c.
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 9531f9f9729e..593501996574 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -374,22 +374,6 @@ static inline void destroy_params(const struct
kernel_param *params,
#define __param_check(name, p, type) \
static inline type __always_unused *__check_##name(void) { return(p); }
-/**
- * param_check_unsafe - Warn and taint the kernel if setting dangerous options.
- *
- * This gets called from all the standard param setters, but can be used from
- * custom setters as well.
- */
-static inline 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);
- add_taint(TAINT_USER, LOCKDEP_STILL_OK);
- }
-}
-
extern struct kernel_param_ops param_ops_byte;
extern int param_set_byte(const char *val, const struct kernel_param *kp);
extern int param_get_byte(char *buffer, const struct kernel_param *kp);
diff --git a/kernel/params.c b/kernel/params.c
index ad8d04563c3a..f3cc977d6a66 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -83,6 +83,15 @@ bool parameq(const char *a, const char *b)
return parameqn(a, b, strlen(a)+1);
}
+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);
+ add_taint(TAINT_USER, LOCKDEP_STILL_OK);
+ }
+}
+
static int parse_one(char *param,
char *val,
const char *doing,
@@ -109,6 +119,7 @@ static int parse_one(char *param,
pr_debug(handling %s with %p\n, param,
params[i].ops-set);
mutex_lock(param_lock);
+ param_check_unsafe(params[i]);
err = params[i].ops-set(val, params[i]);
mutex_unlock(param_lock);
return err;
@@ -233,7 +244,6 @@ char *parse_args(const char *doing,
#define STANDARD_PARAM_DEF(name, type, format, strtolfn) \
int param_set_##name(const char *val, const struct kernel_param *kp) \
{ \
- param_check_unsafe(kp); \
return strtolfn(val, 0, (type *)kp-arg); \
} \
int param_get_##name(char *buffer, const struct kernel_param *kp) \
@@ -266,8 +276,6 @@ int param_set_charp(const char *val, const struct
kernel_param *kp)
return -ENOSPC;
}
- param_check_unsafe(kp);
-
maybe_kfree_parameter(*(char **)kp-arg);
/* This is a hack. We can't kmalloc in early boot, and we
@@ -305,8 +313,6 @@ EXPORT_SYMBOL(param_ops_charp);
/* Actually could be a bool or an int, for historical reasons. */
int param_set_bool(const char *val, const struct kernel_param *kp)
{
- param_check_unsafe(kp);
-
/* No equals means set... */
if (!val) val = 1;
@@ -336,8 +342,6 @@ int param_set_invbool(const char *val, const struct
kernel_param *kp)
bool boolval;
struct kernel_param dummy;
- param_check_unsafe(kp);
-
dummy.arg = boolval;
ret = param_set_bool(val, dummy);
if (ret == 0)
@@ -364,8 +368,6 @@ int param_set_bint(const char *val, const struct
kernel_param *kp)
bool v;
int ret;
- param_check_unsafe(kp);
-
/* Match bool exactly, by re-using it. */
boolkp = *kp;
boolkp.arg = v;
@@ -485,8 +487,6 @@ int param_set_copystring(const char *val, const struct
kernel_param *kp)
{
Re: [Intel-gfx] [PATCH 0/4] module: add support for unsafe, tainting parameters
Jani Nikula jani.nik...@intel.com writes:
This is a generic version of Daniel's patch [1] letting us have unsafe
module parameters (experimental, debugging, testing, etc.) that taint
the kernel when set. Quoting Daniel,
OK, I think the idea is fine, but we'll probably only want this for
a few types (eg. int and bool). So for the moment I prefer a more
naive approach.
Does this work for you?
Subject: module: add taint_int type
An int parameter which taints the kernel if set; i915 at least wants this.
Based-on-patches-by: Daniel Vetter daniel.vet...@ffwll.ch
Based-on-patches-by: Jani Nikula jani.nik...@intel.com
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 494f99e852da..99ba68206ba4 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -408,6 +408,10 @@ extern int param_set_bint(const char *val, const struct
kernel_param *kp);
#define param_get_bint param_get_int
#define param_check_bint param_check_int
+/* An int, which taints the kernel if set. */
+extern struct kernel_param_ops param_ops_taint_int;
+#define param_check_taint_int param_check_int
+
/**
* module_param_array - a parameter which is an array of some type
* @name: the name of the array variable
diff --git a/kernel/params.c b/kernel/params.c
index 34f527023794..3128218158cf 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -375,6 +375,20 @@ struct kernel_param_ops param_ops_bint = {
};
EXPORT_SYMBOL(param_ops_bint);
+static int param_set_taint_int(const char *val, const struct kernel_param *kp)
+{
+ pr_warn(Setting dangerous option %s - tainting kernel\n, kp-name);
+ add_taint(TAINT_USER, LOCKDEP_STILL_OK);
+
+ return param_set_int(val, kp);
+}
+
+struct kernel_param_ops param_ops_taint_int = {
+ .set = param_set_taint_int,
+ .get = param_get_int,
+};
+EXPORT_SYMBOL(param_ops_taint_int);
+
/* We break the rule and mangle the string. */
static int param_array(const char *name,
const char *val,
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/4] module: add support for unsafe, tainting parameters
On Wed, Aug 13, 2014 at 10:25 PM, Rusty Russell ru...@rustcorp.com.au wrote: Jani Nikula jani.nik...@intel.com writes: This is a generic version of Daniel's patch [1] letting us have unsafe module parameters (experimental, debugging, testing, etc.) that taint the kernel when set. Quoting Daniel, OK, I think the idea is fine, but we'll probably only want this for a few types (eg. int and bool). So for the moment I prefer a more naive approach. Does this work for you? Can you please discuss this with yourself from a few months back? We've done the general version since you suggested that just doing it for int is a bit lame ;-) And I actually agreed so asked Jani to look into that. http://mid.mail-archive.com/87r46gywul.fsf@rustcorp.com.au If this is a good idea, you can write a macro module_param_unsafe_named which is a general wrapper. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/4] module: add support for unsafe, tainting parameters
This is a generic version of Daniel's patch [1] letting us have unsafe module parameters (experimental, debugging, testing, etc.) that taint the kernel when set. Quoting Daniel, Users just love to set random piles of options since surely enabling all the experimental stuff helps. Later on we get bug reports because it all fell apart. Even more fun when it's labelled a regression when some change only just made the feature possible (e.g. stolen memory fixes suddenly making fbc possible). Make it clear that users are playing with fire here. Patches 1-3 add the core functionality, patch 4 is our use case as an example. BR, Jani. [1] http://lkml.kernel.org/r/1394011994-30604-1-git-send-email-daniel.vet...@ffwll.ch Jani Nikula (4): module: rename KERNEL_PARAM_FL_NOARG to avoid confusion module: make it possible to have unsafe, tainting module params module: add module_param_unsafe and module_param_named_unsafe drm/i915: taint the kernel if unsafe module parameters are set drivers/gpu/drm/i915/i915_params.c | 8 ++--- drivers/tty/serial/8250/8250_core.c | 2 +- include/linux/moduleparam.h | 64 +++-- kernel/module.c | 2 +- kernel/params.c | 17 -- security/apparmor/lsm.c | 4 +-- 6 files changed, 76 insertions(+), 21 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
