Re: [Intel-gfx] [PATCH 0/4] module: add support for unsafe, tainting parameters

2014-08-21 Thread Jani Nikula
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

2014-08-20 Thread Rusty Russell
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

2014-08-13 Thread Rusty Russell
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

2014-08-13 Thread Daniel Vetter
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