linux-kernel - [PATCH] param: do not set store func without write perm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141211222144.GA7070@www.outflux.net>
Date:	Thu, 11 Dec 2014 14:21:45 -0800
From:	Kees Cook <keescook@...omium.org>
To:	linux-kernel@...r.kernel.org
Cc:	Rusty Russell <rusty@...tcorp.com.au>,
	Christoph Hellwig <hch@...radead.org>,
	Jani Nikula <jani.nikula@...el.com>,
	Hannes Reinecke <hare@...e.de>,
	Geert Uytterhoeven <geert@...ux-m68k.org>
Subject: [PATCH] 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@...omium.org>
---
 kernel/params.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/params.c b/kernel/params.c
index db97b791390f..fd50ce9f1bbf 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -647,7 +647,9 @@ static __modinit int add_sysfs_param(struct module_kobject *mk,
 	sysfs_attr_init(&new->attrs[num].mattr.attr);
 	new->attrs[num].param = kp;
 	new->attrs[num].mattr.show = param_attr_show;
-	new->attrs[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)
+		new->attrs[num].mattr.store = param_attr_store;
 	new->attrs[num].mattr.attr.name = (char *)name;
 	new->attrs[num].mattr.attr.mode = kp->perm;
 	new->num = num+1;
-- 
1.9.1


-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/