From: Kees Cook <keescook@chromium.org> To: Andrew Morton <akpm@linux-foundation.org> Cc: "Kees Cook" <keescook@chromium.org>, "Al Viro" <viro@zeniv.linux.org.uk>, "Richard Weinberger" <richard@nod.at>, "Eric W. Biederman" <ebiederm@xmission.com>, "Andy Lutomirski" <luto@amacapital.net>, "Robert Święcki" <robert@swiecki.net>, "Dmitry Vyukov" <dvyukov@google.com>, "David Howells" <dhowells@redhat.com>, "Miklos Szeredi" <mszeredi@suse.cz>, "Kostya Serebryany" <kcc@google.com>, "Alexander Potapenko" <glider@google.com>, "Eric Dumazet" <edumazet@google.com>, "Sasha Levin" <sasha.levin@oracle.com>, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-hardening@lists.openwall.com Subject: [PATCH 1/2] sysctl: expand use of proc_dointvec_minmax_sysadmin Date: Fri, 22 Jan 2016 14:39:04 -0800 [thread overview] Message-ID: <1453502345-30416-2-git-send-email-keescook@chromium.org> (raw) In-Reply-To: <1453502345-30416-1-git-send-email-keescook@chromium.org> Several sysctls expect a state where the highest value (in extra2) is locked once set for that boot. Yama does this, and kptr_restrict should be doing it. This extracts Yama's logic and adds it to the existing proc_dointvec_minmax_sysadmin, taking care to avoid the simple boolean states (which do not get locked). Since Yama wants to be checking a different capability, we build wrappers for both cases (CAP_SYS_ADMIN and CAP_SYS_PTRACE). Signed-off-by: Kees Cook <keescook@chromium.org> --- Documentation/sysctl/kernel.txt | 4 +++- include/linux/sysctl.h | 18 ++++++++++++++++++ kernel/sysctl.c | 34 +++++++++++++++++++++------------- security/yama/yama_lsm.c | 18 +----------------- 4 files changed, 43 insertions(+), 31 deletions(-) diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt index 73c6b1ef0e84..bbfc5e339a3d 100644 --- a/Documentation/sysctl/kernel.txt +++ b/Documentation/sysctl/kernel.txt @@ -385,7 +385,9 @@ to protect against uses of %pK in dmesg(8) if leaking kernel pointer values to unprivileged users is a concern. When kptr_restrict is set to (2), kernel pointers printed using -%pK will be replaced with 0's regardless of privileges. +%pK will be replaced with 0's regardless of privileges, and the value +will be locked at "2", so that the root user cannot remove this +restriction. ============================================================== diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h index fa7bc29925c9..f8f0b991fe3e 100644 --- a/include/linux/sysctl.h +++ b/include/linux/sysctl.h @@ -23,6 +23,7 @@ #include <linux/list.h> #include <linux/rcupdate.h> +#include <linux/capability.h> #include <linux/wait.h> #include <linux/rbtree.h> #include <uapi/linux/sysctl.h> @@ -55,6 +56,23 @@ extern int proc_doulongvec_ms_jiffies_minmax(struct ctl_table *table, int, void __user *, size_t *, loff_t *); extern int proc_do_large_bitmap(struct ctl_table *, int, void __user *, size_t *, loff_t *); +extern int proc_dointvec_minmax_cap(int cap, struct ctl_table *table, + int write, void __user *buffer, + size_t *lenp, loff_t *ppos); +static inline int proc_dointvec_minmax_cap_sysadmin(struct ctl_table *table, + int write, void __user *buffer, + size_t *lenp, loff_t *ppos) +{ + return proc_dointvec_minmax_cap(CAP_SYS_ADMIN, table, write, buffer, + lenp, ppos); +} +static inline int proc_dointvec_minmax_cap_ptrace(struct ctl_table *table, + int write, void __user *buffer, + size_t *lenp, loff_t *ppos) +{ + return proc_dointvec_minmax_cap(CAP_SYS_PTRACE, table, write, buffer, + lenp, ppos); +} /* * Register a set of sysctl names by calling register_sysctl_table diff --git a/kernel/sysctl.c b/kernel/sysctl.c index c810f8afdb7f..fc8899dd636d 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -181,11 +181,6 @@ static int proc_taint(struct ctl_table *table, int write, void __user *buffer, size_t *lenp, loff_t *ppos); #endif -#ifdef CONFIG_PRINTK -static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write, - void __user *buffer, size_t *lenp, loff_t *ppos); -#endif - static int proc_dointvec_minmax_coredump(struct ctl_table *table, int write, void __user *buffer, size_t *lenp, loff_t *ppos); #ifdef CONFIG_COREDUMP @@ -803,7 +798,7 @@ static struct ctl_table kern_table[] = { .data = &dmesg_restrict, .maxlen = sizeof(int), .mode = 0644, - .proc_handler = proc_dointvec_minmax_sysadmin, + .proc_handler = proc_dointvec_minmax_cap_sysadmin, .extra1 = &zero, .extra2 = &one, }, @@ -812,7 +807,7 @@ static struct ctl_table kern_table[] = { .data = &kptr_restrict, .maxlen = sizeof(int), .mode = 0644, - .proc_handler = proc_dointvec_minmax_sysadmin, + .proc_handler = proc_dointvec_minmax_cap_sysadmin, .extra1 = &zero, .extra2 = &two, }, @@ -2217,16 +2212,29 @@ static int proc_taint(struct ctl_table *table, int write, return err; } -#ifdef CONFIG_PRINTK -static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write, - void __user *buffer, size_t *lenp, loff_t *ppos) +int proc_dointvec_minmax_cap(int cap, struct ctl_table *table, int write, + void __user *buffer, size_t *lenp, loff_t *ppos) { - if (write && !capable(CAP_SYS_ADMIN)) + struct ctl_table table_copy; + int value; + + /* Require init capabilities to make changes. */ + if (write && !capable(cap)) return -EPERM; - return proc_dointvec_minmax(table, write, buffer, lenp, ppos); + /* + * To deal with const sysctl tables, we make a copy to perform + * the locking. When data is >1 and ==extra2, lock extra1 to + * extra2 to stop the value from being changed any further at + * runtime. + */ + table_copy = *table; + value = *(int *)table_copy.data; + if (value > 1 && value == *(int *)table_copy.extra2) + table_copy.extra1 = table_copy.extra2; + + return proc_dointvec_minmax(&table_copy, write, buffer, lenp, ppos); } -#endif struct do_proc_dointvec_minmax_conv_param { int *min; diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c index d3c19c970a06..3215afd08fbd 100644 --- a/security/yama/yama_lsm.c +++ b/security/yama/yama_lsm.c @@ -354,22 +354,6 @@ static struct security_hook_list yama_hooks[] = { }; #ifdef CONFIG_SYSCTL -static int yama_dointvec_minmax(struct ctl_table *table, int write, - void __user *buffer, size_t *lenp, loff_t *ppos) -{ - struct ctl_table table_copy; - - if (write && !capable(CAP_SYS_PTRACE)) - return -EPERM; - - /* Lock the max value if it ever gets set. */ - table_copy = *table; - if (*(int *)table_copy.data == *(int *)table_copy.extra2) - table_copy.extra1 = table_copy.extra2; - - return proc_dointvec_minmax(&table_copy, write, buffer, lenp, ppos); -} - static int zero; static int max_scope = YAMA_SCOPE_NO_ATTACH; @@ -385,7 +369,7 @@ static struct ctl_table yama_sysctl_table[] = { .data = &ptrace_scope, .maxlen = sizeof(int), .mode = 0644, - .proc_handler = yama_dointvec_minmax, + .proc_handler = proc_dointvec_minmax_cap_ptrace, .extra1 = &zero, .extra2 = &max_scope, }, -- 2.6.3
WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook@chromium.org> To: Andrew Morton <akpm@linux-foundation.org> Cc: "Kees Cook" <keescook@chromium.org>, "Al Viro" <viro@zeniv.linux.org.uk>, "Richard Weinberger" <richard@nod.at>, "Eric W. Biederman" <ebiederm@xmission.com>, "Andy Lutomirski" <luto@amacapital.net>, "Robert Święcki" <robert@swiecki.net>, "Dmitry Vyukov" <dvyukov@google.com>, "David Howells" <dhowells@redhat.com>, "Miklos Szeredi" <mszeredi@suse.cz>, "Kostya Serebryany" <kcc@google.com>, "Alexander Potapenko" <glider@google.com>, "Eric Dumazet" <edumazet@google.com>, "Sasha Levin" <sasha.levin@oracle.com>, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-hardening@lists.openwall.com Subject: [kernel-hardening] [PATCH 1/2] sysctl: expand use of proc_dointvec_minmax_sysadmin Date: Fri, 22 Jan 2016 14:39:04 -0800 [thread overview] Message-ID: <1453502345-30416-2-git-send-email-keescook@chromium.org> (raw) In-Reply-To: <1453502345-30416-1-git-send-email-keescook@chromium.org> Several sysctls expect a state where the highest value (in extra2) is locked once set for that boot. Yama does this, and kptr_restrict should be doing it. This extracts Yama's logic and adds it to the existing proc_dointvec_minmax_sysadmin, taking care to avoid the simple boolean states (which do not get locked). Since Yama wants to be checking a different capability, we build wrappers for both cases (CAP_SYS_ADMIN and CAP_SYS_PTRACE). Signed-off-by: Kees Cook <keescook@chromium.org> --- Documentation/sysctl/kernel.txt | 4 +++- include/linux/sysctl.h | 18 ++++++++++++++++++ kernel/sysctl.c | 34 +++++++++++++++++++++------------- security/yama/yama_lsm.c | 18 +----------------- 4 files changed, 43 insertions(+), 31 deletions(-) diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt index 73c6b1ef0e84..bbfc5e339a3d 100644 --- a/Documentation/sysctl/kernel.txt +++ b/Documentation/sysctl/kernel.txt @@ -385,7 +385,9 @@ to protect against uses of %pK in dmesg(8) if leaking kernel pointer values to unprivileged users is a concern. When kptr_restrict is set to (2), kernel pointers printed using -%pK will be replaced with 0's regardless of privileges. +%pK will be replaced with 0's regardless of privileges, and the value +will be locked at "2", so that the root user cannot remove this +restriction. ============================================================== diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h index fa7bc29925c9..f8f0b991fe3e 100644 --- a/include/linux/sysctl.h +++ b/include/linux/sysctl.h @@ -23,6 +23,7 @@ #include <linux/list.h> #include <linux/rcupdate.h> +#include <linux/capability.h> #include <linux/wait.h> #include <linux/rbtree.h> #include <uapi/linux/sysctl.h> @@ -55,6 +56,23 @@ extern int proc_doulongvec_ms_jiffies_minmax(struct ctl_table *table, int, void __user *, size_t *, loff_t *); extern int proc_do_large_bitmap(struct ctl_table *, int, void __user *, size_t *, loff_t *); +extern int proc_dointvec_minmax_cap(int cap, struct ctl_table *table, + int write, void __user *buffer, + size_t *lenp, loff_t *ppos); +static inline int proc_dointvec_minmax_cap_sysadmin(struct ctl_table *table, + int write, void __user *buffer, + size_t *lenp, loff_t *ppos) +{ + return proc_dointvec_minmax_cap(CAP_SYS_ADMIN, table, write, buffer, + lenp, ppos); +} +static inline int proc_dointvec_minmax_cap_ptrace(struct ctl_table *table, + int write, void __user *buffer, + size_t *lenp, loff_t *ppos) +{ + return proc_dointvec_minmax_cap(CAP_SYS_PTRACE, table, write, buffer, + lenp, ppos); +} /* * Register a set of sysctl names by calling register_sysctl_table diff --git a/kernel/sysctl.c b/kernel/sysctl.c index c810f8afdb7f..fc8899dd636d 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -181,11 +181,6 @@ static int proc_taint(struct ctl_table *table, int write, void __user *buffer, size_t *lenp, loff_t *ppos); #endif -#ifdef CONFIG_PRINTK -static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write, - void __user *buffer, size_t *lenp, loff_t *ppos); -#endif - static int proc_dointvec_minmax_coredump(struct ctl_table *table, int write, void __user *buffer, size_t *lenp, loff_t *ppos); #ifdef CONFIG_COREDUMP @@ -803,7 +798,7 @@ static struct ctl_table kern_table[] = { .data = &dmesg_restrict, .maxlen = sizeof(int), .mode = 0644, - .proc_handler = proc_dointvec_minmax_sysadmin, + .proc_handler = proc_dointvec_minmax_cap_sysadmin, .extra1 = &zero, .extra2 = &one, }, @@ -812,7 +807,7 @@ static struct ctl_table kern_table[] = { .data = &kptr_restrict, .maxlen = sizeof(int), .mode = 0644, - .proc_handler = proc_dointvec_minmax_sysadmin, + .proc_handler = proc_dointvec_minmax_cap_sysadmin, .extra1 = &zero, .extra2 = &two, }, @@ -2217,16 +2212,29 @@ static int proc_taint(struct ctl_table *table, int write, return err; } -#ifdef CONFIG_PRINTK -static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write, - void __user *buffer, size_t *lenp, loff_t *ppos) +int proc_dointvec_minmax_cap(int cap, struct ctl_table *table, int write, + void __user *buffer, size_t *lenp, loff_t *ppos) { - if (write && !capable(CAP_SYS_ADMIN)) + struct ctl_table table_copy; + int value; + + /* Require init capabilities to make changes. */ + if (write && !capable(cap)) return -EPERM; - return proc_dointvec_minmax(table, write, buffer, lenp, ppos); + /* + * To deal with const sysctl tables, we make a copy to perform + * the locking. When data is >1 and ==extra2, lock extra1 to + * extra2 to stop the value from being changed any further at + * runtime. + */ + table_copy = *table; + value = *(int *)table_copy.data; + if (value > 1 && value == *(int *)table_copy.extra2) + table_copy.extra1 = table_copy.extra2; + + return proc_dointvec_minmax(&table_copy, write, buffer, lenp, ppos); } -#endif struct do_proc_dointvec_minmax_conv_param { int *min; diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c index d3c19c970a06..3215afd08fbd 100644 --- a/security/yama/yama_lsm.c +++ b/security/yama/yama_lsm.c @@ -354,22 +354,6 @@ static struct security_hook_list yama_hooks[] = { }; #ifdef CONFIG_SYSCTL -static int yama_dointvec_minmax(struct ctl_table *table, int write, - void __user *buffer, size_t *lenp, loff_t *ppos) -{ - struct ctl_table table_copy; - - if (write && !capable(CAP_SYS_PTRACE)) - return -EPERM; - - /* Lock the max value if it ever gets set. */ - table_copy = *table; - if (*(int *)table_copy.data == *(int *)table_copy.extra2) - table_copy.extra1 = table_copy.extra2; - - return proc_dointvec_minmax(&table_copy, write, buffer, lenp, ppos); -} - static int zero; static int max_scope = YAMA_SCOPE_NO_ATTACH; @@ -385,7 +369,7 @@ static struct ctl_table yama_sysctl_table[] = { .data = &ptrace_scope, .maxlen = sizeof(int), .mode = 0644, - .proc_handler = yama_dointvec_minmax, + .proc_handler = proc_dointvec_minmax_cap_ptrace, .extra1 = &zero, .extra2 = &max_scope, }, -- 2.6.3
next prev parent reply other threads:[~2016-01-22 22:39 UTC|newest] Thread overview: 80+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-01-22 22:39 [PATCH 0/2] sysctl: allow CLONE_NEWUSER to be disabled Kees Cook 2016-01-22 22:39 ` [kernel-hardening] " Kees Cook 2016-01-22 22:39 ` Kees Cook [this message] 2016-01-22 22:39 ` [kernel-hardening] [PATCH 1/2] sysctl: expand use of proc_dointvec_minmax_sysadmin Kees Cook 2016-01-23 3:10 ` Eric W. Biederman 2016-01-23 3:10 ` [kernel-hardening] " Eric W. Biederman 2016-01-23 22:25 ` Jann Horn 2016-01-24 1:20 ` Eric W. Biederman 2016-01-24 1:43 ` Al Viro 2016-01-24 1:56 ` Jann Horn 2016-01-24 6:02 ` Eric W. Biederman 2016-01-24 6:32 ` Jann Horn 2016-01-24 6:44 ` Eric W. Biederman 2016-01-22 22:39 ` [PATCH 2/2] sysctl: allow CLONE_NEWUSER to be disabled Kees Cook 2016-01-22 22:39 ` [kernel-hardening] " Kees Cook 2016-01-22 22:47 ` Robert Święcki 2016-01-22 22:47 ` [kernel-hardening] " Robert Święcki 2016-01-22 22:50 ` Kees Cook 2016-01-22 22:50 ` [kernel-hardening] " Kees Cook 2016-01-22 22:55 ` Robert Święcki 2016-01-22 22:55 ` [kernel-hardening] " Robert Święcki 2016-01-22 23:00 ` Kees Cook 2016-01-22 23:00 ` [kernel-hardening] " Kees Cook 2016-01-23 0:44 ` Serge Hallyn 2016-01-23 0:44 ` [kernel-hardening] " Serge Hallyn 2016-01-23 0:44 ` Serge Hallyn 2016-01-23 0:44 ` [kernel-hardening] " Serge Hallyn 2016-01-23 0:59 ` Ben Hutchings 2016-01-24 20:59 ` Kees Cook 2016-01-24 20:59 ` Kees Cook 2016-01-24 22:20 ` Andy Lutomirski 2016-01-25 18:51 ` Kees Cook 2016-01-22 22:49 ` [PATCH 0/2] " Richard Weinberger 2016-01-22 22:49 ` [kernel-hardening] " Richard Weinberger 2016-01-23 3:02 ` Eric W. Biederman 2016-01-23 3:02 ` [kernel-hardening] " Eric W. Biederman 2016-01-24 20:57 ` Kees Cook 2016-01-24 20:57 ` [kernel-hardening] " Kees Cook 2016-01-26 7:38 ` Serge Hallyn 2016-01-24 22:22 ` Andy Lutomirski 2016-01-24 22:22 ` [kernel-hardening] " Andy Lutomirski 2016-01-25 18:51 ` Kees Cook 2016-01-25 18:51 ` [kernel-hardening] " Kees Cook 2016-01-25 18:53 ` Andy Lutomirski 2016-01-25 18:53 ` [kernel-hardening] " Andy Lutomirski 2016-01-25 18:56 ` Kees Cook 2016-01-25 18:56 ` [kernel-hardening] " Kees Cook 2016-01-25 19:33 ` Eric W. Biederman 2016-01-25 19:33 ` [kernel-hardening] " Eric W. Biederman 2016-01-25 22:34 ` Kees Cook 2016-01-25 22:34 ` [kernel-hardening] " Kees Cook 2016-01-25 23:33 ` Andy Lutomirski 2016-01-25 23:33 ` [kernel-hardening] " Andy Lutomirski 2016-01-26 2:27 ` Daniel Micay 2016-01-26 4:57 ` Eric W. Biederman 2016-01-26 4:57 ` [kernel-hardening] " Eric W. Biederman 2016-01-26 14:38 ` Josh Boyer 2016-01-26 14:38 ` [kernel-hardening] " Josh Boyer 2016-01-26 14:46 ` Austin S. Hemmelgarn 2016-01-26 14:46 ` [kernel-hardening] " Austin S. Hemmelgarn 2016-01-26 14:56 ` Josh Boyer 2016-01-26 14:56 ` [kernel-hardening] " Josh Boyer 2016-01-26 17:20 ` Serge Hallyn 2016-01-26 19:56 ` Josh Boyer 2016-01-26 20:11 ` Austin S. Hemmelgarn 2016-01-26 17:15 ` Serge Hallyn 2016-01-26 18:09 ` Austin S. Hemmelgarn 2016-01-26 18:27 ` Andy Lutomirski 2016-01-26 18:45 ` Austin S. Hemmelgarn 2016-01-26 23:15 ` Kees Cook 2016-01-26 23:13 ` Kees Cook 2016-01-27 10:27 ` Eric W. Biederman 2016-01-27 12:32 ` Austin S. Hemmelgarn 2016-01-28 14:41 ` Robert Święcki 2016-01-28 14:41 ` Robert Święcki 2016-01-26 23:47 ` Josh Boyer 2016-01-26 16:37 ` Kees Cook 2016-01-26 16:37 ` [kernel-hardening] " Kees Cook 2016-01-28 8:56 ` Serge E. Hallyn 2016-01-28 12:53 ` Austin S. Hemmelgarn
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1453502345-30416-2-git-send-email-keescook@chromium.org \ --to=keescook@chromium.org \ --cc=akpm@linux-foundation.org \ --cc=dhowells@redhat.com \ --cc=dvyukov@google.com \ --cc=ebiederm@xmission.com \ --cc=edumazet@google.com \ --cc=glider@google.com \ --cc=kcc@google.com \ --cc=kernel-hardening@lists.openwall.com \ --cc=linux-doc@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=luto@amacapital.net \ --cc=mszeredi@suse.cz \ --cc=richard@nod.at \ --cc=robert@swiecki.net \ --cc=sasha.levin@oracle.com \ --cc=viro@zeniv.linux.org.uk \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.