* [PATCH 0/6] file capabilities cleanups: introduction
@ 2008-09-27 2:27 Serge E. Hallyn
2008-09-27 2:27 ` [PATCH 1/6] file capabilities: add no_file_caps switch (v3) Serge E. Hallyn
0 siblings, 1 reply; 17+ messages in thread
From: Serge E. Hallyn @ 2008-09-27 2:27 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-security-module
Following is a set of file capabilities cleanups. The first
two patches are a repost of my previous patches which
introduce a no_file_caps boot option, and remove the
CONFIG_SECURITY_FILE_CAPABILITIES config option. The rest
of the patches both clean up some of the capabilities code
and reduce the kernel size (since enabling file capabilities
grew it).
Andrew Morgan, if you have a moment, please do take a close look
and make sure I'm not doing anything stupid/wrong in the cleanups!
However ltp shows no difference with and without the patchset.
Following are the kernel sizes after some of the patches.
original, pre-patch, with file capabilities compiled out:
text data bss dec hex filename
4188468 234432 316472 4739372 48512c vmlinux
original, pre-patch, with file capabilities compiled in:
4189356 234432 316472 4740260 4854a4 vmlinux
plain with fcaps always-on:
4189392 234456 316472 4740320 4854e0 vmlinux
with non-inline cap_safe_nice:
4189112 234456 316472 4740040 4853c8 vmlinux
with cleaned-up setcap:
4189120 234456 316472 4740048 4853d0 vmlinux
with needless check for target!=current removed from cap_capset:
4189104 234456 316472 4740032 4853c0 vmlinux
with needless(?) bprm_clear_caps calls removed:
4189088 234456 316472 4740016 4853b0 vmlinux
thanks,
-serge
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/6] file capabilities: add no_file_caps switch (v3)
2008-09-27 2:27 [PATCH 0/6] file capabilities cleanups: introduction Serge E. Hallyn
@ 2008-09-27 2:27 ` Serge E. Hallyn
2008-09-27 2:27 ` [PATCH 2/6] file capabilities: remove CONFIG_SECURITY_FILE_CAPABILITIES Serge E. Hallyn
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Serge E. Hallyn @ 2008-09-27 2:27 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-security-module, Serge Hallyn
From: Serge Hallyn <serue@us.ibm.com>
Add a no_file_caps boot option when file capabilities are
compiled into the kernel (CONFIG_SECURITY_FILE_CAPABILITIES=y).
This allows distributions to ship a kernel with file capabilities
compiled in, without forcing users to use (and understand and
trust) them.
When no_file_caps is specified at boot, then when a process executes
a file, any file capabilities stored with that file will not be
used in the calculation of the process' new capability sets.
This means that booting with the no_file_caps boot option will
not be the same as booting a kernel with file capabilities
compiled out - in particular a task with CAP_SETPCAP will not
have any chance of passing capabilities to another task (which
isn't "really" possible anyway, and which may soon by killed
altogether by David Howells in any case), and it will instead
be able to put new capabilities in its pI. However since fI
will always be empty and pI is masked with fI, it gains the
task nothing.
We also support the extra prctl options, setting securebits and
dropping capabilities from the per-process bounding set.
The other remaining difference is that killpriv, task_setscheduler,
setioprio, and setnice will continue to be hooked. That will
be noticable in the case where a root task changed its uid
while keeping some caps, and another task owned by the new uid
tries to change settings for the more privileged task.
Changelog:
Sep 23 2008: nixed file_caps_enabled when file caps are
not compiled in as it isn't used.
Document no_file_caps in kernel-parameters.txt.
Signed-off-by: Serge Hallyn <serue@us.ibm.com>
Acked-by: Andrew G. Morgan <morgan@kernel.org>
---
Documentation/kernel-parameters.txt | 4 ++++
include/linux/capability.h | 3 +++
kernel/capability.c | 11 +++++++++++
security/commoncap.c | 5 +++++
4 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 1150444..cbb04bf 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1393,6 +1393,10 @@ and is between 256 and 4096 characters. It is defined in the file
instruction doesn't work correctly and not to
use it.
+ no_file_caps Tells the kernel not to honor file capabilities. The
+ only way then for a file to be executed with privilege
+ is to be setuid root or executed by root.
+
nohalt [IA-64] Tells the kernel not to use the power saving
function PAL_HALT_LIGHT when idle. This increases
power-consumption. On the positive side, it reduces
diff --git a/include/linux/capability.h b/include/linux/capability.h
index 9d1fe30..5bc145b 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -68,6 +68,9 @@ typedef struct __user_cap_data_struct {
#define VFS_CAP_U32 VFS_CAP_U32_2
#define VFS_CAP_REVISION VFS_CAP_REVISION_2
+#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
+extern int file_caps_enabled;
+#endif
struct vfs_cap_data {
__le32 magic_etc; /* Little endian */
diff --git a/kernel/capability.c b/kernel/capability.c
index 33e51e7..e13a685 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -33,6 +33,17 @@ EXPORT_SYMBOL(__cap_empty_set);
EXPORT_SYMBOL(__cap_full_set);
EXPORT_SYMBOL(__cap_init_eff_set);
+#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
+int file_caps_enabled = 1;
+
+static int __init file_caps_disable(char *str)
+{
+ file_caps_enabled = 0;
+ return 1;
+}
+__setup("no_file_caps", file_caps_disable);
+#endif
+
/*
* More recent versions of libcap are available from:
*
diff --git a/security/commoncap.c b/security/commoncap.c
index e4c4b3f..5f1fec0 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -279,6 +279,11 @@ static int get_file_caps(struct linux_binprm *bprm)
struct vfs_cap_data vcaps;
struct inode *inode;
+ if (!file_caps_enabled) {
+ bprm_clear_caps(bprm);
+ return 0;
+ }
+
if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID) {
bprm_clear_caps(bprm);
return 0;
--
1.5.1.1.GIT
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/6] file capabilities: remove CONFIG_SECURITY_FILE_CAPABILITIES
2008-09-27 2:27 ` [PATCH 1/6] file capabilities: add no_file_caps switch (v3) Serge E. Hallyn
@ 2008-09-27 2:27 ` Serge E. Hallyn
2008-09-27 4:25 ` Andrew G. Morgan
2008-09-27 2:27 ` [PATCH 3/6] file capabilities: uninline cap_safe_nice Serge E. Hallyn
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Serge E. Hallyn @ 2008-09-27 2:27 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-security-module, Serge Hallyn
From: Serge Hallyn <serue@us.ibm.com>
Remove the option to compile the kernel without file capabilities. Not
compiling file capabilities actually makes the kernel less safe, as it
includes the possibility for a task changing another task's capabilities.
Some are concerned that userspace tools (and user education) are not
up to the task of properly configuring file capabilities on a system.
For those cases, there is now the ability to boot with the no_file_caps
boot option. This will prevent file capabilities from being used in
the capabilities recalculation at exec, but will not change the rest
of the kernel behavior which used to be switchable using the
CONFIG_SECURITY_FILE_CAPABILITIES option.
Signed-off-by: Serge Hallyn <serue@us.ibm.com>
---
fs/open.c | 8 --
include/linux/capability.h | 2 -
include/linux/init_task.h | 4 -
kernel/capability.c | 158 --------------------------------------------
security/Kconfig | 9 ---
security/commoncap.c | 53 ---------------
6 files changed, 0 insertions(+), 234 deletions(-)
diff --git a/fs/open.c b/fs/open.c
index 07da935..6e1cd6e 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -444,14 +444,6 @@ asmlinkage long sys_faccessat(int dfd, const char __user *filename, int mode)
/*
* Clear the capabilities if we switch to a non-root user
*/
-#ifndef CONFIG_SECURITY_FILE_CAPABILITIES
- /*
- * FIXME: There is a race here against sys_capset. The
- * capabilities can change yet we will restore the old
- * value below. We should hold task_capabilities_lock,
- * but we cannot because user_path_at can sleep.
- */
-#endif /* ndef CONFIG_SECURITY_FILE_CAPABILITIES */
if (current->uid)
old_cap = cap_set_effective(__cap_empty_set);
else
diff --git a/include/linux/capability.h b/include/linux/capability.h
index 5bc145b..07a9ada 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -68,9 +68,7 @@ typedef struct __user_cap_data_struct {
#define VFS_CAP_U32 VFS_CAP_U32_2
#define VFS_CAP_REVISION VFS_CAP_REVISION_2
-#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
extern int file_caps_enabled;
-#endif
struct vfs_cap_data {
__le32 magic_etc; /* Little endian */
diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index 021d8e7..7c177b9 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -102,16 +102,12 @@ extern struct group_info init_groups;
#define INIT_IDS
#endif
-#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
/*
* Because of the reduced scope of CAP_SETPCAP when filesystem
* capabilities are in effect, it is safe to allow CAP_SETPCAP to
* be available in the default configuration.
*/
# define CAP_INIT_BSET CAP_FULL_SET
-#else
-# define CAP_INIT_BSET CAP_INIT_EFF_SET
-#endif
/*
* INIT_TASK is used to set up the first task table, touch at
diff --git a/kernel/capability.c b/kernel/capability.c
index e13a685..d39c989 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -33,7 +33,6 @@ EXPORT_SYMBOL(__cap_empty_set);
EXPORT_SYMBOL(__cap_full_set);
EXPORT_SYMBOL(__cap_init_eff_set);
-#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
int file_caps_enabled = 1;
static int __init file_caps_disable(char *str)
@@ -42,7 +41,6 @@ static int __init file_caps_disable(char *str)
return 1;
}
__setup("no_file_caps", file_caps_disable);
-#endif
/*
* More recent versions of libcap are available from:
@@ -126,160 +124,6 @@ static int cap_validate_magic(cap_user_header_t header, unsigned *tocopy)
return 0;
}
-#ifndef CONFIG_SECURITY_FILE_CAPABILITIES
-
-/*
- * Without filesystem capability support, we nominally support one process
- * setting the capabilities of another
- */
-static inline int cap_get_target_pid(pid_t pid, kernel_cap_t *pEp,
- kernel_cap_t *pIp, kernel_cap_t *pPp)
-{
- struct task_struct *target;
- int ret;
-
- spin_lock(&task_capability_lock);
- read_lock(&tasklist_lock);
-
- if (pid && pid != task_pid_vnr(current)) {
- target = find_task_by_vpid(pid);
- if (!target) {
- ret = -ESRCH;
- goto out;
- }
- } else
- target = current;
-
- ret = security_capget(target, pEp, pIp, pPp);
-
-out:
- read_unlock(&tasklist_lock);
- spin_unlock(&task_capability_lock);
-
- return ret;
-}
-
-/*
- * cap_set_pg - set capabilities for all processes in a given process
- * group. We call this holding task_capability_lock and tasklist_lock.
- */
-static inline int cap_set_pg(int pgrp_nr, kernel_cap_t *effective,
- kernel_cap_t *inheritable,
- kernel_cap_t *permitted)
-{
- struct task_struct *g, *target;
- int ret = -EPERM;
- int found = 0;
- struct pid *pgrp;
-
- spin_lock(&task_capability_lock);
- read_lock(&tasklist_lock);
-
- pgrp = find_vpid(pgrp_nr);
- do_each_pid_task(pgrp, PIDTYPE_PGID, g) {
- target = g;
- while_each_thread(g, target) {
- if (!security_capset_check(target, effective,
- inheritable, permitted)) {
- security_capset_set(target, effective,
- inheritable, permitted);
- ret = 0;
- }
- found = 1;
- }
- } while_each_pid_task(pgrp, PIDTYPE_PGID, g);
-
- read_unlock(&tasklist_lock);
- spin_unlock(&task_capability_lock);
-
- if (!found)
- ret = 0;
- return ret;
-}
-
-/*
- * cap_set_all - set capabilities for all processes other than init
- * and self. We call this holding task_capability_lock and tasklist_lock.
- */
-static inline int cap_set_all(kernel_cap_t *effective,
- kernel_cap_t *inheritable,
- kernel_cap_t *permitted)
-{
- struct task_struct *g, *target;
- int ret = -EPERM;
- int found = 0;
-
- spin_lock(&task_capability_lock);
- read_lock(&tasklist_lock);
-
- do_each_thread(g, target) {
- if (target == current
- || is_container_init(target->group_leader))
- continue;
- found = 1;
- if (security_capset_check(target, effective, inheritable,
- permitted))
- continue;
- ret = 0;
- security_capset_set(target, effective, inheritable, permitted);
- } while_each_thread(g, target);
-
- read_unlock(&tasklist_lock);
- spin_unlock(&task_capability_lock);
-
- if (!found)
- ret = 0;
-
- return ret;
-}
-
-/*
- * Given the target pid does not refer to the current process we
- * need more elaborate support... (This support is not present when
- * filesystem capabilities are configured.)
- */
-static inline int do_sys_capset_other_tasks(pid_t pid, kernel_cap_t *effective,
- kernel_cap_t *inheritable,
- kernel_cap_t *permitted)
-{
- struct task_struct *target;
- int ret;
-
- if (!capable(CAP_SETPCAP))
- return -EPERM;
-
- if (pid == -1) /* all procs other than current and init */
- return cap_set_all(effective, inheritable, permitted);
-
- else if (pid < 0) /* all procs in process group */
- return cap_set_pg(-pid, effective, inheritable, permitted);
-
- /* target != current */
- spin_lock(&task_capability_lock);
- read_lock(&tasklist_lock);
-
- target = find_task_by_vpid(pid);
- if (!target)
- ret = -ESRCH;
- else {
- ret = security_capset_check(target, effective, inheritable,
- permitted);
-
- /* having verified that the proposed changes are legal,
- we now put them into effect. */
- if (!ret)
- security_capset_set(target, effective, inheritable,
- permitted);
- }
-
- read_unlock(&tasklist_lock);
- spin_unlock(&task_capability_lock);
-
- return ret;
-}
-
-#else /* ie., def CONFIG_SECURITY_FILE_CAPABILITIES */
-
/*
* If we have configured with filesystem capability support, then the
* only thing that can change the capabilities of the current process
@@ -327,8 +171,6 @@ static inline int do_sys_capset_other_tasks(pid_t pid,
return -EPERM;
}
-#endif /* ie., ndef CONFIG_SECURITY_FILE_CAPABILITIES */
-
/*
* Atomically modify the effective capabilities returning the original
* value. No permission check is performed here - it is assumed that the
diff --git a/security/Kconfig b/security/Kconfig
index 5592939..2cf7fb9 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -73,15 +73,6 @@ config SECURITY_NETWORK_XFRM
IPSec.
If you are unsure how to answer this question, answer N.
-config SECURITY_FILE_CAPABILITIES
- bool "File POSIX Capabilities"
- default n
- help
- This enables filesystem capabilities, allowing you to give
- binaries a subset of root's powers without using setuid 0.
-
- If in doubt, answer N.
-
config SECURITY_ROOTPLUG
bool "Root Plug Support"
depends on USB=y && SECURITY
diff --git a/security/commoncap.c b/security/commoncap.c
index 5f1fec0..9bfef94 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -93,8 +93,6 @@ int cap_capget (struct task_struct *target, kernel_cap_t *effective,
return 0;
}
-#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
-
static inline int cap_block_setpcap(struct task_struct *target)
{
/*
@@ -116,17 +114,6 @@ static inline int cap_inh_is_capped(void)
static inline int cap_limit_ptraced_target(void) { return 1; }
-#else /* ie., ndef CONFIG_SECURITY_FILE_CAPABILITIES */
-
-static inline int cap_block_setpcap(struct task_struct *t) { return 0; }
-static inline int cap_inh_is_capped(void) { return 1; }
-static inline int cap_limit_ptraced_target(void)
-{
- return !capable(CAP_SETPCAP);
-}
-
-#endif /* def CONFIG_SECURITY_FILE_CAPABILITIES */
-
int cap_capset_check (struct task_struct *target, kernel_cap_t *effective,
kernel_cap_t *inheritable, kernel_cap_t *permitted)
{
@@ -176,8 +163,6 @@ static inline void bprm_clear_caps(struct linux_binprm *bprm)
bprm->cap_effective = false;
}
-#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
-
int cap_inode_need_killpriv(struct dentry *dentry)
{
struct inode *inode = dentry->d_inode;
@@ -317,24 +302,6 @@ out:
return rc;
}
-#else
-int cap_inode_need_killpriv(struct dentry *dentry)
-{
- return 0;
-}
-
-int cap_inode_killpriv(struct dentry *dentry)
-{
- return 0;
-}
-
-static inline int get_file_caps(struct linux_binprm *bprm)
-{
- bprm_clear_caps(bprm);
- return 0;
-}
-#endif
-
int cap_bprm_set_security (struct linux_binprm *bprm)
{
int ret;
@@ -535,7 +502,6 @@ int cap_task_post_setuid (uid_t old_ruid, uid_t old_euid, uid_t old_suid,
return 0;
}
-#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
/*
* Rationale: code calling task_setscheduler, task_setioprio, and
* task_setnice, assumes that
@@ -587,22 +553,6 @@ static long cap_prctl_drop(unsigned long cap)
return 0;
}
-#else
-int cap_task_setscheduler (struct task_struct *p, int policy,
- struct sched_param *lp)
-{
- return 0;
-}
-int cap_task_setioprio (struct task_struct *p, int ioprio)
-{
- return 0;
-}
-int cap_task_setnice (struct task_struct *p, int nice)
-{
- return 0;
-}
-#endif
-
int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
unsigned long arg4, unsigned long arg5, long *rc_p)
{
@@ -615,7 +565,6 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
else
error = !!cap_raised(current->cap_bset, arg2);
break;
-#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
case PR_CAPBSET_DROP:
error = cap_prctl_drop(arg2);
break;
@@ -662,8 +611,6 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
error = current->securebits;
break;
-#endif /* def CONFIG_SECURITY_FILE_CAPABILITIES */
-
case PR_GET_KEEPCAPS:
if (issecure(SECURE_KEEP_CAPS))
error = 1;
--
1.5.1.1.GIT
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/6] file capabilities: uninline cap_safe_nice
2008-09-27 2:27 ` [PATCH 1/6] file capabilities: add no_file_caps switch (v3) Serge E. Hallyn
2008-09-27 2:27 ` [PATCH 2/6] file capabilities: remove CONFIG_SECURITY_FILE_CAPABILITIES Serge E. Hallyn
@ 2008-09-27 2:27 ` Serge E. Hallyn
2008-09-27 4:26 ` Andrew G. Morgan
2008-09-27 2:27 ` [PATCH 4/6] file capabilities: clean up setcap code Serge E. Hallyn
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Serge E. Hallyn @ 2008-09-27 2:27 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-security-module, Serge E. Hallyn
This reduces the kernel size by 289 bytes.
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
security/commoncap.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/security/commoncap.c b/security/commoncap.c
index 9bfef94..d48fdd8 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -512,7 +512,7 @@ int cap_task_post_setuid (uid_t old_ruid, uid_t old_euid, uid_t old_suid,
* yet with increased caps.
* So we check for increased caps on the target process.
*/
-static inline int cap_safe_nice(struct task_struct *p)
+static int cap_safe_nice(struct task_struct *p)
{
if (!cap_issubset(p->cap_permitted, current->cap_permitted) &&
!capable(CAP_SYS_NICE))
--
1.5.1.1.GIT
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/6] file capabilities: clean up setcap code
2008-09-27 2:27 ` [PATCH 1/6] file capabilities: add no_file_caps switch (v3) Serge E. Hallyn
2008-09-27 2:27 ` [PATCH 2/6] file capabilities: remove CONFIG_SECURITY_FILE_CAPABILITIES Serge E. Hallyn
2008-09-27 2:27 ` [PATCH 3/6] file capabilities: uninline cap_safe_nice Serge E. Hallyn
@ 2008-09-27 2:27 ` Serge E. Hallyn
2008-09-27 4:58 ` Andrew G. Morgan
2008-09-27 2:27 ` [PATCH 5/6] file capabilities: remove needless inline functions Serge E. Hallyn
2008-09-27 2:27 ` [PATCH 6/6] file capabilities: remove needless (?) bprm_clear_caps calls Serge E. Hallyn
4 siblings, 1 reply; 17+ messages in thread
From: Serge E. Hallyn @ 2008-09-27 2:27 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-security-module, Serge E. Hallyn
Clean up the sys_capset codepath a bit to account for the fact
that you can now not ever, never, capset on another task.
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
kernel/capability.c | 83 +++++++++++++++++++-------------------------------
1 files changed, 32 insertions(+), 51 deletions(-)
diff --git a/kernel/capability.c b/kernel/capability.c
index d39c989..92dd85b 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -132,46 +132,31 @@ static int cap_validate_magic(cap_user_header_t header, unsigned *tocopy)
* process. The net result is that we can limit our use of locks to
* when we are reading the caps of another process.
*/
-static inline int cap_get_target_pid(pid_t pid, kernel_cap_t *pEp,
+static int cap_get_target_pid(pid_t pid, kernel_cap_t *pEp,
kernel_cap_t *pIp, kernel_cap_t *pPp)
{
int ret;
+ struct task_struct *target;
- if (pid && (pid != task_pid_vnr(current))) {
- struct task_struct *target;
+ if (!pid || pid == task_pid_vnr(current))
+ return security_capget(current, pEp, pIp, pPp);
- spin_lock(&task_capability_lock);
- read_lock(&tasklist_lock);
+ spin_lock(&task_capability_lock);
+ read_lock(&tasklist_lock);
- target = find_task_by_vpid(pid);
- if (!target)
- ret = -ESRCH;
- else
- ret = security_capget(target, pEp, pIp, pPp);
+ target = find_task_by_vpid(pid);
+ if (!target)
+ ret = -ESRCH;
+ else
+ ret = security_capget(target, pEp, pIp, pPp);
- read_unlock(&tasklist_lock);
- spin_unlock(&task_capability_lock);
- } else
- ret = security_capget(current, pEp, pIp, pPp);
+ read_unlock(&tasklist_lock);
+ spin_unlock(&task_capability_lock);
return ret;
}
/*
- * With filesystem capability support configured, the kernel does not
- * permit the changing of capabilities in one process by another
- * process. (CAP_SETPCAP has much less broad semantics when configured
- * this way.)
- */
-static inline int do_sys_capset_other_tasks(pid_t pid,
- kernel_cap_t *effective,
- kernel_cap_t *inheritable,
- kernel_cap_t *permitted)
-{
- return -EPERM;
-}
-
-/*
* Atomically modify the effective capabilities returning the original
* value. No permission check is performed here - it is assumed that the
* caller is permitted to set the desired effective capabilities.
@@ -293,6 +278,9 @@ asmlinkage long sys_capset(cap_user_header_t header, const cap_user_data_t data)
if (get_user(pid, &header->pid))
return -EFAULT;
+ if (pid && (pid != task_pid_vnr(current)))
+ return -EPERM;
+
if (copy_from_user(&kdata, data, tocopy
* sizeof(struct __user_cap_data_struct))) {
return -EFAULT;
@@ -310,30 +298,23 @@ asmlinkage long sys_capset(cap_user_header_t header, const cap_user_data_t data)
i++;
}
- if (pid && (pid != task_pid_vnr(current)))
- ret = do_sys_capset_other_tasks(pid, &effective, &inheritable,
- &permitted);
- else {
- /*
- * This lock is required even when filesystem
- * capability support is configured - it protects the
- * sys_capget() call from returning incorrect data in
- * the case that the targeted process is not the
- * current one.
- */
- spin_lock(&task_capability_lock);
+ /*
+ * This lock protects the sys_capget() call from
+ * returning incorrect data in the case that the targeted
+ * process is not the current one.
+ */
+ spin_lock(&task_capability_lock);
- ret = security_capset_check(current, &effective, &inheritable,
- &permitted);
- /*
- * Having verified that the proposed changes are
- * legal, we now put them into effect.
- */
- if (!ret)
- security_capset_set(current, &effective, &inheritable,
- &permitted);
- spin_unlock(&task_capability_lock);
- }
+ ret = security_capset_check(current, &effective, &inheritable,
+ &permitted);
+ /*
+ * Having verified that the proposed changes are
+ * legal, we now put them into effect.
+ */
+ if (!ret)
+ security_capset_set(current, &effective, &inheritable,
+ &permitted);
+ spin_unlock(&task_capability_lock);
return ret;
--
1.5.1.1.GIT
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/6] file capabilities: remove needless inline functions
2008-09-27 2:27 ` [PATCH 1/6] file capabilities: add no_file_caps switch (v3) Serge E. Hallyn
` (2 preceding siblings ...)
2008-09-27 2:27 ` [PATCH 4/6] file capabilities: clean up setcap code Serge E. Hallyn
@ 2008-09-27 2:27 ` Serge E. Hallyn
2008-09-27 4:39 ` Andrew G. Morgan
2008-09-27 2:27 ` [PATCH 6/6] file capabilities: remove needless (?) bprm_clear_caps calls Serge E. Hallyn
4 siblings, 1 reply; 17+ messages in thread
From: Serge E. Hallyn @ 2008-09-27 2:27 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-security-module, Serge E. Hallyn
cap_limit_ptraced_target always returns 1, so nix it.
cap_block_setpcap can't return 1 any more, because
kernel/capabilities.c:sys_capset() will return -EPERM
if it is called on a task other than current, and will
never get to cap_capset_check.
This brings the vmlinux size with my config down another
16 bytes (making up for the 8 byte increase from the
last patch).
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
security/commoncap.c | 22 +++-------------------
1 files changed, 3 insertions(+), 19 deletions(-)
diff --git a/security/commoncap.c b/security/commoncap.c
index d48fdd8..e5afb7c 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -93,15 +93,6 @@ int cap_capget (struct task_struct *target, kernel_cap_t *effective,
return 0;
}
-static inline int cap_block_setpcap(struct task_struct *target)
-{
- /*
- * No support for remote process capability manipulation with
- * filesystem capability support.
- */
- return (target != current);
-}
-
static inline int cap_inh_is_capped(void)
{
/*
@@ -112,14 +103,9 @@ static inline int cap_inh_is_capped(void)
return (cap_capable(current, CAP_SETPCAP) != 0);
}
-static inline int cap_limit_ptraced_target(void) { return 1; }
-
int cap_capset_check (struct task_struct *target, kernel_cap_t *effective,
kernel_cap_t *inheritable, kernel_cap_t *permitted)
{
- if (cap_block_setpcap(target)) {
- return -EPERM;
- }
if (cap_inh_is_capped()
&& !cap_issubset(*inheritable,
cap_combine(target->cap_inheritable,
@@ -343,11 +329,9 @@ void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
bprm->e_uid = current->uid;
bprm->e_gid = current->gid;
}
- if (cap_limit_ptraced_target()) {
- bprm->cap_post_exec_permitted = cap_intersect(
- bprm->cap_post_exec_permitted,
- current->cap_permitted);
- }
+ bprm->cap_post_exec_permitted = cap_intersect(
+ bprm->cap_post_exec_permitted,
+ current->cap_permitted);
}
}
--
1.5.1.1.GIT
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/6] file capabilities: remove needless (?) bprm_clear_caps calls
2008-09-27 2:27 ` [PATCH 1/6] file capabilities: add no_file_caps switch (v3) Serge E. Hallyn
` (3 preceding siblings ...)
2008-09-27 2:27 ` [PATCH 5/6] file capabilities: remove needless inline functions Serge E. Hallyn
@ 2008-09-27 2:27 ` Serge E. Hallyn
2008-09-27 2:27 ` Serge E. Hallyn
4 siblings, 1 reply; 17+ messages in thread
From: Serge E. Hallyn @ 2008-09-27 2:27 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-security-module, Serge E. Hallyn
Two error conditions at the top of get_file_caps() call
bprm_clear_caps(). However, the bprm capabilities have not
yet been set at this point.
Remove those calls (saving another 16 bytes on vmlinux).
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
security/commoncap.c | 8 ++------
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/security/commoncap.c b/security/commoncap.c
index e5afb7c..43bba7a 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -250,15 +250,11 @@ static int get_file_caps(struct linux_binprm *bprm)
struct vfs_cap_data vcaps;
struct inode *inode;
- if (!file_caps_enabled) {
- bprm_clear_caps(bprm);
+ if (!file_caps_enabled)
return 0;
- }
- if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID) {
- bprm_clear_caps(bprm);
+ if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID)
return 0;
- }
dentry = dget(bprm->file->f_dentry);
inode = dentry->d_inode;
--
1.5.1.1.GIT
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/6] file capabilities: remove needless (?) bprm_clear_caps calls
2008-09-27 2:27 ` [PATCH 6/6] file capabilities: remove needless (?) bprm_clear_caps calls Serge E. Hallyn
@ 2008-09-27 2:27 ` Serge E. Hallyn
2008-09-27 2:27 ` Serge E. Hallyn
0 siblings, 1 reply; 17+ messages in thread
From: Serge E. Hallyn @ 2008-09-27 2:27 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-security-module
Andrew Morgan, if you have a moment, please do take a close look
and make sure I'm not doing anything stupid/wrong in the cleanups!
However ltp shows no difference with and without the patchset.
Following are the kernel sizes after some of the patches.
original, pre-patch, with file capabilities compiled out:
text data bss dec hex filename
4188468 234432 316472 4739372 48512c vmlinux
original, pre-patch, with file capabilities compiled in:
4189356 234432 316472 4740260 4854a4 vmlinux
plain with fcaps always-on:
4189392 234456 316472 4740320 4854e0 vmlinux
with non-inline cap_safe_nice:
4189112 234456 316472 4740040 4853c8 vmlinux
with cleaned-up setcap:
4189120 234456 316472 4740048 4853d0 vmlinux
with needless check for target!=current removed from cap_capset:
4189104 234456 316472 4740032 4853c0 vmlinux
with needless(?) bprm_clear_caps calls removed:
4189088 234456 316472 4740016 4853b0 vmlinux
thanks,
-serge
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 6/6] file capabilities: remove needless (?) bprm_clear_caps calls
2008-09-27 2:27 ` Serge E. Hallyn
@ 2008-09-27 2:27 ` Serge E. Hallyn
0 siblings, 0 replies; 17+ messages in thread
From: Serge E. Hallyn @ 2008-09-27 2:27 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-security-module
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/6] file capabilities: remove CONFIG_SECURITY_FILE_CAPABILITIES
2008-09-27 2:27 ` [PATCH 2/6] file capabilities: remove CONFIG_SECURITY_FILE_CAPABILITIES Serge E. Hallyn
@ 2008-09-27 4:25 ` Andrew G. Morgan
0 siblings, 0 replies; 17+ messages in thread
From: Andrew G. Morgan @ 2008-09-27 4:25 UTC (permalink / raw)
To: Serge E. Hallyn; +Cc: linux-kernel, linux-security-module
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Serge E. Hallyn wrote:
> From: Serge Hallyn <serue@us.ibm.com>
>
> Remove the option to compile the kernel without file capabilities. Not
> compiling file capabilities actually makes the kernel less safe, as it
> includes the possibility for a task changing another task's capabilities.
>
> Some are concerned that userspace tools (and user education) are not
> up to the task of properly configuring file capabilities on a system.
> For those cases, there is now the ability to boot with the no_file_caps
> boot option. This will prevent file capabilities from being used in
> the capabilities recalculation at exec, but will not change the rest
> of the kernel behavior which used to be switchable using the
> CONFIG_SECURITY_FILE_CAPABILITIES option.
>
> Signed-off-by: Serge Hallyn <serue@us.ibm.com>
Acked-by: Andrew G. Morgan <morgan@kernel.org>
> ---
> fs/open.c | 8 --
> include/linux/capability.h | 2 -
> include/linux/init_task.h | 4 -
> kernel/capability.c | 158 --------------------------------------------
> security/Kconfig | 9 ---
> security/commoncap.c | 53 ---------------
> 6 files changed, 0 insertions(+), 234 deletions(-)
>
> diff --git a/fs/open.c b/fs/open.c
> index 07da935..6e1cd6e 100644
[...snip...]
Cheers
Andrew
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFI3bW9+bHCR3gb8jsRAoD6AKCdF8HNGdT8MPqWUBqrf8+BXGEyZwCfZc2T
+/hD1+FB2fTLae+vEbKpWX0=
=NerD
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/6] file capabilities: uninline cap_safe_nice
2008-09-27 2:27 ` [PATCH 3/6] file capabilities: uninline cap_safe_nice Serge E. Hallyn
@ 2008-09-27 4:26 ` Andrew G. Morgan
2008-09-27 5:27 ` James Morris
0 siblings, 1 reply; 17+ messages in thread
From: Andrew G. Morgan @ 2008-09-27 4:26 UTC (permalink / raw)
To: Serge E. Hallyn; +Cc: linux-kernel, linux-security-module
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Serge E. Hallyn wrote:
> This reduces the kernel size by 289 bytes.
>
> Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
Acked-by: Andrew G. Morgan <morgan@kernel.org>
Cheers
Andrew
> ---
> security/commoncap.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 9bfef94..d48fdd8 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -512,7 +512,7 @@ int cap_task_post_setuid (uid_t old_ruid, uid_t old_euid, uid_t old_suid,
> * yet with increased caps.
> * So we check for increased caps on the target process.
> */
> -static inline int cap_safe_nice(struct task_struct *p)
> +static int cap_safe_nice(struct task_struct *p)
> {
> if (!cap_issubset(p->cap_permitted, current->cap_permitted) &&
> !capable(CAP_SYS_NICE))
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFI3bX0+bHCR3gb8jsRApVqAKCuFmvJTlIiAYxXW5W3JNncFYmmrQCfa95h
Ik/MLVnpPtZdmuInkJSOof4=
=3aYe
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/6] file capabilities: remove needless inline functions
2008-09-27 2:27 ` [PATCH 5/6] file capabilities: remove needless inline functions Serge E. Hallyn
@ 2008-09-27 4:39 ` Andrew G. Morgan
2008-09-27 13:40 ` Serge E. Hallyn
0 siblings, 1 reply; 17+ messages in thread
From: Andrew G. Morgan @ 2008-09-27 4:39 UTC (permalink / raw)
To: Serge E. Hallyn; +Cc: linux-kernel, linux-security-module
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Serge,
I'd much rather simply remove the target argument from the
security_capset_check() call. Relying on the caller to not do something
bad seems fragile... If the code internally operates on current only,
then it doesn't need a target argument... No? (Evidently, such a change
is also needed to selinux_capset_check() too, but this doesn't look like
it will pose a problem for the selinux code.)
Cheers
Andrew
Serge E. Hallyn wrote:
> cap_limit_ptraced_target always returns 1, so nix it.
>
> cap_block_setpcap can't return 1 any more, because
> kernel/capabilities.c:sys_capset() will return -EPERM
> if it is called on a task other than current, and will
> never get to cap_capset_check.
>
> This brings the vmlinux size with my config down another
> 16 bytes (making up for the 8 byte increase from the
> last patch).
>
> Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> ---
> security/commoncap.c | 22 +++-------------------
> 1 files changed, 3 insertions(+), 19 deletions(-)
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index d48fdd8..e5afb7c 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -93,15 +93,6 @@ int cap_capget (struct task_struct *target, kernel_cap_t *effective,
> return 0;
> }
>
> -static inline int cap_block_setpcap(struct task_struct *target)
> -{
> - /*
> - * No support for remote process capability manipulation with
> - * filesystem capability support.
> - */
> - return (target != current);
> -}
> -
> static inline int cap_inh_is_capped(void)
> {
> /*
> @@ -112,14 +103,9 @@ static inline int cap_inh_is_capped(void)
> return (cap_capable(current, CAP_SETPCAP) != 0);
> }
>
> -static inline int cap_limit_ptraced_target(void) { return 1; }
> -
> int cap_capset_check (struct task_struct *target, kernel_cap_t *effective,
> kernel_cap_t *inheritable, kernel_cap_t *permitted)
> {
> - if (cap_block_setpcap(target)) {
> - return -EPERM;
> - }
> if (cap_inh_is_capped()
> && !cap_issubset(*inheritable,
> cap_combine(target->cap_inheritable,
> @@ -343,11 +329,9 @@ void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
> bprm->e_uid = current->uid;
> bprm->e_gid = current->gid;
> }
> - if (cap_limit_ptraced_target()) {
> - bprm->cap_post_exec_permitted = cap_intersect(
> - bprm->cap_post_exec_permitted,
> - current->cap_permitted);
> - }
> + bprm->cap_post_exec_permitted = cap_intersect(
> + bprm->cap_post_exec_permitted,
> + current->cap_permitted);
> }
> }
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFI3bjz+bHCR3gb8jsRAqJpAJ9Ca1pADkG5BnGoOVZA+EmZbuRPfgCgoQ95
ljvsvj7Ssp+0mXDuCy0/TnU=
=79ni
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] file capabilities: clean up setcap code
2008-09-27 2:27 ` [PATCH 4/6] file capabilities: clean up setcap code Serge E. Hallyn
@ 2008-09-27 4:58 ` Andrew G. Morgan
2008-09-27 13:43 ` Serge E. Hallyn
0 siblings, 1 reply; 17+ messages in thread
From: Andrew G. Morgan @ 2008-09-27 4:58 UTC (permalink / raw)
To: Serge E. Hallyn; +Cc: linux-kernel, linux-security-module
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Serge,
I have to say I'm a bit confused by this one. Specifically, the
cap_get_target_pid() change. In your 5/6 patch, you say this change
("the previous patch") makes the kernel bigger? Is this because of the
cap_get_target_pid() changes? Since you are fighting to reduce space, if
it bloats the code does the cap_get_target_pid() part of the change make
sense?
Cheers
Andrew
Serge E. Hallyn wrote:
> Clean up the sys_capset codepath a bit to account for the fact
> that you can now not ever, never, capset on another task.
>
> Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> ---
> kernel/capability.c | 83 +++++++++++++++++++-------------------------------
> 1 files changed, 32 insertions(+), 51 deletions(-)
>
> diff --git a/kernel/capability.c b/kernel/capability.c
> index d39c989..92dd85b 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -132,46 +132,31 @@ static int cap_validate_magic(cap_user_header_t header, unsigned *tocopy)
> * process. The net result is that we can limit our use of locks to
> * when we are reading the caps of another process.
> */
> -static inline int cap_get_target_pid(pid_t pid, kernel_cap_t *pEp,
> +static int cap_get_target_pid(pid_t pid, kernel_cap_t *pEp,
> kernel_cap_t *pIp, kernel_cap_t *pPp)
> {
> int ret;
> + struct task_struct *target;
>
> - if (pid && (pid != task_pid_vnr(current))) {
> - struct task_struct *target;
> + if (!pid || pid == task_pid_vnr(current))
> + return security_capget(current, pEp, pIp, pPp);
>
> - spin_lock(&task_capability_lock);
> - read_lock(&tasklist_lock);
> + spin_lock(&task_capability_lock);
> + read_lock(&tasklist_lock);
>
> - target = find_task_by_vpid(pid);
> - if (!target)
> - ret = -ESRCH;
> - else
> - ret = security_capget(target, pEp, pIp, pPp);
> + target = find_task_by_vpid(pid);
> + if (!target)
> + ret = -ESRCH;
> + else
> + ret = security_capget(target, pEp, pIp, pPp);
>
> - read_unlock(&tasklist_lock);
> - spin_unlock(&task_capability_lock);
> - } else
> - ret = security_capget(current, pEp, pIp, pPp);
> + read_unlock(&tasklist_lock);
> + spin_unlock(&task_capability_lock);
>
> return ret;
> }
>
> /*
> - * With filesystem capability support configured, the kernel does not
> - * permit the changing of capabilities in one process by another
> - * process. (CAP_SETPCAP has much less broad semantics when configured
> - * this way.)
> - */
> -static inline int do_sys_capset_other_tasks(pid_t pid,
> - kernel_cap_t *effective,
> - kernel_cap_t *inheritable,
> - kernel_cap_t *permitted)
> -{
> - return -EPERM;
> -}
> -
> -/*
> * Atomically modify the effective capabilities returning the original
> * value. No permission check is performed here - it is assumed that the
> * caller is permitted to set the desired effective capabilities.
> @@ -293,6 +278,9 @@ asmlinkage long sys_capset(cap_user_header_t header, const cap_user_data_t data)
> if (get_user(pid, &header->pid))
> return -EFAULT;
>
> + if (pid && (pid != task_pid_vnr(current)))
> + return -EPERM;
> +
> if (copy_from_user(&kdata, data, tocopy
> * sizeof(struct __user_cap_data_struct))) {
> return -EFAULT;
> @@ -310,30 +298,23 @@ asmlinkage long sys_capset(cap_user_header_t header, const cap_user_data_t data)
> i++;
> }
>
> - if (pid && (pid != task_pid_vnr(current)))
> - ret = do_sys_capset_other_tasks(pid, &effective, &inheritable,
> - &permitted);
> - else {
> - /*
> - * This lock is required even when filesystem
> - * capability support is configured - it protects the
> - * sys_capget() call from returning incorrect data in
> - * the case that the targeted process is not the
> - * current one.
> - */
> - spin_lock(&task_capability_lock);
> + /*
> + * This lock protects the sys_capget() call from
> + * returning incorrect data in the case that the targeted
> + * process is not the current one.
> + */
> + spin_lock(&task_capability_lock);
>
> - ret = security_capset_check(current, &effective, &inheritable,
> - &permitted);
> - /*
> - * Having verified that the proposed changes are
> - * legal, we now put them into effect.
> - */
> - if (!ret)
> - security_capset_set(current, &effective, &inheritable,
> - &permitted);
> - spin_unlock(&task_capability_lock);
> - }
> + ret = security_capset_check(current, &effective, &inheritable,
> + &permitted);
> + /*
> + * Having verified that the proposed changes are
> + * legal, we now put them into effect.
> + */
> + if (!ret)
> + security_capset_set(current, &effective, &inheritable,
> + &permitted);
> + spin_unlock(&task_capability_lock);
>
>
> return ret;
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFI3b1s+bHCR3gb8jsRAkWCAJ4j5Q5NQc2TD8B+WOYJ1JIqV1GdqQCg1qQU
+qzZPOvwo/W/73BuA+HvuxQ=
=fRje
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/6] file capabilities: uninline cap_safe_nice
2008-09-27 4:26 ` Andrew G. Morgan
@ 2008-09-27 5:27 ` James Morris
0 siblings, 0 replies; 17+ messages in thread
From: James Morris @ 2008-09-27 5:27 UTC (permalink / raw)
To: Andrew G. Morgan; +Cc: Serge E. Hallyn, linux-kernel, linux-security-module
On Fri, 26 Sep 2008, Andrew G. Morgan wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
>
> Serge E. Hallyn wrote:
> > This reduces the kernel size by 289 bytes.
> >
> > Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
>
> Acked-by: Andrew G. Morgan <morgan@kernel.org>
>
This one makes sense on its own, applied to:
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
- James
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/6] file capabilities: remove needless inline functions
2008-09-27 4:39 ` Andrew G. Morgan
@ 2008-09-27 13:40 ` Serge E. Hallyn
2008-09-29 21:53 ` Serge E. Hallyn
0 siblings, 1 reply; 17+ messages in thread
From: Serge E. Hallyn @ 2008-09-27 13:40 UTC (permalink / raw)
To: Andrew G. Morgan; +Cc: linux-kernel, linux-security-module
Quoting Andrew G. Morgan (morgan@kernel.org):
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Serge,
>
> I'd much rather simply remove the target argument from the
> security_capset_check() call. Relying on the caller to not do something
Yes, I did do that in a patch on top of all of these. It increased the
kernel size by 8 (or was it 16) bytes :) I assume that was because
now there were more references to 'current' which is inlined? So I
should be able to fix that by saving current() away at the top of
the fn.
Will try that.
thanks,
-serge
> bad seems fragile... If the code internally operates on current only,
> then it doesn't need a target argument... No? (Evidently, such a change
> is also needed to selinux_capset_check() too, but this doesn't look like
> it will pose a problem for the selinux code.)
>
> Cheers
>
> Andrew
>
> Serge E. Hallyn wrote:
> > cap_limit_ptraced_target always returns 1, so nix it.
> >
> > cap_block_setpcap can't return 1 any more, because
> > kernel/capabilities.c:sys_capset() will return -EPERM
> > if it is called on a task other than current, and will
> > never get to cap_capset_check.
> >
> > This brings the vmlinux size with my config down another
> > 16 bytes (making up for the 8 byte increase from the
> > last patch).
> >
> > Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> > ---
> > security/commoncap.c | 22 +++-------------------
> > 1 files changed, 3 insertions(+), 19 deletions(-)
> >
> > diff --git a/security/commoncap.c b/security/commoncap.c
> > index d48fdd8..e5afb7c 100644
> > --- a/security/commoncap.c
> > +++ b/security/commoncap.c
> > @@ -93,15 +93,6 @@ int cap_capget (struct task_struct *target, kernel_cap_t *effective,
> > return 0;
> > }
> >
> > -static inline int cap_block_setpcap(struct task_struct *target)
> > -{
> > - /*
> > - * No support for remote process capability manipulation with
> > - * filesystem capability support.
> > - */
> > - return (target != current);
> > -}
> > -
> > static inline int cap_inh_is_capped(void)
> > {
> > /*
> > @@ -112,14 +103,9 @@ static inline int cap_inh_is_capped(void)
> > return (cap_capable(current, CAP_SETPCAP) != 0);
> > }
> >
> > -static inline int cap_limit_ptraced_target(void) { return 1; }
> > -
> > int cap_capset_check (struct task_struct *target, kernel_cap_t *effective,
> > kernel_cap_t *inheritable, kernel_cap_t *permitted)
> > {
> > - if (cap_block_setpcap(target)) {
> > - return -EPERM;
> > - }
> > if (cap_inh_is_capped()
> > && !cap_issubset(*inheritable,
> > cap_combine(target->cap_inheritable,
> > @@ -343,11 +329,9 @@ void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
> > bprm->e_uid = current->uid;
> > bprm->e_gid = current->gid;
> > }
> > - if (cap_limit_ptraced_target()) {
> > - bprm->cap_post_exec_permitted = cap_intersect(
> > - bprm->cap_post_exec_permitted,
> > - current->cap_permitted);
> > - }
> > + bprm->cap_post_exec_permitted = cap_intersect(
> > + bprm->cap_post_exec_permitted,
> > + current->cap_permitted);
> > }
> > }
> >
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.7 (Darwin)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
> iD8DBQFI3bjz+bHCR3gb8jsRAqJpAJ9Ca1pADkG5BnGoOVZA+EmZbuRPfgCgoQ95
> ljvsvj7Ssp+0mXDuCy0/TnU=
> =79ni
> -----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] file capabilities: clean up setcap code
2008-09-27 4:58 ` Andrew G. Morgan
@ 2008-09-27 13:43 ` Serge E. Hallyn
0 siblings, 0 replies; 17+ messages in thread
From: Serge E. Hallyn @ 2008-09-27 13:43 UTC (permalink / raw)
To: Andrew G. Morgan; +Cc: linux-kernel, linux-security-module
Quoting Andrew G. Morgan (morgan@kernel.org):
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Serge,
>
> I have to say I'm a bit confused by this one. Specifically, the
> cap_get_target_pid() change. In your 5/6 patch, you say this change
> ("the previous patch") makes the kernel bigger? Is this because of the
> cap_get_target_pid() changes? Since you are fighting to reduce space, if
> it bloats the code does the cap_get_target_pid() part of the change make
> sense?
Yes I think it does. Yes my goal was to decrease the kernel size, but
having cleaner code - and getting rid of dead codepaths - is more
important.
It may be hard to tell by looking at the patch, but I think the
end-result is really far better.
thanks,
-serge
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/6] file capabilities: remove needless inline functions
2008-09-27 13:40 ` Serge E. Hallyn
@ 2008-09-29 21:53 ` Serge E. Hallyn
0 siblings, 0 replies; 17+ messages in thread
From: Serge E. Hallyn @ 2008-09-29 21:53 UTC (permalink / raw)
To: Andrew G. Morgan; +Cc: linux-kernel, linux-security-module
Quoting Serge E. Hallyn (serue@us.ibm.com):
> Quoting Andrew G. Morgan (morgan@kernel.org):
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> >
> > Serge,
> >
> > I'd much rather simply remove the target argument from the
> > security_capset_check() call. Relying on the caller to not do something
>
> Yes, I did do that in a patch on top of all of these. It increased the
> kernel size by 8 (or was it 16) bytes :) I assume that was because
> now there were more references to 'current' which is inlined? So I
> should be able to fix that by saving current() away at the top of
> the fn.
>
> Will try that.
>
> thanks,
> -serge
>
> > bad seems fragile... If the code internally operates on current only,
> > then it doesn't need a target argument... No? (Evidently, such a change
> > is also needed to selinux_capset_check() too, but this doesn't look like
> > it will pose a problem for the selinux code.)
Here is a patch (on top of the others) doing that.
thanks,
-serge
Subject: [PATCH] capabilities: remove target argument from capset_check and set
Since it is no longer possible to set capabilities on another task,
remove the 'target' option from capset_set and capset_check.
Since the userspace API shouldn't change, we maintain 'pid' as a
valid option to the actual sys_capset(), continuing to return
-EPERM when pid is not current's pid.
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
include/linux/security.h | 49 ++++++++++++++++------------------------------
kernel/capability.c | 6 ++----
security/commoncap.c | 29 +++++++++++++++------------
security/security.c | 18 ++++++++---------
security/selinux/hooks.c | 15 ++++++++------
5 files changed, 51 insertions(+), 66 deletions(-)
35b1a7906d26fd10da34e5cd350470abb812a91c
diff --git a/include/linux/security.h b/include/linux/security.h
index 80c4d00..7b9f7a2 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -49,8 +49,8 @@ extern int cap_settime(struct timespec *
extern int cap_ptrace_may_access(struct task_struct *child, unsigned int mode);
extern int cap_ptrace_traceme(struct task_struct *parent);
extern int cap_capget(struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted);
-extern int cap_capset_check(struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted);
-extern void cap_capset_set(struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted);
+extern int cap_capset_check(kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted);
+extern void cap_capset_set(kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted);
extern int cap_bprm_set_security(struct linux_binprm *bprm);
extern void cap_bprm_apply_creds(struct linux_binprm *bprm, int unsafe);
extern int cap_bprm_secureexec(struct linux_binprm *bprm);
@@ -1187,24 +1187,15 @@ static inline void security_free_mnt_opt
* Return 0 if the capability sets were successfully obtained.
* @capset_check:
* Check permission before setting the @effective, @inheritable, and
- * @permitted capability sets for the @target process.
- * Caveat: @target is also set to current if a set of processes is
- * specified (i.e. all processes other than current and init or a
- * particular process group). Hence, the capset_set hook may need to
- * revalidate permission to the actual target process.
- * @target contains the task_struct structure for target process.
+ * @permitted capability sets for the current process.
* @effective contains the effective capability set.
* @inheritable contains the inheritable capability set.
* @permitted contains the permitted capability set.
* Return 0 if permission is granted.
* @capset_set:
* Set the @effective, @inheritable, and @permitted capability sets for
- * the @target process. Since capset_check cannot always check permission
- * to the real @target process, this hook may also perform permission
- * checking to determine if the current process is allowed to set the
- * capability sets of the @target process. However, this hook has no way
- * of returning an error due to the structure of the sys_capset code.
- * @target contains the task_struct structure for target process.
+ * the current process. This hook has no way of returning an error due to
+ * the structure of the sys_capset code.
* @effective contains the effective capability set.
* @inheritable contains the inheritable capability set.
* @permitted contains the permitted capability set.
@@ -1299,12 +1290,10 @@ struct security_operations {
int (*capget) (struct task_struct *target,
kernel_cap_t *effective,
kernel_cap_t *inheritable, kernel_cap_t *permitted);
- int (*capset_check) (struct task_struct *target,
- kernel_cap_t *effective,
+ int (*capset_check) (kernel_cap_t *effective,
kernel_cap_t *inheritable,
kernel_cap_t *permitted);
- void (*capset_set) (struct task_struct *target,
- kernel_cap_t *effective,
+ void (*capset_set) (kernel_cap_t *effective,
kernel_cap_t *inheritable,
kernel_cap_t *permitted);
int (*capable) (struct task_struct *tsk, int cap);
@@ -1573,12 +1562,10 @@ int security_capget(struct task_struct *
kernel_cap_t *effective,
kernel_cap_t *inheritable,
kernel_cap_t *permitted);
-int security_capset_check(struct task_struct *target,
- kernel_cap_t *effective,
+int security_capset_check(kernel_cap_t *effective,
kernel_cap_t *inheritable,
kernel_cap_t *permitted);
-void security_capset_set(struct task_struct *target,
- kernel_cap_t *effective,
+void security_capset_set(kernel_cap_t *effective,
kernel_cap_t *inheritable,
kernel_cap_t *permitted);
int security_capable(struct task_struct *tsk, int cap);
@@ -1768,20 +1755,18 @@ static inline int security_capget(struct
return cap_capget(target, effective, inheritable, permitted);
}
-static inline int security_capset_check(struct task_struct *target,
- kernel_cap_t *effective,
- kernel_cap_t *inheritable,
- kernel_cap_t *permitted)
+static inline int security_capset_check(kernel_cap_t *effective,
+ kernel_cap_t *inheritable,
+ kernel_cap_t *permitted)
{
- return cap_capset_check(target, effective, inheritable, permitted);
+ return cap_capset_check(effective, inheritable, permitted);
}
-static inline void security_capset_set(struct task_struct *target,
- kernel_cap_t *effective,
- kernel_cap_t *inheritable,
- kernel_cap_t *permitted)
+static inline void security_capset_set(kernel_cap_t *effective,
+ kernel_cap_t *inheritable,
+ kernel_cap_t *permitted)
{
- cap_capset_set(target, effective, inheritable, permitted);
+ cap_capset_set(effective, inheritable, permitted);
}
static inline int security_capable(struct task_struct *tsk, int cap)
diff --git a/kernel/capability.c b/kernel/capability.c
index 92dd85b..f119288 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -305,15 +305,13 @@ asmlinkage long sys_capset(cap_user_head
*/
spin_lock(&task_capability_lock);
- ret = security_capset_check(current, &effective, &inheritable,
- &permitted);
+ ret = security_capset_check(&effective, &inheritable, &permitted);
/*
* Having verified that the proposed changes are
* legal, we now put them into effect.
*/
if (!ret)
- security_capset_set(current, &effective, &inheritable,
- &permitted);
+ security_capset_set(&effective, &inheritable, &permitted);
spin_unlock(&task_capability_lock);
diff --git a/security/commoncap.c b/security/commoncap.c
index 43bba7a..031bb90 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -103,27 +103,29 @@ static inline int cap_inh_is_capped(void
return (cap_capable(current, CAP_SETPCAP) != 0);
}
-int cap_capset_check (struct task_struct *target, kernel_cap_t *effective,
- kernel_cap_t *inheritable, kernel_cap_t *permitted)
+int cap_capset_check (kernel_cap_t *effective, kernel_cap_t *inheritable,
+ kernel_cap_t *permitted)
{
+ struct task_struct *t = current;
+
if (cap_inh_is_capped()
&& !cap_issubset(*inheritable,
- cap_combine(target->cap_inheritable,
- current->cap_permitted))) {
+ cap_combine(t->cap_inheritable,
+ t->cap_permitted))) {
/* incapable of using this inheritable set */
return -EPERM;
}
if (!cap_issubset(*inheritable,
- cap_combine(target->cap_inheritable,
- current->cap_bset))) {
+ cap_combine(t->cap_inheritable,
+ t->cap_bset))) {
/* no new pI capabilities outside bounding set */
return -EPERM;
}
/* verify restrictions on target's new Permitted set */
if (!cap_issubset (*permitted,
- cap_combine (target->cap_permitted,
- current->cap_permitted))) {
+ cap_combine (t->cap_permitted,
+ t->cap_permitted))) {
return -EPERM;
}
@@ -135,12 +137,13 @@ int cap_capset_check (struct task_struct
return 0;
}
-void cap_capset_set (struct task_struct *target, kernel_cap_t *effective,
- kernel_cap_t *inheritable, kernel_cap_t *permitted)
+void cap_capset_set (kernel_cap_t *effective, kernel_cap_t *inheritable,
+ kernel_cap_t *permitted)
{
- target->cap_effective = *effective;
- target->cap_inheritable = *inheritable;
- target->cap_permitted = *permitted;
+ struct task_struct *t = current;
+ t->cap_effective = *effective;
+ t->cap_inheritable = *inheritable;
+ t->cap_permitted = *permitted;
}
static inline void bprm_clear_caps(struct linux_binprm *bprm)
diff --git a/security/security.c b/security/security.c
index 3a4b4f5..78502ac 100644
--- a/security/security.c
+++ b/security/security.c
@@ -145,20 +145,18 @@ int security_capget(struct task_struct *
return security_ops->capget(target, effective, inheritable, permitted);
}
-int security_capset_check(struct task_struct *target,
- kernel_cap_t *effective,
- kernel_cap_t *inheritable,
- kernel_cap_t *permitted)
+int security_capset_check(kernel_cap_t *effective,
+ kernel_cap_t *inheritable,
+ kernel_cap_t *permitted)
{
- return security_ops->capset_check(target, effective, inheritable, permitted);
+ return security_ops->capset_check(effective, inheritable, permitted);
}
-void security_capset_set(struct task_struct *target,
- kernel_cap_t *effective,
- kernel_cap_t *inheritable,
- kernel_cap_t *permitted)
+void security_capset_set(kernel_cap_t *effective,
+ kernel_cap_t *inheritable,
+ kernel_cap_t *permitted)
{
- security_ops->capset_set(target, effective, inheritable, permitted);
+ security_ops->capset_set(effective, inheritable, permitted);
}
int security_capable(struct task_struct *tsk, int cap)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 03fc6a8..d06e943 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1780,22 +1780,23 @@ static int selinux_capget(struct task_st
return secondary_ops->capget(target, effective, inheritable, permitted);
}
-static int selinux_capset_check(struct task_struct *target, kernel_cap_t *effective,
- kernel_cap_t *inheritable, kernel_cap_t *permitted)
+static int selinux_capset_check(kernel_cap_t *effective, kernel_cap_t *inheritable,
+ kernel_cap_t *permitted)
{
int error;
+ struct task_struct *t = current;
- error = secondary_ops->capset_check(target, effective, inheritable, permitted);
+ error = secondary_ops->capset_check(effective, inheritable, permitted);
if (error)
return error;
- return task_has_perm(current, target, PROCESS__SETCAP);
+ return task_has_perm(t, t, PROCESS__SETCAP);
}
-static void selinux_capset_set(struct task_struct *target, kernel_cap_t *effective,
- kernel_cap_t *inheritable, kernel_cap_t *permitted)
+static void selinux_capset_set(kernel_cap_t *effective, kernel_cap_t *inheritable,
+ kernel_cap_t *permitted)
{
- secondary_ops->capset_set(target, effective, inheritable, permitted);
+ secondary_ops->capset_set(effective, inheritable, permitted);
}
static int selinux_capable(struct task_struct *tsk, int cap)
--
1.1.6
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2008-09-29 21:54 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-27 2:27 [PATCH 0/6] file capabilities cleanups: introduction Serge E. Hallyn
2008-09-27 2:27 ` [PATCH 1/6] file capabilities: add no_file_caps switch (v3) Serge E. Hallyn
2008-09-27 2:27 ` [PATCH 2/6] file capabilities: remove CONFIG_SECURITY_FILE_CAPABILITIES Serge E. Hallyn
2008-09-27 4:25 ` Andrew G. Morgan
2008-09-27 2:27 ` [PATCH 3/6] file capabilities: uninline cap_safe_nice Serge E. Hallyn
2008-09-27 4:26 ` Andrew G. Morgan
2008-09-27 5:27 ` James Morris
2008-09-27 2:27 ` [PATCH 4/6] file capabilities: clean up setcap code Serge E. Hallyn
2008-09-27 4:58 ` Andrew G. Morgan
2008-09-27 13:43 ` Serge E. Hallyn
2008-09-27 2:27 ` [PATCH 5/6] file capabilities: remove needless inline functions Serge E. Hallyn
2008-09-27 4:39 ` Andrew G. Morgan
2008-09-27 13:40 ` Serge E. Hallyn
2008-09-29 21:53 ` Serge E. Hallyn
2008-09-27 2:27 ` [PATCH 6/6] file capabilities: remove needless (?) bprm_clear_caps calls Serge E. Hallyn
2008-09-27 2:27 ` Serge E. Hallyn
2008-09-27 2:27 ` Serge E. Hallyn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox