| Message ID | 1429739711-9415-4-git-send-email-mcgrof@do-not-panic.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
Hello, On Wed, Apr 22, 2015 at 02:55:06PM -0700, Luis R. Rodriguez wrote: > +int param_set_bool_enable_only(const char *val, const struct kernel_param *kp) > +{ > + int err = 0; > + bool new_value; > + bool orig_value = *(bool *)kp->arg; > + struct kernel_param dummy_kp = *kp; > + > + dummy_kp.arg = &new_value; > + > + err = param_set_bool(val, &dummy_kp); > + if (err) > + return err; > + > + /* Don't let them unset it once it's set! */ > + if (!new_value && orig_value) > + return -EROFS; I know that this was moved from another place but as we're making it generic now I'm a bit curious about -EROFS. Wouldn't -EINVAL be a more conventional choice here? Thanks.
On Thu, Apr 23, 2015 at 11:22:49AM -0400, Tejun Heo wrote: > Hello, > > On Wed, Apr 22, 2015 at 02:55:06PM -0700, Luis R. Rodriguez wrote: > > +int param_set_bool_enable_only(const char *val, const struct kernel_param *kp) > > +{ > > + int err = 0; > > + bool new_value; > > + bool orig_value = *(bool *)kp->arg; > > + struct kernel_param dummy_kp = *kp; > > + > > + dummy_kp.arg = &new_value; > > + > > + err = param_set_bool(val, &dummy_kp); > > + if (err) > > + return err; > > + > > + /* Don't let them unset it once it's set! */ > > + if (!new_value && orig_value) > > + return -EROFS; > > I know that this was moved from another place but as we're making it > generic now I'm a bit curious about -EROFS. Wouldn't -EINVAL be a > more conventional choice here? Let's see, I tested to see what errors we get: Userspace: -EROFS: mcgrof@ergon ~/devel/test-sig-force-general $ sudo insmod ./hello.ko test_sig_enforce=1 insmod: ERROR: could not insert module ./hello.ko: Read-only file system -EINVAL: mcgrof@ergon ~/devel/test-sig-force-general $ sudo insmod ./hello.ko test_sig_enforce=1 insmod: ERROR: could not insert module ./hello.ko: Invalid parameters Kernel space: Both of these returns yield this on the kernel ring buffer: mcgrof@ergon ~/devel/test-sig-force-general $ sudo dmesg -c [124677.202875] hello: `1' invalid for parameter `test_sig_enforce' Alternative candidates: #define EBADRQC 56 /* Invalid request code */ #define EOPNOTSUPP 95 /* Operation not supported on transport endpoint */ Perhaps EOPNOTSUPP is best? Luis -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h index 5d0f4d9..7e00799 100644 --- a/include/linux/moduleparam.h +++ b/include/linux/moduleparam.h @@ -427,6 +427,12 @@ extern int param_set_bool(const char *val, const struct kernel_param *kp); extern int param_get_bool(char *buffer, const struct kernel_param *kp); #define param_check_bool(name, p) __param_check(name, p, bool) +extern const struct kernel_param_ops param_ops_bool_enable_only; +extern int param_set_bool_enable_only(const char *val, + const struct kernel_param *kp); +/* getter is the same as for the regular bool */ +#define param_check_bool_enable_only param_check_bool + extern const struct kernel_param_ops param_ops_invbool; extern int param_set_invbool(const char *val, const struct kernel_param *kp); extern int param_get_invbool(char *buffer, const struct kernel_param *kp); diff --git a/kernel/module.c b/kernel/module.c index de12c4a..43a1ef3 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -111,37 +111,6 @@ static bool sig_enforce = true; #else static bool sig_enforce = false; -static int param_set_bool_enable_only(const char *val, - const struct kernel_param *kp) -{ - int err = 0; - bool new_value; - bool orig_value = *(bool *)kp->arg; - struct kernel_param dummy_kp = *kp; - - dummy_kp.arg = &new_value; - - err = param_set_bool(val, &dummy_kp); - if (err) - return err; - - /* Don't let them unset it once it's set! */ - if (!new_value && orig_value) - return -EROFS; - - if (new_value) - err = param_set_bool(val, kp); - - return err; -} - -static const struct kernel_param_ops param_ops_bool_enable_only = { - .flags = KERNEL_PARAM_OPS_FL_NOARG, - .set = param_set_bool_enable_only, - .get = param_get_bool, -}; -#define param_check_bool_enable_only param_check_bool - module_param(sig_enforce, bool_enable_only, 0644); #endif /* !CONFIG_MODULE_SIG_FORCE */ #endif /* CONFIG_MODULE_SIG */ diff --git a/kernel/params.c b/kernel/params.c index b7635c0..324624e 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -335,6 +335,36 @@ const struct kernel_param_ops param_ops_bool = { }; EXPORT_SYMBOL(param_ops_bool); +int param_set_bool_enable_only(const char *val, const struct kernel_param *kp) +{ + int err = 0; + bool new_value; + bool orig_value = *(bool *)kp->arg; + struct kernel_param dummy_kp = *kp; + + dummy_kp.arg = &new_value; + + err = param_set_bool(val, &dummy_kp); + if (err) + return err; + + /* Don't let them unset it once it's set! */ + if (!new_value && orig_value) + return -EROFS; + + if (new_value) + err = param_set_bool(val, kp); + + return err; +} +EXPORT_SYMBOL_GPL(param_set_bool_enable_only); + +const struct kernel_param_ops param_ops_bool_enable_only = { + .flags = KERNEL_PARAM_OPS_FL_NOARG, + .set = param_set_bool_enable_only, + .get = param_get_bool, +}; + /* This one must be bool. */ int param_set_invbool(const char *val, const struct kernel_param *kp) {