param: do not set store func without write perm

When a module_param is defined without DAC write permissions, it can
still be changed at runtime and updated. Drivers using a 0444 permission
may be surprised that these values can still be changed.

For drivers that want to allow updates, any S_IW* flag will set the
"store" function as before. Drivers without S_IW* flags will have the
"store" function unset, unforcing a read-only value. Drivers that wish
neither "store" nor "get" can continue to use "0" for perms to stay out
of sysfs entirely.

Old behavior:
  # cd /sys/module/snd/parameters
  # ls -l
  total 0
  -r--r--r-- 1 root root 4096 Dec 11 13:55 cards_limit
  -r--r--r-- 1 root root 4096 Dec 11 13:55 major
  -r--r--r-- 1 root root 4096 Dec 11 13:55 slots
  # cat major
  116
  # echo -1 > major
  -bash: major: Permission denied
  # chmod u+w major
  # echo -1 > major
  # cat major
  -1

New behavior:
  ...
  # chmod u+w major
  # echo -1 > major
  -bash: echo: write error: Input/output error

Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/kernel/params.c b/kernel/params.c
index 795321a..0af9b2c 100644 (file)
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -645,7 +645,9 @@ static __modinit int add_sysfs_param(struct module_kobject *mk,
        sysfs_attr_init(&mk->mp->attrs[mk->mp->num].mattr.attr);
        mk->mp->attrs[mk->mp->num].param = kp;
        mk->mp->attrs[mk->mp->num].mattr.show = param_attr_show;
-       mk->mp->attrs[mk->mp->num].mattr.store = param_attr_store;
+       /* Do not allow runtime DAC changes to make param writable. */
+       if ((kp->perm & (S_IWUSR | S_IWGRP | S_IWOTH)) != 0)
+               mk->mp->attrs[mk->mp->num].mattr.store = param_attr_store;
        mk->mp->attrs[mk->mp->num].mattr.attr.name = (char *)name;
        mk->mp->attrs[mk->mp->num].mattr.attr.mode = kp->perm;
        mk->mp->num++;