* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2008-12-31 15:15 ` [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2] David Howells
@ 2008-12-31 23:24 ` James Morris
2009-01-01 23:53 ` J. Bruce Fields
` (2 subsequent siblings)
3 siblings, 0 replies; 41+ messages in thread
From: James Morris @ 2008-12-31 23:24 UTC (permalink / raw)
To: David Howells
Cc: Christoph Hellwig, linux-kernel, linux-fsdevel,
linux-security-module
[adding lsm list to the cc]
On Wed, 31 Dec 2008, David Howells wrote:
>
> Here's an improved patch. It differentiates the use of objective and
> subjective capabilities by making capable() only check current's subjective
> caps, but making has_capability() check only the objective caps of whatever
> process is specified.
>
> It's a bit more involved, but I think it's the right thing to do.
>
> David
>
> ---
> From: David Howells <dhowells@redhat.com>
> Subject: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat()
>
> Fix a regression in cap_capable() due to:
>
> commit 5ff7711e635b32f0a1e558227d030c7e45b4a465
> Author: David Howells <dhowells@redhat.com>
> Date: Wed Dec 31 02:52:28 2008 +0000
>
> CRED: Differentiate objective and effective subjective credentials on a task
>
>
> The problem is that the above patch allows a process to have two sets of
> credentials, and for the most part uses the subjective credentials when
> accessing current's creds.
>
> There is, however, one exception: cap_capable(), and thus capable(), uses the
> real/objective credentials of the target task, whether or not it is the current
> task.
>
> Ordinarily this doesn't matter, since usually the two cred pointers in current
> point to the same set of creds. However, sys_faccessat() makes use of this
> facility to override the credentials of the calling process to make its test,
> without affecting the creds as seen from other processes.
>
> One of the things sys_faccessat() does is to make an adjustment to the
> effective capabilities mask, which cap_capable(), as it stands, then ignores.
>
> The affected capability check is in generic_permission():
>
> if (!(mask & MAY_EXEC) || execute_ok(inode))
> if (capable(CAP_DAC_OVERRIDE))
> return 0;
>
>
> This change splits capable() from has_capability() down into the commoncap and
> SELinux code. The capable() security op now only deals with the current
> process, and uses the current process's subjective creds. A new security op -
> task_capable() - is introduced that can check any task's objective creds.
>
> strictly the capable() security op is superfluous with the presence of the
> task_capable() op, however it should be faster to call the capable() op since
> two fewer arguments need be passed down through the various layers.
>
>
> This can be tested by compiling the following program from the XFS testsuite:
>
> /*
> * t_access_root.c - trivial test program to show permission bug.
> *
> * Written by Michael Kerrisk - copyright ownership not pursued.
> * Sourced from: http://linux.derkeiler.com/Mailing-Lists/Kernel/2003-10/6030.html
> */
> #include <limits.h>
> #include <unistd.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <fcntl.h>
> #include <sys/stat.h>
>
> #define UID 500
> #define GID 100
> #define PERM 0
> #define TESTPATH "/tmp/t_access"
>
> static void
> errExit(char *msg)
> {
> perror(msg);
> exit(EXIT_FAILURE);
> } /* errExit */
>
> static void
> accessTest(char *file, int mask, char *mstr)
> {
> printf("access(%s, %s) returns %d\n", file, mstr, access(file, mask));
> } /* accessTest */
>
> int
> main(int argc, char *argv[])
> {
> int fd, perm, uid, gid;
> char *testpath;
> char cmd[PATH_MAX + 20];
>
> testpath = (argc > 1) ? argv[1] : TESTPATH;
> perm = (argc > 2) ? strtoul(argv[2], NULL, 8) : PERM;
> uid = (argc > 3) ? atoi(argv[3]) : UID;
> gid = (argc > 4) ? atoi(argv[4]) : GID;
>
> unlink(testpath);
>
> fd = open(testpath, O_RDWR | O_CREAT, 0);
> if (fd == -1) errExit("open");
>
> if (fchown(fd, uid, gid) == -1) errExit("fchown");
> if (fchmod(fd, perm) == -1) errExit("fchmod");
> close(fd);
>
> snprintf(cmd, sizeof(cmd), "ls -l %s", testpath);
> system(cmd);
>
> if (seteuid(uid) == -1) errExit("seteuid");
>
> accessTest(testpath, 0, "0");
> accessTest(testpath, R_OK, "R_OK");
> accessTest(testpath, W_OK, "W_OK");
> accessTest(testpath, X_OK, "X_OK");
> accessTest(testpath, R_OK | W_OK, "R_OK | W_OK");
> accessTest(testpath, R_OK | X_OK, "R_OK | X_OK");
> accessTest(testpath, W_OK | X_OK, "W_OK | X_OK");
> accessTest(testpath, R_OK | W_OK | X_OK, "R_OK | W_OK | X_OK");
>
> exit(EXIT_SUCCESS);
> } /* main */
>
>
> This can be run against an Ext3 filesystem as well as against an XFS
> filesystem. If successful, it will show:
>
> [root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043
> ---------- 1 dhowells dhowells 0 2008-12-31 03:00 /tmp/xxx
> access(/tmp/xxx, 0) returns 0
> access(/tmp/xxx, R_OK) returns 0
> access(/tmp/xxx, W_OK) returns 0
> access(/tmp/xxx, X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK) returns 0
> access(/tmp/xxx, R_OK | X_OK) returns -1
> access(/tmp/xxx, W_OK | X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1
>
> If unsuccessful, it will show:
>
> [root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043
> ---------- 1 dhowells dhowells 0 2008-12-31 02:56 /tmp/xxx
> access(/tmp/xxx, 0) returns 0
> access(/tmp/xxx, R_OK) returns -1
> access(/tmp/xxx, W_OK) returns -1
> access(/tmp/xxx, X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK) returns -1
> access(/tmp/xxx, R_OK | X_OK) returns -1
> access(/tmp/xxx, W_OK | X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1
>
> I've also tested the fix with the SELinux and syscalls LTP testsuites.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> include/linux/capability.h | 17 +++++++++++++--
> include/linux/security.h | 49 ++++++++++++++++++++++++++++++++++++--------
> kernel/capability.c | 2 +-
> security/capability.c | 1 +
> security/commoncap.c | 42 ++++++++++++++++++++++++++------------
> security/root_plug.c | 1 +
> security/security.c | 25 +++++++++++++++++++---
> security/selinux/hooks.c | 26 ++++++++++++++++++-----
> security/smack/smack_lsm.c | 1 +
> 9 files changed, 129 insertions(+), 35 deletions(-)
>
>
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index e22f48c..5b8a132 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -529,8 +529,21 @@ extern const kernel_cap_t __cap_init_eff_set;
> *
> * Note that this does not set PF_SUPERPRIV on the task.
> */
> -#define has_capability(t, cap) (security_capable((t), (cap)) == 0)
> -#define has_capability_noaudit(t, cap) (security_capable_noaudit((t), (cap)) == 0)
> +#define has_capability(t, cap) (security_task_capable((t), (cap)) == 0)
> +
> +/**
> + * has_capability_noaudit - Determine if a task has a superior capability available (unaudited)
> + * @t: The task in question
> + * @cap: The capability to be tested for
> + *
> + * Return true if the specified task has the given superior capability
> + * currently in effect, false if not, but don't write an audit message for the
> + * check.
> + *
> + * Note that this does not set PF_SUPERPRIV on the task.
> + */
> +#define has_capability_noaudit(t, cap) \
> + (security_task_capable_noaudit((t), (cap)) == 0)
>
> extern int capable(int cap);
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 3416cb8..76989b8 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -48,7 +48,9 @@ struct audit_krule;
> * These functions are in security/capability.c and are used
> * as the default capabilities functions
> */
> -extern int cap_capable(struct task_struct *tsk, int cap, int audit);
> +extern int cap_capable(int cap, int audit);
> +extern int cap_task_capable(struct task_struct *tsk, const struct cred *cred,
> + int cap, int audit);
> extern int cap_settime(struct timespec *ts, struct timezone *tz);
> extern int cap_ptrace_may_access(struct task_struct *child, unsigned int mode);
> extern int cap_ptrace_traceme(struct task_struct *parent);
> @@ -1195,9 +1197,18 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
> * @permitted contains the permitted capability set.
> * Return 0 and update @new if permission is granted.
> * @capable:
> - * Check whether the @tsk process has the @cap capability.
> + * Check whether the current process has the @cap capability in its
> + * subjective/effective credentials.
> + * @cap contains the capability <include/linux/capability.h>.
> + * @audit: Whether to write an audit message or not
> + * Return 0 if the capability is granted for @tsk.
> + * @task_capable:
> + * Check whether the @tsk process has the @cap capability in its
> + * objective/real credentials.
> * @tsk contains the task_struct for the process.
> + * @cred contains the credentials to use.
> * @cap contains the capability <include/linux/capability.h>.
> + * @audit: Whether to write an audit message or not
> * Return 0 if the capability is granted for @tsk.
> * @acct:
> * Check permission before enabling or disabling process accounting. If
> @@ -1290,7 +1301,9 @@ struct security_operations {
> const kernel_cap_t *effective,
> const kernel_cap_t *inheritable,
> const kernel_cap_t *permitted);
> - int (*capable) (struct task_struct *tsk, int cap, int audit);
> + int (*capable) (int cap, int audit);
> + int (*task_capable) (struct task_struct *tsk, const struct cred *cred,
> + int cap, int audit);
> int (*acct) (struct file *file);
> int (*sysctl) (struct ctl_table *table, int op);
> int (*quotactl) (int cmds, int type, int id, struct super_block *sb);
> @@ -1556,8 +1569,9 @@ int security_capset(struct cred *new, const struct cred *old,
> const kernel_cap_t *effective,
> const kernel_cap_t *inheritable,
> const kernel_cap_t *permitted);
> -int security_capable(struct task_struct *tsk, int cap);
> -int security_capable_noaudit(struct task_struct *tsk, int cap);
> +int security_capable(int cap);
> +int security_task_capable(struct task_struct *tsk, int cap);
> +int security_task_capable_noaudit(struct task_struct *tsk, int cap);
> int security_acct(struct file *file);
> int security_sysctl(struct ctl_table *table, int op);
> int security_quotactl(int cmds, int type, int id, struct super_block *sb);
> @@ -1754,14 +1768,31 @@ static inline int security_capset(struct cred *new,
> return cap_capset(new, old, effective, inheritable, permitted);
> }
>
> -static inline int security_capable(struct task_struct *tsk, int cap)
> +static inline int security_capable(int cap)
> {
> - return cap_capable(tsk, cap, SECURITY_CAP_AUDIT);
> + return cap_capable(cap, SECURITY_CAP_AUDIT);
> }
>
> -static inline int security_capable_noaudit(struct task_struct *tsk, int cap)
> +static inline int security_task_capable(struct task_struct *tsk, int cap)
> {
> - return cap_capable(tsk, cap, SECURITY_CAP_NOAUDIT);
> + int ret;
> +
> + rcu_read_lock();
> + ret = cap_task_capable(tsk, __task_cred(tsk), cap, SECURITY_CAP_AUDIT);
> + rcu_read_unlock();
> + return ret;
> +}
> +
> +static inline
> +int security_task_capable_noaudit(struct task_struct *tsk, int cap)
> +{
> + int ret;
> +
> + rcu_read_lock();
> + ret = cap_task_capable(tsk, __task_cred(tsk), cap,
> + SECURITY_CAP_NOAUDIT);
> + rcu_read_unlock();
> + return ret;
> }
>
> static inline int security_acct(struct file *file)
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 36b4b4d..df62f53 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -308,7 +308,7 @@ int capable(int cap)
> BUG();
> }
>
> - if (has_capability(current, cap)) {
> + if (security_capable(cap) == 0) {
> current->flags |= PF_SUPERPRIV;
> return 1;
> }
> diff --git a/security/capability.c b/security/capability.c
> index 2dce66f..fd1493d 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -826,6 +826,7 @@ void security_fixup_ops(struct security_operations *ops)
> set_to_cap_if_null(ops, capset);
> set_to_cap_if_null(ops, acct);
> set_to_cap_if_null(ops, capable);
> + set_to_cap_if_null(ops, task_capable);
> set_to_cap_if_null(ops, quotactl);
> set_to_cap_if_null(ops, quota_on);
> set_to_cap_if_null(ops, sysctl);
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 7971354..7f0b2a6 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -43,28 +43,44 @@ int cap_netlink_recv(struct sk_buff *skb, int cap)
> EXPORT_SYMBOL(cap_netlink_recv);
>
> /**
> - * cap_capable - Determine whether a task has a particular effective capability
> - * @tsk: The task to query
> + * cap_capable - Determine whether current has a particular effective capability
> * @cap: The capability to check for
> * @audit: Whether to write an audit message or not
> *
> * Determine whether the nominated task has the specified capability amongst
> - * its effective set, returning 0 if it does, -ve if it does not.
> + * its effective set, returning 0 if it does, -ve if it does not. Note that
> + * this uses current's subjective/effective credentials.
> *
> * NOTE WELL: cap_capable() cannot be used like the kernel's capable()
> * function. That is, it has the reverse semantics: cap_capable() returns 0
> * when a task has a capability, but the kernel's capable() returns 1 for this
> * case.
> */
> -int cap_capable(struct task_struct *tsk, int cap, int audit)
> +int cap_capable(int cap, int audit)
> {
> - __u32 cap_raised;
> + return cap_raised(current_cap(), cap) ? 0 : -EPERM;
> +}
>
> - /* Derived from include/linux/sched.h:capable. */
> - rcu_read_lock();
> - cap_raised = cap_raised(__task_cred(tsk)->cap_effective, cap);
> - rcu_read_unlock();
> - return cap_raised ? 0 : -EPERM;
> +/**
> + * cap_has_capability - Determine whether a task has a particular effective capability
> + * @tsk: The task to query
> + * @cred: The credentials to use
> + * @cap: The capability to check for
> + * @audit: Whether to write an audit message or not
> + *
> + * Determine whether the nominated task has the specified capability amongst
> + * its effective set, returning 0 if it does, -ve if it does not. Note that
> + * this uses the task's objective/real credentials.
> + *
> + * NOTE WELL: cap_has_capability() cannot be used like the kernel's
> + * has_capability() function. That is, it has the reverse semantics:
> + * cap_has_capability() returns 0 when a task has a capability, but the
> + * kernel's has_capability() returns 1 for this case.
> + */
> +int cap_task_capable(struct task_struct *tsk, const struct cred *cred, int cap,
> + int audit)
> +{
> + return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM;
> }
>
> /**
> @@ -160,7 +176,7 @@ static inline int cap_inh_is_capped(void)
> /* they are so limited unless the current task has the CAP_SETPCAP
> * capability
> */
> - if (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) == 0)
> + if (cap_capable(CAP_SETPCAP, SECURITY_CAP_AUDIT) == 0)
> return 0;
> #endif
> return 1;
> @@ -869,7 +885,7 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> & (new->securebits ^ arg2)) /*[1]*/
> || ((new->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/
> || (arg2 & ~(SECURE_ALL_LOCKS | SECURE_ALL_BITS)) /*[3]*/
> - || (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) != 0) /*[4]*/
> + || (cap_capable(CAP_SETPCAP, SECURITY_CAP_AUDIT) != 0) /*[4]*/
> /*
> * [1] no changing of bits that are locked
> * [2] no unlocking of locks
> @@ -950,7 +966,7 @@ int cap_vm_enough_memory(struct mm_struct *mm, long pages)
> {
> int cap_sys_admin = 0;
>
> - if (cap_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT) == 0)
> + if (cap_capable(CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT) == 0)
> cap_sys_admin = 1;
> return __vm_enough_memory(mm, pages, cap_sys_admin);
> }
> diff --git a/security/root_plug.c b/security/root_plug.c
> index 40fb4f1..559578f 100644
> --- a/security/root_plug.c
> +++ b/security/root_plug.c
> @@ -77,6 +77,7 @@ static struct security_operations rootplug_security_ops = {
> .capget = cap_capget,
> .capset = cap_capset,
> .capable = cap_capable,
> + .task_capable = cap_task_capable,
>
> .bprm_set_creds = cap_bprm_set_creds,
>
> diff --git a/security/security.c b/security/security.c
> index d85dbb3..9bbc8e5 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -154,14 +154,31 @@ int security_capset(struct cred *new, const struct cred *old,
> effective, inheritable, permitted);
> }
>
> -int security_capable(struct task_struct *tsk, int cap)
> +int security_capable(int cap)
> {
> - return security_ops->capable(tsk, cap, SECURITY_CAP_AUDIT);
> + return security_ops->capable(cap, SECURITY_CAP_AUDIT);
> }
>
> -int security_capable_noaudit(struct task_struct *tsk, int cap)
> +int security_task_capable(struct task_struct *tsk, int cap)
> {
> - return security_ops->capable(tsk, cap, SECURITY_CAP_NOAUDIT);
> + const struct cred *cred;
> + int ret;
> +
> + cred = get_task_cred(tsk);
> + ret = security_ops->task_capable(tsk, cred, cap, SECURITY_CAP_AUDIT);
> + put_cred(cred);
> + return ret;
> +}
> +
> +int security_task_capable_noaudit(struct task_struct *tsk, int cap)
> +{
> + const struct cred *cred;
> + int ret;
> +
> + cred = get_task_cred(tsk);
> + ret = security_ops->task_capable(tsk, cred, cap, SECURITY_CAP_NOAUDIT);
> + put_cred(cred);
> + return ret;
> }
>
> int security_acct(struct file *file)
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index dbeaa78..5a66cd3 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1433,12 +1433,13 @@ static int current_has_perm(const struct task_struct *tsk,
>
> /* Check whether a task is allowed to use a capability. */
> static int task_has_capability(struct task_struct *tsk,
> + const struct cred *cred,
> int cap, int audit)
> {
> struct avc_audit_data ad;
> struct av_decision avd;
> u16 sclass;
> - u32 sid = task_sid(tsk);
> + u32 sid = cred_sid(cred);
> u32 av = CAP_TO_MASK(cap);
> int rc;
>
> @@ -1865,15 +1866,27 @@ static int selinux_capset(struct cred *new, const struct cred *old,
> return cred_has_perm(old, new, PROCESS__SETCAP);
> }
>
> -static int selinux_capable(struct task_struct *tsk, int cap, int audit)
> +static int selinux_capable(int cap, int audit)
> +{
> + int rc;
> +
> + rc = secondary_ops->capable(cap, audit);
> + if (rc)
> + return rc;
> +
> + return task_has_capability(current, current_cred(), cap, audit);
> +}
> +
> +static int selinux_task_capable(struct task_struct *tsk,
> + const struct cred *cred, int cap, int audit)
> {
> int rc;
>
> - rc = secondary_ops->capable(tsk, cap, audit);
> + rc = secondary_ops->task_capable(tsk, cred, cap, audit);
> if (rc)
> return rc;
>
> - return task_has_capability(tsk, cap, audit);
> + return task_has_capability(tsk, cred, cap, audit);
> }
>
> static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid)
> @@ -2037,7 +2050,7 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages)
> {
> int rc, cap_sys_admin = 0;
>
> - rc = selinux_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT);
> + rc = selinux_capable(CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT);
> if (rc == 0)
> cap_sys_admin = 1;
>
> @@ -2880,7 +2893,7 @@ static int selinux_inode_getsecurity(const struct inode *inode, const char *name
> * and lack of permission just means that we fall back to the
> * in-core context value, not a denial.
> */
> - error = selinux_capable(current, CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT);
> + error = selinux_capable(CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT);
> if (!error)
> error = security_sid_to_context_force(isec->sid, &context,
> &size);
> @@ -5568,6 +5581,7 @@ static struct security_operations selinux_ops = {
> .capset = selinux_capset,
> .sysctl = selinux_sysctl,
> .capable = selinux_capable,
> + .task_capable = selinux_task_capable,
> .quotactl = selinux_quotactl,
> .quota_on = selinux_quota_on,
> .syslog = selinux_syslog,
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 1b5551d..8e53d43 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -2628,6 +2628,7 @@ struct security_operations smack_ops = {
> .capget = cap_capget,
> .capset = cap_capset,
> .capable = cap_capable,
> + .task_capable = cap_task_capable,
> .syslog = smack_syslog,
> .settime = cap_settime,
> .vm_enough_memory = cap_vm_enough_memory,
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2008-12-31 15:15 ` [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2] David Howells
2008-12-31 23:24 ` James Morris
@ 2009-01-01 23:53 ` J. Bruce Fields
2009-01-02 1:19 ` David Howells
2009-01-02 16:48 ` [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2] J. Bruce Fields
2009-01-05 2:07 ` James Morris
3 siblings, 1 reply; 41+ messages in thread
From: J. Bruce Fields @ 2009-01-01 23:53 UTC (permalink / raw)
To: David Howells; +Cc: Christoph Hellwig, jmorris, linux-kernel, linux-fsdevel
On Wed, Dec 31, 2008 at 03:15:42PM +0000, David Howells wrote:
>
> Here's an improved patch. It differentiates the use of objective and
> subjective capabilities by making capable() only check current's subjective
> caps, but making has_capability() check only the objective caps of whatever
> process is specified.
>
> It's a bit more involved, but I think it's the right thing to do.
Hm. newpynfs is also giving me failures having to do with the v4
server's permissions-checking. I'll investigate, but is it possible
nfsd also needs a fix?
--b.
>
> David
>
> ---
> From: David Howells <dhowells@redhat.com>
> Subject: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat()
>
> Fix a regression in cap_capable() due to:
>
> commit 5ff7711e635b32f0a1e558227d030c7e45b4a465
> Author: David Howells <dhowells@redhat.com>
> Date: Wed Dec 31 02:52:28 2008 +0000
>
> CRED: Differentiate objective and effective subjective credentials on a task
>
>
> The problem is that the above patch allows a process to have two sets of
> credentials, and for the most part uses the subjective credentials when
> accessing current's creds.
>
> There is, however, one exception: cap_capable(), and thus capable(), uses the
> real/objective credentials of the target task, whether or not it is the current
> task.
>
> Ordinarily this doesn't matter, since usually the two cred pointers in current
> point to the same set of creds. However, sys_faccessat() makes use of this
> facility to override the credentials of the calling process to make its test,
> without affecting the creds as seen from other processes.
>
> One of the things sys_faccessat() does is to make an adjustment to the
> effective capabilities mask, which cap_capable(), as it stands, then ignores.
>
> The affected capability check is in generic_permission():
>
> if (!(mask & MAY_EXEC) || execute_ok(inode))
> if (capable(CAP_DAC_OVERRIDE))
> return 0;
>
>
> This change splits capable() from has_capability() down into the commoncap and
> SELinux code. The capable() security op now only deals with the current
> process, and uses the current process's subjective creds. A new security op -
> task_capable() - is introduced that can check any task's objective creds.
>
> strictly the capable() security op is superfluous with the presence of the
> task_capable() op, however it should be faster to call the capable() op since
> two fewer arguments need be passed down through the various layers.
>
>
> This can be tested by compiling the following program from the XFS testsuite:
>
> /*
> * t_access_root.c - trivial test program to show permission bug.
> *
> * Written by Michael Kerrisk - copyright ownership not pursued.
> * Sourced from: http://linux.derkeiler.com/Mailing-Lists/Kernel/2003-10/6030.html
> */
> #include <limits.h>
> #include <unistd.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <fcntl.h>
> #include <sys/stat.h>
>
> #define UID 500
> #define GID 100
> #define PERM 0
> #define TESTPATH "/tmp/t_access"
>
> static void
> errExit(char *msg)
> {
> perror(msg);
> exit(EXIT_FAILURE);
> } /* errExit */
>
> static void
> accessTest(char *file, int mask, char *mstr)
> {
> printf("access(%s, %s) returns %d\n", file, mstr, access(file, mask));
> } /* accessTest */
>
> int
> main(int argc, char *argv[])
> {
> int fd, perm, uid, gid;
> char *testpath;
> char cmd[PATH_MAX + 20];
>
> testpath = (argc > 1) ? argv[1] : TESTPATH;
> perm = (argc > 2) ? strtoul(argv[2], NULL, 8) : PERM;
> uid = (argc > 3) ? atoi(argv[3]) : UID;
> gid = (argc > 4) ? atoi(argv[4]) : GID;
>
> unlink(testpath);
>
> fd = open(testpath, O_RDWR | O_CREAT, 0);
> if (fd == -1) errExit("open");
>
> if (fchown(fd, uid, gid) == -1) errExit("fchown");
> if (fchmod(fd, perm) == -1) errExit("fchmod");
> close(fd);
>
> snprintf(cmd, sizeof(cmd), "ls -l %s", testpath);
> system(cmd);
>
> if (seteuid(uid) == -1) errExit("seteuid");
>
> accessTest(testpath, 0, "0");
> accessTest(testpath, R_OK, "R_OK");
> accessTest(testpath, W_OK, "W_OK");
> accessTest(testpath, X_OK, "X_OK");
> accessTest(testpath, R_OK | W_OK, "R_OK | W_OK");
> accessTest(testpath, R_OK | X_OK, "R_OK | X_OK");
> accessTest(testpath, W_OK | X_OK, "W_OK | X_OK");
> accessTest(testpath, R_OK | W_OK | X_OK, "R_OK | W_OK | X_OK");
>
> exit(EXIT_SUCCESS);
> } /* main */
>
>
> This can be run against an Ext3 filesystem as well as against an XFS
> filesystem. If successful, it will show:
>
> [root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043
> ---------- 1 dhowells dhowells 0 2008-12-31 03:00 /tmp/xxx
> access(/tmp/xxx, 0) returns 0
> access(/tmp/xxx, R_OK) returns 0
> access(/tmp/xxx, W_OK) returns 0
> access(/tmp/xxx, X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK) returns 0
> access(/tmp/xxx, R_OK | X_OK) returns -1
> access(/tmp/xxx, W_OK | X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1
>
> If unsuccessful, it will show:
>
> [root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043
> ---------- 1 dhowells dhowells 0 2008-12-31 02:56 /tmp/xxx
> access(/tmp/xxx, 0) returns 0
> access(/tmp/xxx, R_OK) returns -1
> access(/tmp/xxx, W_OK) returns -1
> access(/tmp/xxx, X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK) returns -1
> access(/tmp/xxx, R_OK | X_OK) returns -1
> access(/tmp/xxx, W_OK | X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1
>
> I've also tested the fix with the SELinux and syscalls LTP testsuites.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> include/linux/capability.h | 17 +++++++++++++--
> include/linux/security.h | 49 ++++++++++++++++++++++++++++++++++++--------
> kernel/capability.c | 2 +-
> security/capability.c | 1 +
> security/commoncap.c | 42 ++++++++++++++++++++++++++------------
> security/root_plug.c | 1 +
> security/security.c | 25 +++++++++++++++++++---
> security/selinux/hooks.c | 26 ++++++++++++++++++-----
> security/smack/smack_lsm.c | 1 +
> 9 files changed, 129 insertions(+), 35 deletions(-)
>
>
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index e22f48c..5b8a132 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -529,8 +529,21 @@ extern const kernel_cap_t __cap_init_eff_set;
> *
> * Note that this does not set PF_SUPERPRIV on the task.
> */
> -#define has_capability(t, cap) (security_capable((t), (cap)) == 0)
> -#define has_capability_noaudit(t, cap) (security_capable_noaudit((t), (cap)) == 0)
> +#define has_capability(t, cap) (security_task_capable((t), (cap)) == 0)
> +
> +/**
> + * has_capability_noaudit - Determine if a task has a superior capability available (unaudited)
> + * @t: The task in question
> + * @cap: The capability to be tested for
> + *
> + * Return true if the specified task has the given superior capability
> + * currently in effect, false if not, but don't write an audit message for the
> + * check.
> + *
> + * Note that this does not set PF_SUPERPRIV on the task.
> + */
> +#define has_capability_noaudit(t, cap) \
> + (security_task_capable_noaudit((t), (cap)) == 0)
>
> extern int capable(int cap);
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 3416cb8..76989b8 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -48,7 +48,9 @@ struct audit_krule;
> * These functions are in security/capability.c and are used
> * as the default capabilities functions
> */
> -extern int cap_capable(struct task_struct *tsk, int cap, int audit);
> +extern int cap_capable(int cap, int audit);
> +extern int cap_task_capable(struct task_struct *tsk, const struct cred *cred,
> + int cap, int audit);
> extern int cap_settime(struct timespec *ts, struct timezone *tz);
> extern int cap_ptrace_may_access(struct task_struct *child, unsigned int mode);
> extern int cap_ptrace_traceme(struct task_struct *parent);
> @@ -1195,9 +1197,18 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
> * @permitted contains the permitted capability set.
> * Return 0 and update @new if permission is granted.
> * @capable:
> - * Check whether the @tsk process has the @cap capability.
> + * Check whether the current process has the @cap capability in its
> + * subjective/effective credentials.
> + * @cap contains the capability <include/linux/capability.h>.
> + * @audit: Whether to write an audit message or not
> + * Return 0 if the capability is granted for @tsk.
> + * @task_capable:
> + * Check whether the @tsk process has the @cap capability in its
> + * objective/real credentials.
> * @tsk contains the task_struct for the process.
> + * @cred contains the credentials to use.
> * @cap contains the capability <include/linux/capability.h>.
> + * @audit: Whether to write an audit message or not
> * Return 0 if the capability is granted for @tsk.
> * @acct:
> * Check permission before enabling or disabling process accounting. If
> @@ -1290,7 +1301,9 @@ struct security_operations {
> const kernel_cap_t *effective,
> const kernel_cap_t *inheritable,
> const kernel_cap_t *permitted);
> - int (*capable) (struct task_struct *tsk, int cap, int audit);
> + int (*capable) (int cap, int audit);
> + int (*task_capable) (struct task_struct *tsk, const struct cred *cred,
> + int cap, int audit);
> int (*acct) (struct file *file);
> int (*sysctl) (struct ctl_table *table, int op);
> int (*quotactl) (int cmds, int type, int id, struct super_block *sb);
> @@ -1556,8 +1569,9 @@ int security_capset(struct cred *new, const struct cred *old,
> const kernel_cap_t *effective,
> const kernel_cap_t *inheritable,
> const kernel_cap_t *permitted);
> -int security_capable(struct task_struct *tsk, int cap);
> -int security_capable_noaudit(struct task_struct *tsk, int cap);
> +int security_capable(int cap);
> +int security_task_capable(struct task_struct *tsk, int cap);
> +int security_task_capable_noaudit(struct task_struct *tsk, int cap);
> int security_acct(struct file *file);
> int security_sysctl(struct ctl_table *table, int op);
> int security_quotactl(int cmds, int type, int id, struct super_block *sb);
> @@ -1754,14 +1768,31 @@ static inline int security_capset(struct cred *new,
> return cap_capset(new, old, effective, inheritable, permitted);
> }
>
> -static inline int security_capable(struct task_struct *tsk, int cap)
> +static inline int security_capable(int cap)
> {
> - return cap_capable(tsk, cap, SECURITY_CAP_AUDIT);
> + return cap_capable(cap, SECURITY_CAP_AUDIT);
> }
>
> -static inline int security_capable_noaudit(struct task_struct *tsk, int cap)
> +static inline int security_task_capable(struct task_struct *tsk, int cap)
> {
> - return cap_capable(tsk, cap, SECURITY_CAP_NOAUDIT);
> + int ret;
> +
> + rcu_read_lock();
> + ret = cap_task_capable(tsk, __task_cred(tsk), cap, SECURITY_CAP_AUDIT);
> + rcu_read_unlock();
> + return ret;
> +}
> +
> +static inline
> +int security_task_capable_noaudit(struct task_struct *tsk, int cap)
> +{
> + int ret;
> +
> + rcu_read_lock();
> + ret = cap_task_capable(tsk, __task_cred(tsk), cap,
> + SECURITY_CAP_NOAUDIT);
> + rcu_read_unlock();
> + return ret;
> }
>
> static inline int security_acct(struct file *file)
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 36b4b4d..df62f53 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -308,7 +308,7 @@ int capable(int cap)
> BUG();
> }
>
> - if (has_capability(current, cap)) {
> + if (security_capable(cap) == 0) {
> current->flags |= PF_SUPERPRIV;
> return 1;
> }
> diff --git a/security/capability.c b/security/capability.c
> index 2dce66f..fd1493d 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -826,6 +826,7 @@ void security_fixup_ops(struct security_operations *ops)
> set_to_cap_if_null(ops, capset);
> set_to_cap_if_null(ops, acct);
> set_to_cap_if_null(ops, capable);
> + set_to_cap_if_null(ops, task_capable);
> set_to_cap_if_null(ops, quotactl);
> set_to_cap_if_null(ops, quota_on);
> set_to_cap_if_null(ops, sysctl);
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 7971354..7f0b2a6 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -43,28 +43,44 @@ int cap_netlink_recv(struct sk_buff *skb, int cap)
> EXPORT_SYMBOL(cap_netlink_recv);
>
> /**
> - * cap_capable - Determine whether a task has a particular effective capability
> - * @tsk: The task to query
> + * cap_capable - Determine whether current has a particular effective capability
> * @cap: The capability to check for
> * @audit: Whether to write an audit message or not
> *
> * Determine whether the nominated task has the specified capability amongst
> - * its effective set, returning 0 if it does, -ve if it does not.
> + * its effective set, returning 0 if it does, -ve if it does not. Note that
> + * this uses current's subjective/effective credentials.
> *
> * NOTE WELL: cap_capable() cannot be used like the kernel's capable()
> * function. That is, it has the reverse semantics: cap_capable() returns 0
> * when a task has a capability, but the kernel's capable() returns 1 for this
> * case.
> */
> -int cap_capable(struct task_struct *tsk, int cap, int audit)
> +int cap_capable(int cap, int audit)
> {
> - __u32 cap_raised;
> + return cap_raised(current_cap(), cap) ? 0 : -EPERM;
> +}
>
> - /* Derived from include/linux/sched.h:capable. */
> - rcu_read_lock();
> - cap_raised = cap_raised(__task_cred(tsk)->cap_effective, cap);
> - rcu_read_unlock();
> - return cap_raised ? 0 : -EPERM;
> +/**
> + * cap_has_capability - Determine whether a task has a particular effective capability
> + * @tsk: The task to query
> + * @cred: The credentials to use
> + * @cap: The capability to check for
> + * @audit: Whether to write an audit message or not
> + *
> + * Determine whether the nominated task has the specified capability amongst
> + * its effective set, returning 0 if it does, -ve if it does not. Note that
> + * this uses the task's objective/real credentials.
> + *
> + * NOTE WELL: cap_has_capability() cannot be used like the kernel's
> + * has_capability() function. That is, it has the reverse semantics:
> + * cap_has_capability() returns 0 when a task has a capability, but the
> + * kernel's has_capability() returns 1 for this case.
> + */
> +int cap_task_capable(struct task_struct *tsk, const struct cred *cred, int cap,
> + int audit)
> +{
> + return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM;
> }
>
> /**
> @@ -160,7 +176,7 @@ static inline int cap_inh_is_capped(void)
> /* they are so limited unless the current task has the CAP_SETPCAP
> * capability
> */
> - if (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) == 0)
> + if (cap_capable(CAP_SETPCAP, SECURITY_CAP_AUDIT) == 0)
> return 0;
> #endif
> return 1;
> @@ -869,7 +885,7 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> & (new->securebits ^ arg2)) /*[1]*/
> || ((new->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/
> || (arg2 & ~(SECURE_ALL_LOCKS | SECURE_ALL_BITS)) /*[3]*/
> - || (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) != 0) /*[4]*/
> + || (cap_capable(CAP_SETPCAP, SECURITY_CAP_AUDIT) != 0) /*[4]*/
> /*
> * [1] no changing of bits that are locked
> * [2] no unlocking of locks
> @@ -950,7 +966,7 @@ int cap_vm_enough_memory(struct mm_struct *mm, long pages)
> {
> int cap_sys_admin = 0;
>
> - if (cap_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT) == 0)
> + if (cap_capable(CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT) == 0)
> cap_sys_admin = 1;
> return __vm_enough_memory(mm, pages, cap_sys_admin);
> }
> diff --git a/security/root_plug.c b/security/root_plug.c
> index 40fb4f1..559578f 100644
> --- a/security/root_plug.c
> +++ b/security/root_plug.c
> @@ -77,6 +77,7 @@ static struct security_operations rootplug_security_ops = {
> .capget = cap_capget,
> .capset = cap_capset,
> .capable = cap_capable,
> + .task_capable = cap_task_capable,
>
> .bprm_set_creds = cap_bprm_set_creds,
>
> diff --git a/security/security.c b/security/security.c
> index d85dbb3..9bbc8e5 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -154,14 +154,31 @@ int security_capset(struct cred *new, const struct cred *old,
> effective, inheritable, permitted);
> }
>
> -int security_capable(struct task_struct *tsk, int cap)
> +int security_capable(int cap)
> {
> - return security_ops->capable(tsk, cap, SECURITY_CAP_AUDIT);
> + return security_ops->capable(cap, SECURITY_CAP_AUDIT);
> }
>
> -int security_capable_noaudit(struct task_struct *tsk, int cap)
> +int security_task_capable(struct task_struct *tsk, int cap)
> {
> - return security_ops->capable(tsk, cap, SECURITY_CAP_NOAUDIT);
> + const struct cred *cred;
> + int ret;
> +
> + cred = get_task_cred(tsk);
> + ret = security_ops->task_capable(tsk, cred, cap, SECURITY_CAP_AUDIT);
> + put_cred(cred);
> + return ret;
> +}
> +
> +int security_task_capable_noaudit(struct task_struct *tsk, int cap)
> +{
> + const struct cred *cred;
> + int ret;
> +
> + cred = get_task_cred(tsk);
> + ret = security_ops->task_capable(tsk, cred, cap, SECURITY_CAP_NOAUDIT);
> + put_cred(cred);
> + return ret;
> }
>
> int security_acct(struct file *file)
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index dbeaa78..5a66cd3 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1433,12 +1433,13 @@ static int current_has_perm(const struct task_struct *tsk,
>
> /* Check whether a task is allowed to use a capability. */
> static int task_has_capability(struct task_struct *tsk,
> + const struct cred *cred,
> int cap, int audit)
> {
> struct avc_audit_data ad;
> struct av_decision avd;
> u16 sclass;
> - u32 sid = task_sid(tsk);
> + u32 sid = cred_sid(cred);
> u32 av = CAP_TO_MASK(cap);
> int rc;
>
> @@ -1865,15 +1866,27 @@ static int selinux_capset(struct cred *new, const struct cred *old,
> return cred_has_perm(old, new, PROCESS__SETCAP);
> }
>
> -static int selinux_capable(struct task_struct *tsk, int cap, int audit)
> +static int selinux_capable(int cap, int audit)
> +{
> + int rc;
> +
> + rc = secondary_ops->capable(cap, audit);
> + if (rc)
> + return rc;
> +
> + return task_has_capability(current, current_cred(), cap, audit);
> +}
> +
> +static int selinux_task_capable(struct task_struct *tsk,
> + const struct cred *cred, int cap, int audit)
> {
> int rc;
>
> - rc = secondary_ops->capable(tsk, cap, audit);
> + rc = secondary_ops->task_capable(tsk, cred, cap, audit);
> if (rc)
> return rc;
>
> - return task_has_capability(tsk, cap, audit);
> + return task_has_capability(tsk, cred, cap, audit);
> }
>
> static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid)
> @@ -2037,7 +2050,7 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages)
> {
> int rc, cap_sys_admin = 0;
>
> - rc = selinux_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT);
> + rc = selinux_capable(CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT);
> if (rc == 0)
> cap_sys_admin = 1;
>
> @@ -2880,7 +2893,7 @@ static int selinux_inode_getsecurity(const struct inode *inode, const char *name
> * and lack of permission just means that we fall back to the
> * in-core context value, not a denial.
> */
> - error = selinux_capable(current, CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT);
> + error = selinux_capable(CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT);
> if (!error)
> error = security_sid_to_context_force(isec->sid, &context,
> &size);
> @@ -5568,6 +5581,7 @@ static struct security_operations selinux_ops = {
> .capset = selinux_capset,
> .sysctl = selinux_sysctl,
> .capable = selinux_capable,
> + .task_capable = selinux_task_capable,
> .quotactl = selinux_quotactl,
> .quota_on = selinux_quota_on,
> .syslog = selinux_syslog,
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 1b5551d..8e53d43 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -2628,6 +2628,7 @@ struct security_operations smack_ops = {
> .capget = cap_capget,
> .capset = cap_capset,
> .capable = cap_capable,
> + .task_capable = cap_task_capable,
> .syslog = smack_syslog,
> .settime = cap_settime,
> .vm_enough_memory = cap_vm_enough_memory,
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-01 23:53 ` J. Bruce Fields
@ 2009-01-02 1:19 ` David Howells
2009-01-02 5:19 ` J. Bruce Fields
0 siblings, 1 reply; 41+ messages in thread
From: David Howells @ 2009-01-02 1:19 UTC (permalink / raw)
To: J. Bruce Fields
Cc: dhowells, Christoph Hellwig, jmorris, linux-kernel, linux-fsdevel
J. Bruce Fields <bfields@fieldses.org> wrote:
> Hm. newpynfs is also giving me failures having to do with the v4
> server's permissions-checking. I'll investigate, but is it possible
> nfsd also needs a fix?
It's possible. Have you seen whether this patch fixes those failures also?
nfsd uses this facility also.
David
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-02 1:19 ` David Howells
@ 2009-01-02 5:19 ` J. Bruce Fields
2009-01-02 11:59 ` David Howells
0 siblings, 1 reply; 41+ messages in thread
From: J. Bruce Fields @ 2009-01-02 5:19 UTC (permalink / raw)
To: David Howells; +Cc: Christoph Hellwig, jmorris, linux-kernel, linux-fsdevel
On Fri, Jan 02, 2009 at 01:19:12AM +0000, David Howells wrote:
> J. Bruce Fields <bfields@fieldses.org> wrote:
>
> > Hm. newpynfs is also giving me failures having to do with the v4
> > server's permissions-checking. I'll investigate, but is it possible
> > nfsd also needs a fix?
>
> It's possible. Have you seen whether this patch fixes those failures also?
> nfsd uses this facility also.
No. I started bisecting, and it does appear to be a regression from the
cred patches, but at some point in the middle there it hangs on boot (a
softlockup report blames a spinlock in set_groups).
--b.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-02 5:19 ` J. Bruce Fields
@ 2009-01-02 11:59 ` David Howells
2009-01-02 16:45 ` J. Bruce Fields
0 siblings, 1 reply; 41+ messages in thread
From: David Howells @ 2009-01-02 11:59 UTC (permalink / raw)
To: J. Bruce Fields
Cc: dhowells, Christoph Hellwig, jmorris, linux-kernel, linux-fsdevel
J. Bruce Fields <bfields@fieldses.org> wrote:
> No. I started bisecting, and it does appear to be a regression from the
> cred patches, but at some point in the middle there it hangs on boot (a
> softlockup report blames a spinlock in set_groups).
Do you remember which patch you were at?
David
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-02 11:59 ` David Howells
@ 2009-01-02 16:45 ` J. Bruce Fields
2009-01-03 18:49 ` J. Bruce Fields
` (2 more replies)
0 siblings, 3 replies; 41+ messages in thread
From: J. Bruce Fields @ 2009-01-02 16:45 UTC (permalink / raw)
To: David Howells; +Cc: Christoph Hellwig, jmorris, linux-kernel, linux-fsdevel
On Fri, Jan 02, 2009 at 11:59:38AM +0000, David Howells wrote:
> J. Bruce Fields <bfields@fieldses.org> wrote:
>
> > No. I started bisecting, and it does appear to be a regression from the
> > cred patches, but at some point in the middle there it hangs on boot (a
> > softlockup report blames a spinlock in set_groups).
>
> Do you remember which patch you were at?
It appears that:
- 1cdcbec1a3372c0c49c59d292e708fd07b509f18 "CRED: Neuter
sys_capset()" is good
- 98870ab0a5a3f1822aee681d2997017e1c87d026 "CRED: Documentation"
is bad
- f1752eec6145c97163dbce62d17cf5d928e28a27 and
d84f4f992cbd76e8f39c488cf0c5d123843923b1 produce the soft
lookup in set_groups()
... and I haven't figured out what's in between. And the test failure
is nfsd_lookup() returning OK on a directory when it should return
nfserr_perm. I assume that's the result of inode_permission(directory
inode, MAY_EXEC) returning 0 when it shouldn't, but I haven't confirmed
that.
--b.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-02 16:45 ` J. Bruce Fields
@ 2009-01-03 18:49 ` J. Bruce Fields
2009-01-03 23:03 ` David Howells
2009-01-05 13:11 ` David Howells
2009-01-05 15:57 ` David Howells
2009-01-05 16:48 ` David Howells
2 siblings, 2 replies; 41+ messages in thread
From: J. Bruce Fields @ 2009-01-03 18:49 UTC (permalink / raw)
To: David Howells; +Cc: Christoph Hellwig, jmorris, linux-kernel, linux-fsdevel
On Fri, Jan 02, 2009 at 11:45:05AM -0500, bfields wrote:
> On Fri, Jan 02, 2009 at 11:59:38AM +0000, David Howells wrote:
> > J. Bruce Fields <bfields@fieldses.org> wrote:
> >
> > > No. I started bisecting, and it does appear to be a regression from the
> > > cred patches, but at some point in the middle there it hangs on boot (a
> > > softlockup report blames a spinlock in set_groups).
> >
> > Do you remember which patch you were at?
More precisely:
- The last working commit is b6dff3ec... "CRED: Separate task
security context from task_struct".
- The first commit exhibiting the permissions problem is
a6f76f2... "CRED: Make execve() take advantage of
copy-on-write credentials".
- The 9 commits in between (from f1752eec to d84f4f9) result in
a soft lookup on boot.
--b.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-03 18:49 ` J. Bruce Fields
@ 2009-01-03 23:03 ` David Howells
2009-01-04 2:03 ` J. Bruce Fields
2009-01-05 13:11 ` David Howells
1 sibling, 1 reply; 41+ messages in thread
From: David Howells @ 2009-01-03 23:03 UTC (permalink / raw)
To: J. Bruce Fields
Cc: dhowells, Christoph Hellwig, jmorris, linux-kernel, linux-fsdevel
J. Bruce Fields <bfields@fieldses.org> wrote:
> More precisely:
> - The last working commit is b6dff3ec... "CRED: Separate task
> security context from task_struct".
> - The first commit exhibiting the permissions problem is
> a6f76f2... "CRED: Make execve() take advantage of
> copy-on-write credentials".
> - The 9 commits in between (from f1752eec to d84f4f9) result in
> a soft lookup on boot.
Okay, I'll have a look at that, but did you manage to find out if the patch I
posted fixed the problem you originally mentioned?
David
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-03 23:03 ` David Howells
@ 2009-01-04 2:03 ` J. Bruce Fields
0 siblings, 0 replies; 41+ messages in thread
From: J. Bruce Fields @ 2009-01-04 2:03 UTC (permalink / raw)
To: David Howells; +Cc: Christoph Hellwig, jmorris, linux-kernel, linux-fsdevel
On Sat, Jan 03, 2009 at 11:03:26PM +0000, David Howells wrote:
> J. Bruce Fields <bfields@fieldses.org> wrote:
>
> > More precisely:
> > - The last working commit is b6dff3ec... "CRED: Separate task
> > security context from task_struct".
> > - The first commit exhibiting the permissions problem is
> > a6f76f2... "CRED: Make execve() take advantage of
> > copy-on-write credentials".
> > - The 9 commits in between (from f1752eec to d84f4f9) result in
> > a soft lookup on boot.
>
> Okay, I'll have a look at that, but did you manage to find out if the patch I
> posted fixed the problem you originally mentioned?
I tested that patch, yes, but it didn't fix the problem.
--b.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-03 18:49 ` J. Bruce Fields
2009-01-03 23:03 ` David Howells
@ 2009-01-05 13:11 ` David Howells
1 sibling, 0 replies; 41+ messages in thread
From: David Howells @ 2009-01-05 13:11 UTC (permalink / raw)
To: J. Bruce Fields
Cc: dhowells, Christoph Hellwig, jmorris, linux-kernel, linux-fsdevel
J. Bruce Fields <bfields@fieldses.org> wrote:
> > > > No. I started bisecting, and it does appear to be a regression from the
> > > > cred patches, but at some point in the middle there it hangs on boot (a
> > > > softlockup report blames a spinlock in set_groups).
> > >
> > > Do you remember which patch you were at?
>
> More precisely:
> - The last working commit is b6dff3ec... "CRED: Separate task
> security context from task_struct".
> - The first commit exhibiting the permissions problem is
> a6f76f2... "CRED: Make execve() take advantage of
> copy-on-write credentials".
I presume by 'first' you mean 'latest'.
> - The 9 commits in between (from f1752eec to d84f4f9) result in
> a soft lookup on boot.
I think the problem may be that f1752eec removes the lock initialisation for
init_cred from the INIT_CRED() macro:
- .lock = __SPIN_LOCK_UNLOCKED(p.lock), \
but doesn't add it to the out of line init_cred:
+struct cred init_cred = {
+ .usage = ATOMIC_INIT(3),
+ .securebits = SECUREBITS_DEFAULT,
+ .cap_inheritable = CAP_INIT_INH_SET,
+ .cap_permitted = CAP_FULL_SET,
+ .cap_effective = CAP_INIT_EFF_SET,
+ .cap_bset = CAP_INIT_BSET,
+ .user = INIT_USER,
+ .group_info = &init_groups,
+};
Can you try adding:
.lock = __SPIN_LOCK_UNLOCKED(init_cred.lock),
to that and also add:
spin_lock_init(&pcred->lock);
into copy_creds() see if the problem goes away?
I'm a bit surprised that lockdep doesn't bark at this one - I thought it
checked for lock initialisation.
David
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-02 16:45 ` J. Bruce Fields
2009-01-03 18:49 ` J. Bruce Fields
@ 2009-01-05 15:57 ` David Howells
2009-01-05 16:48 ` David Howells
2 siblings, 0 replies; 41+ messages in thread
From: David Howells @ 2009-01-05 15:57 UTC (permalink / raw)
To: J. Bruce Fields
Cc: dhowells, Christoph Hellwig, jmorris, linux-kernel, linux-fsdevel
J. Bruce Fields <bfields@fieldses.org> wrote:
> ... and I haven't figured out what's in between. And the test failure
> is nfsd_lookup() returning OK on a directory when it should return
> nfserr_perm. I assume that's the result of inode_permission(directory
> inode, MAY_EXEC) returning 0 when it shouldn't, but I haven't confirmed
> that.
I appear to have reproduced this on my test machine. I'll have a poke around.
David
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-02 16:45 ` J. Bruce Fields
2009-01-03 18:49 ` J. Bruce Fields
2009-01-05 15:57 ` David Howells
@ 2009-01-05 16:48 ` David Howells
2009-01-05 17:19 ` [PATCH] CRED: Fix NFSD regression David Howells
2 siblings, 1 reply; 41+ messages in thread
From: David Howells @ 2009-01-05 16:48 UTC (permalink / raw)
To: J. Bruce Fields
Cc: dhowells, Christoph Hellwig, jmorris, linux-kernel, linux-fsdevel
J. Bruce Fields <bfields@fieldses.org> wrote:
> And the test failure
> is nfsd_lookup() returning OK on a directory when it should return
> nfserr_perm. I assume that's the result of inode_permission(directory
> inode, MAY_EXEC) returning 0 when it shouldn't, but I haven't confirmed
> that.
The first problem directly related to this is, I think, addressed by the
attached patch.
There seems to be more to it, though.
David
---
diff --git a/fs/nfsd/auth.c b/fs/nfsd/auth.c
index 0184fe9..64bc1f5 100644
--- a/fs/nfsd/auth.c
+++ b/fs/nfsd/auth.c
@@ -76,7 +76,7 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct svc_export *exp)
ret = set_groups(new, gi);
put_group_info(gi);
- if (!ret)
+ if (ret < 0)
goto error;
if (new->uid)
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH] CRED: Fix NFSD regression
2009-01-05 16:48 ` David Howells
@ 2009-01-05 17:19 ` David Howells
2009-01-05 22:22 ` James Morris
2009-01-06 19:41 ` J. Bruce Fields
0 siblings, 2 replies; 41+ messages in thread
From: David Howells @ 2009-01-05 17:19 UTC (permalink / raw)
To: bfields; +Cc: dhowells, hch, jmorris, linux-kernel, linux-fsdevel
Fix a regression in NFSD's permission checking introduced by the credentials
patches. There are two parts to the problem, both in nfsd_setuser():
(1) The return value of set_groups() is -ve if in error, not 0, and should be
checked appropriately. 0 indicates success.
(2) The UID to use for fs accesses is in new->fsuid, not new->uid (which is
0). This causes CAP_DAC_OVERRIDE to always be set, rather than being
cleared if the UID is anything other than 0 after squashing.
Reported-by: J. Bruce Fields <bfields@fieldses.org>
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/nfsd/auth.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/auth.c b/fs/nfsd/auth.c
index 0184fe9..c903e04 100644
--- a/fs/nfsd/auth.c
+++ b/fs/nfsd/auth.c
@@ -76,10 +76,10 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct svc_export *exp)
ret = set_groups(new, gi);
put_group_info(gi);
- if (!ret)
+ if (ret < 0)
goto error;
- if (new->uid)
+ if (new->fsuid)
new->cap_effective = cap_drop_nfsd_set(new->cap_effective);
else
new->cap_effective = cap_raise_nfsd_set(new->cap_effective,
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH] CRED: Fix NFSD regression
2009-01-05 17:19 ` [PATCH] CRED: Fix NFSD regression David Howells
@ 2009-01-05 22:22 ` James Morris
2009-01-06 19:41 ` J. Bruce Fields
1 sibling, 0 replies; 41+ messages in thread
From: James Morris @ 2009-01-05 22:22 UTC (permalink / raw)
To: David Howells; +Cc: bfields, hch, linux-kernel, linux-fsdevel
On Mon, 5 Jan 2009, David Howells wrote:
> Fix a regression in NFSD's permission checking introduced by the credentials
> patches. There are two parts to the problem, both in nfsd_setuser():
>
> (1) The return value of set_groups() is -ve if in error, not 0, and should be
> checked appropriately. 0 indicates success.
>
> (2) The UID to use for fs accesses is in new->fsuid, not new->uid (which is
> 0). This causes CAP_DAC_OVERRIDE to always be set, rather than being
> cleared if the UID is anything other than 0 after squashing.
>
> Reported-by: J. Bruce Fields <bfields@fieldses.org>
> Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: James Morris <jmorris@namei.org>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix NFSD regression
2009-01-05 17:19 ` [PATCH] CRED: Fix NFSD regression David Howells
2009-01-05 22:22 ` James Morris
@ 2009-01-06 19:41 ` J. Bruce Fields
2009-01-06 19:56 ` David Howells
1 sibling, 1 reply; 41+ messages in thread
From: J. Bruce Fields @ 2009-01-06 19:41 UTC (permalink / raw)
To: David Howells; +Cc: hch, jmorris, linux-kernel, linux-fsdevel
On Mon, Jan 05, 2009 at 05:19:37PM +0000, David Howells wrote:
> Fix a regression in NFSD's permission checking introduced by the credentials
> patches. There are two parts to the problem, both in nfsd_setuser():
>
> (1) The return value of set_groups() is -ve if in error, not 0, and should be
> checked appropriately. 0 indicates success.
>
> (2) The UID to use for fs accesses is in new->fsuid, not new->uid (which is
> 0). This causes CAP_DAC_OVERRIDE to always be set, rather than being
> cleared if the UID is anything other than 0 after squashing.
>
> Reported-by: J. Bruce Fields <bfields@fieldses.org>
> Signed-off-by: David Howells <dhowells@redhat.com>
OK, I've tested this and applied it for 2.6.29.
Though it only actually fixes the regression if your other patch ("CRED:
Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2")
is also applied. I'd like to send in my pull request today, but I
usually try not to send a pull request whose tip doesn't pass at least
my basic regression tests.... And that other patch is outside my
baliwick. Can someone else handle it?
--b.
> ---
>
> fs/nfsd/auth.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
>
> diff --git a/fs/nfsd/auth.c b/fs/nfsd/auth.c
> index 0184fe9..c903e04 100644
> --- a/fs/nfsd/auth.c
> +++ b/fs/nfsd/auth.c
> @@ -76,10 +76,10 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct svc_export *exp)
>
> ret = set_groups(new, gi);
> put_group_info(gi);
> - if (!ret)
> + if (ret < 0)
> goto error;
>
> - if (new->uid)
> + if (new->fsuid)
> new->cap_effective = cap_drop_nfsd_set(new->cap_effective);
> else
> new->cap_effective = cap_raise_nfsd_set(new->cap_effective,
>
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH] CRED: Fix NFSD regression
2009-01-06 19:41 ` J. Bruce Fields
@ 2009-01-06 19:56 ` David Howells
2009-01-06 20:08 ` J. Bruce Fields
0 siblings, 1 reply; 41+ messages in thread
From: David Howells @ 2009-01-06 19:56 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: dhowells, hch, jmorris, linux-kernel, linux-fsdevel
J. Bruce Fields <bfields@fieldses.org> wrote:
> OK, I've tested this and applied it for 2.6.29.
>
> Though it only actually fixes the regression if your other patch ("CRED:
> Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2")
> is also applied. I'd like to send in my pull request today, but I
> usually try not to send a pull request whose tip doesn't pass at least
> my basic regression tests.... And that other patch is outside my
> baliwick. Can someone else handle it?
Well, if you're willing to supply an Acked-by or Tested-by for one or both
patches I can pass them on to James or Linus.
David
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH] CRED: Fix NFSD regression
2009-01-06 19:56 ` David Howells
@ 2009-01-06 20:08 ` J. Bruce Fields
0 siblings, 0 replies; 41+ messages in thread
From: J. Bruce Fields @ 2009-01-06 20:08 UTC (permalink / raw)
To: David Howells; +Cc: hch, jmorris, linux-kernel, linux-fsdevel
On Tue, Jan 06, 2009 at 07:56:11PM +0000, David Howells wrote:
> J. Bruce Fields <bfields@fieldses.org> wrote:
>
> > OK, I've tested this and applied it for 2.6.29.
> >
> > Though it only actually fixes the regression if your other patch ("CRED:
> > Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2")
> > is also applied. I'd like to send in my pull request today, but I
> > usually try not to send a pull request whose tip doesn't pass at least
> > my basic regression tests.... And that other patch is outside my
> > baliwick. Can someone else handle it?
>
> Well, if you're willing to supply an Acked-by or Tested-by for one or both
> patches I can pass them on to James or Linus.
The nfsd-specific one I've already signed off on and queued for Linus.
For the other one, sure, for what it's worth:
Tested-by: J. Bruce Fields <bfields@citi.umich.edu>
Thanks!--b.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2008-12-31 15:15 ` [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2] David Howells
2008-12-31 23:24 ` James Morris
2009-01-01 23:53 ` J. Bruce Fields
@ 2009-01-02 16:48 ` J. Bruce Fields
2009-01-02 19:18 ` David Howells
2009-01-05 2:07 ` James Morris
3 siblings, 1 reply; 41+ messages in thread
From: J. Bruce Fields @ 2009-01-02 16:48 UTC (permalink / raw)
To: David Howells; +Cc: Christoph Hellwig, jmorris, linux-kernel, linux-fsdevel
On Wed, Dec 31, 2008 at 03:15:42PM +0000, David Howells wrote:
>
> Here's an improved patch. It differentiates the use of objective and
> subjective capabilities by making capable() only check current's subjective
> caps, but making has_capability() check only the objective caps of whatever
> process is specified.
>
> It's a bit more involved, but I think it's the right thing to do.
>
> David
>
> ---
> From: David Howells <dhowells@redhat.com>
> Subject: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat()
>
> Fix a regression in cap_capable() due to:
>
> commit 5ff7711e635b32f0a1e558227d030c7e45b4a465
By the way, this should be 3b11a1decef07c19443d24ae926982bc8ec9f4c0,
which is the patch that was committed by James Morris and is in Linus's
tree. I assume 5ff7711... is the commitid in your tree and that James
applied it as a patch instead of pulling from you.
--b.
> Author: David Howells <dhowells@redhat.com>
> Date: Wed Dec 31 02:52:28 2008 +0000
>
> CRED: Differentiate objective and effective subjective credentials on a task
>
>
> The problem is that the above patch allows a process to have two sets of
> credentials, and for the most part uses the subjective credentials when
> accessing current's creds.
>
> There is, however, one exception: cap_capable(), and thus capable(), uses the
> real/objective credentials of the target task, whether or not it is the current
> task.
>
> Ordinarily this doesn't matter, since usually the two cred pointers in current
> point to the same set of creds. However, sys_faccessat() makes use of this
> facility to override the credentials of the calling process to make its test,
> without affecting the creds as seen from other processes.
>
> One of the things sys_faccessat() does is to make an adjustment to the
> effective capabilities mask, which cap_capable(), as it stands, then ignores.
>
> The affected capability check is in generic_permission():
>
> if (!(mask & MAY_EXEC) || execute_ok(inode))
> if (capable(CAP_DAC_OVERRIDE))
> return 0;
>
>
> This change splits capable() from has_capability() down into the commoncap and
> SELinux code. The capable() security op now only deals with the current
> process, and uses the current process's subjective creds. A new security op -
> task_capable() - is introduced that can check any task's objective creds.
>
> strictly the capable() security op is superfluous with the presence of the
> task_capable() op, however it should be faster to call the capable() op since
> two fewer arguments need be passed down through the various layers.
>
>
> This can be tested by compiling the following program from the XFS testsuite:
>
> /*
> * t_access_root.c - trivial test program to show permission bug.
> *
> * Written by Michael Kerrisk - copyright ownership not pursued.
> * Sourced from: http://linux.derkeiler.com/Mailing-Lists/Kernel/2003-10/6030.html
> */
> #include <limits.h>
> #include <unistd.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <fcntl.h>
> #include <sys/stat.h>
>
> #define UID 500
> #define GID 100
> #define PERM 0
> #define TESTPATH "/tmp/t_access"
>
> static void
> errExit(char *msg)
> {
> perror(msg);
> exit(EXIT_FAILURE);
> } /* errExit */
>
> static void
> accessTest(char *file, int mask, char *mstr)
> {
> printf("access(%s, %s) returns %d\n", file, mstr, access(file, mask));
> } /* accessTest */
>
> int
> main(int argc, char *argv[])
> {
> int fd, perm, uid, gid;
> char *testpath;
> char cmd[PATH_MAX + 20];
>
> testpath = (argc > 1) ? argv[1] : TESTPATH;
> perm = (argc > 2) ? strtoul(argv[2], NULL, 8) : PERM;
> uid = (argc > 3) ? atoi(argv[3]) : UID;
> gid = (argc > 4) ? atoi(argv[4]) : GID;
>
> unlink(testpath);
>
> fd = open(testpath, O_RDWR | O_CREAT, 0);
> if (fd == -1) errExit("open");
>
> if (fchown(fd, uid, gid) == -1) errExit("fchown");
> if (fchmod(fd, perm) == -1) errExit("fchmod");
> close(fd);
>
> snprintf(cmd, sizeof(cmd), "ls -l %s", testpath);
> system(cmd);
>
> if (seteuid(uid) == -1) errExit("seteuid");
>
> accessTest(testpath, 0, "0");
> accessTest(testpath, R_OK, "R_OK");
> accessTest(testpath, W_OK, "W_OK");
> accessTest(testpath, X_OK, "X_OK");
> accessTest(testpath, R_OK | W_OK, "R_OK | W_OK");
> accessTest(testpath, R_OK | X_OK, "R_OK | X_OK");
> accessTest(testpath, W_OK | X_OK, "W_OK | X_OK");
> accessTest(testpath, R_OK | W_OK | X_OK, "R_OK | W_OK | X_OK");
>
> exit(EXIT_SUCCESS);
> } /* main */
>
>
> This can be run against an Ext3 filesystem as well as against an XFS
> filesystem. If successful, it will show:
>
> [root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043
> ---------- 1 dhowells dhowells 0 2008-12-31 03:00 /tmp/xxx
> access(/tmp/xxx, 0) returns 0
> access(/tmp/xxx, R_OK) returns 0
> access(/tmp/xxx, W_OK) returns 0
> access(/tmp/xxx, X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK) returns 0
> access(/tmp/xxx, R_OK | X_OK) returns -1
> access(/tmp/xxx, W_OK | X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1
>
> If unsuccessful, it will show:
>
> [root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043
> ---------- 1 dhowells dhowells 0 2008-12-31 02:56 /tmp/xxx
> access(/tmp/xxx, 0) returns 0
> access(/tmp/xxx, R_OK) returns -1
> access(/tmp/xxx, W_OK) returns -1
> access(/tmp/xxx, X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK) returns -1
> access(/tmp/xxx, R_OK | X_OK) returns -1
> access(/tmp/xxx, W_OK | X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1
>
> I've also tested the fix with the SELinux and syscalls LTP testsuites.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> include/linux/capability.h | 17 +++++++++++++--
> include/linux/security.h | 49 ++++++++++++++++++++++++++++++++++++--------
> kernel/capability.c | 2 +-
> security/capability.c | 1 +
> security/commoncap.c | 42 ++++++++++++++++++++++++++------------
> security/root_plug.c | 1 +
> security/security.c | 25 +++++++++++++++++++---
> security/selinux/hooks.c | 26 ++++++++++++++++++-----
> security/smack/smack_lsm.c | 1 +
> 9 files changed, 129 insertions(+), 35 deletions(-)
>
>
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index e22f48c..5b8a132 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -529,8 +529,21 @@ extern const kernel_cap_t __cap_init_eff_set;
> *
> * Note that this does not set PF_SUPERPRIV on the task.
> */
> -#define has_capability(t, cap) (security_capable((t), (cap)) == 0)
> -#define has_capability_noaudit(t, cap) (security_capable_noaudit((t), (cap)) == 0)
> +#define has_capability(t, cap) (security_task_capable((t), (cap)) == 0)
> +
> +/**
> + * has_capability_noaudit - Determine if a task has a superior capability available (unaudited)
> + * @t: The task in question
> + * @cap: The capability to be tested for
> + *
> + * Return true if the specified task has the given superior capability
> + * currently in effect, false if not, but don't write an audit message for the
> + * check.
> + *
> + * Note that this does not set PF_SUPERPRIV on the task.
> + */
> +#define has_capability_noaudit(t, cap) \
> + (security_task_capable_noaudit((t), (cap)) == 0)
>
> extern int capable(int cap);
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 3416cb8..76989b8 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -48,7 +48,9 @@ struct audit_krule;
> * These functions are in security/capability.c and are used
> * as the default capabilities functions
> */
> -extern int cap_capable(struct task_struct *tsk, int cap, int audit);
> +extern int cap_capable(int cap, int audit);
> +extern int cap_task_capable(struct task_struct *tsk, const struct cred *cred,
> + int cap, int audit);
> extern int cap_settime(struct timespec *ts, struct timezone *tz);
> extern int cap_ptrace_may_access(struct task_struct *child, unsigned int mode);
> extern int cap_ptrace_traceme(struct task_struct *parent);
> @@ -1195,9 +1197,18 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
> * @permitted contains the permitted capability set.
> * Return 0 and update @new if permission is granted.
> * @capable:
> - * Check whether the @tsk process has the @cap capability.
> + * Check whether the current process has the @cap capability in its
> + * subjective/effective credentials.
> + * @cap contains the capability <include/linux/capability.h>.
> + * @audit: Whether to write an audit message or not
> + * Return 0 if the capability is granted for @tsk.
> + * @task_capable:
> + * Check whether the @tsk process has the @cap capability in its
> + * objective/real credentials.
> * @tsk contains the task_struct for the process.
> + * @cred contains the credentials to use.
> * @cap contains the capability <include/linux/capability.h>.
> + * @audit: Whether to write an audit message or not
> * Return 0 if the capability is granted for @tsk.
> * @acct:
> * Check permission before enabling or disabling process accounting. If
> @@ -1290,7 +1301,9 @@ struct security_operations {
> const kernel_cap_t *effective,
> const kernel_cap_t *inheritable,
> const kernel_cap_t *permitted);
> - int (*capable) (struct task_struct *tsk, int cap, int audit);
> + int (*capable) (int cap, int audit);
> + int (*task_capable) (struct task_struct *tsk, const struct cred *cred,
> + int cap, int audit);
> int (*acct) (struct file *file);
> int (*sysctl) (struct ctl_table *table, int op);
> int (*quotactl) (int cmds, int type, int id, struct super_block *sb);
> @@ -1556,8 +1569,9 @@ int security_capset(struct cred *new, const struct cred *old,
> const kernel_cap_t *effective,
> const kernel_cap_t *inheritable,
> const kernel_cap_t *permitted);
> -int security_capable(struct task_struct *tsk, int cap);
> -int security_capable_noaudit(struct task_struct *tsk, int cap);
> +int security_capable(int cap);
> +int security_task_capable(struct task_struct *tsk, int cap);
> +int security_task_capable_noaudit(struct task_struct *tsk, int cap);
> int security_acct(struct file *file);
> int security_sysctl(struct ctl_table *table, int op);
> int security_quotactl(int cmds, int type, int id, struct super_block *sb);
> @@ -1754,14 +1768,31 @@ static inline int security_capset(struct cred *new,
> return cap_capset(new, old, effective, inheritable, permitted);
> }
>
> -static inline int security_capable(struct task_struct *tsk, int cap)
> +static inline int security_capable(int cap)
> {
> - return cap_capable(tsk, cap, SECURITY_CAP_AUDIT);
> + return cap_capable(cap, SECURITY_CAP_AUDIT);
> }
>
> -static inline int security_capable_noaudit(struct task_struct *tsk, int cap)
> +static inline int security_task_capable(struct task_struct *tsk, int cap)
> {
> - return cap_capable(tsk, cap, SECURITY_CAP_NOAUDIT);
> + int ret;
> +
> + rcu_read_lock();
> + ret = cap_task_capable(tsk, __task_cred(tsk), cap, SECURITY_CAP_AUDIT);
> + rcu_read_unlock();
> + return ret;
> +}
> +
> +static inline
> +int security_task_capable_noaudit(struct task_struct *tsk, int cap)
> +{
> + int ret;
> +
> + rcu_read_lock();
> + ret = cap_task_capable(tsk, __task_cred(tsk), cap,
> + SECURITY_CAP_NOAUDIT);
> + rcu_read_unlock();
> + return ret;
> }
>
> static inline int security_acct(struct file *file)
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 36b4b4d..df62f53 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -308,7 +308,7 @@ int capable(int cap)
> BUG();
> }
>
> - if (has_capability(current, cap)) {
> + if (security_capable(cap) == 0) {
> current->flags |= PF_SUPERPRIV;
> return 1;
> }
> diff --git a/security/capability.c b/security/capability.c
> index 2dce66f..fd1493d 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -826,6 +826,7 @@ void security_fixup_ops(struct security_operations *ops)
> set_to_cap_if_null(ops, capset);
> set_to_cap_if_null(ops, acct);
> set_to_cap_if_null(ops, capable);
> + set_to_cap_if_null(ops, task_capable);
> set_to_cap_if_null(ops, quotactl);
> set_to_cap_if_null(ops, quota_on);
> set_to_cap_if_null(ops, sysctl);
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 7971354..7f0b2a6 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -43,28 +43,44 @@ int cap_netlink_recv(struct sk_buff *skb, int cap)
> EXPORT_SYMBOL(cap_netlink_recv);
>
> /**
> - * cap_capable - Determine whether a task has a particular effective capability
> - * @tsk: The task to query
> + * cap_capable - Determine whether current has a particular effective capability
> * @cap: The capability to check for
> * @audit: Whether to write an audit message or not
> *
> * Determine whether the nominated task has the specified capability amongst
> - * its effective set, returning 0 if it does, -ve if it does not.
> + * its effective set, returning 0 if it does, -ve if it does not. Note that
> + * this uses current's subjective/effective credentials.
> *
> * NOTE WELL: cap_capable() cannot be used like the kernel's capable()
> * function. That is, it has the reverse semantics: cap_capable() returns 0
> * when a task has a capability, but the kernel's capable() returns 1 for this
> * case.
> */
> -int cap_capable(struct task_struct *tsk, int cap, int audit)
> +int cap_capable(int cap, int audit)
> {
> - __u32 cap_raised;
> + return cap_raised(current_cap(), cap) ? 0 : -EPERM;
> +}
>
> - /* Derived from include/linux/sched.h:capable. */
> - rcu_read_lock();
> - cap_raised = cap_raised(__task_cred(tsk)->cap_effective, cap);
> - rcu_read_unlock();
> - return cap_raised ? 0 : -EPERM;
> +/**
> + * cap_has_capability - Determine whether a task has a particular effective capability
> + * @tsk: The task to query
> + * @cred: The credentials to use
> + * @cap: The capability to check for
> + * @audit: Whether to write an audit message or not
> + *
> + * Determine whether the nominated task has the specified capability amongst
> + * its effective set, returning 0 if it does, -ve if it does not. Note that
> + * this uses the task's objective/real credentials.
> + *
> + * NOTE WELL: cap_has_capability() cannot be used like the kernel's
> + * has_capability() function. That is, it has the reverse semantics:
> + * cap_has_capability() returns 0 when a task has a capability, but the
> + * kernel's has_capability() returns 1 for this case.
> + */
> +int cap_task_capable(struct task_struct *tsk, const struct cred *cred, int cap,
> + int audit)
> +{
> + return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM;
> }
>
> /**
> @@ -160,7 +176,7 @@ static inline int cap_inh_is_capped(void)
> /* they are so limited unless the current task has the CAP_SETPCAP
> * capability
> */
> - if (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) == 0)
> + if (cap_capable(CAP_SETPCAP, SECURITY_CAP_AUDIT) == 0)
> return 0;
> #endif
> return 1;
> @@ -869,7 +885,7 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> & (new->securebits ^ arg2)) /*[1]*/
> || ((new->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/
> || (arg2 & ~(SECURE_ALL_LOCKS | SECURE_ALL_BITS)) /*[3]*/
> - || (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) != 0) /*[4]*/
> + || (cap_capable(CAP_SETPCAP, SECURITY_CAP_AUDIT) != 0) /*[4]*/
> /*
> * [1] no changing of bits that are locked
> * [2] no unlocking of locks
> @@ -950,7 +966,7 @@ int cap_vm_enough_memory(struct mm_struct *mm, long pages)
> {
> int cap_sys_admin = 0;
>
> - if (cap_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT) == 0)
> + if (cap_capable(CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT) == 0)
> cap_sys_admin = 1;
> return __vm_enough_memory(mm, pages, cap_sys_admin);
> }
> diff --git a/security/root_plug.c b/security/root_plug.c
> index 40fb4f1..559578f 100644
> --- a/security/root_plug.c
> +++ b/security/root_plug.c
> @@ -77,6 +77,7 @@ static struct security_operations rootplug_security_ops = {
> .capget = cap_capget,
> .capset = cap_capset,
> .capable = cap_capable,
> + .task_capable = cap_task_capable,
>
> .bprm_set_creds = cap_bprm_set_creds,
>
> diff --git a/security/security.c b/security/security.c
> index d85dbb3..9bbc8e5 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -154,14 +154,31 @@ int security_capset(struct cred *new, const struct cred *old,
> effective, inheritable, permitted);
> }
>
> -int security_capable(struct task_struct *tsk, int cap)
> +int security_capable(int cap)
> {
> - return security_ops->capable(tsk, cap, SECURITY_CAP_AUDIT);
> + return security_ops->capable(cap, SECURITY_CAP_AUDIT);
> }
>
> -int security_capable_noaudit(struct task_struct *tsk, int cap)
> +int security_task_capable(struct task_struct *tsk, int cap)
> {
> - return security_ops->capable(tsk, cap, SECURITY_CAP_NOAUDIT);
> + const struct cred *cred;
> + int ret;
> +
> + cred = get_task_cred(tsk);
> + ret = security_ops->task_capable(tsk, cred, cap, SECURITY_CAP_AUDIT);
> + put_cred(cred);
> + return ret;
> +}
> +
> +int security_task_capable_noaudit(struct task_struct *tsk, int cap)
> +{
> + const struct cred *cred;
> + int ret;
> +
> + cred = get_task_cred(tsk);
> + ret = security_ops->task_capable(tsk, cred, cap, SECURITY_CAP_NOAUDIT);
> + put_cred(cred);
> + return ret;
> }
>
> int security_acct(struct file *file)
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index dbeaa78..5a66cd3 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1433,12 +1433,13 @@ static int current_has_perm(const struct task_struct *tsk,
>
> /* Check whether a task is allowed to use a capability. */
> static int task_has_capability(struct task_struct *tsk,
> + const struct cred *cred,
> int cap, int audit)
> {
> struct avc_audit_data ad;
> struct av_decision avd;
> u16 sclass;
> - u32 sid = task_sid(tsk);
> + u32 sid = cred_sid(cred);
> u32 av = CAP_TO_MASK(cap);
> int rc;
>
> @@ -1865,15 +1866,27 @@ static int selinux_capset(struct cred *new, const struct cred *old,
> return cred_has_perm(old, new, PROCESS__SETCAP);
> }
>
> -static int selinux_capable(struct task_struct *tsk, int cap, int audit)
> +static int selinux_capable(int cap, int audit)
> +{
> + int rc;
> +
> + rc = secondary_ops->capable(cap, audit);
> + if (rc)
> + return rc;
> +
> + return task_has_capability(current, current_cred(), cap, audit);
> +}
> +
> +static int selinux_task_capable(struct task_struct *tsk,
> + const struct cred *cred, int cap, int audit)
> {
> int rc;
>
> - rc = secondary_ops->capable(tsk, cap, audit);
> + rc = secondary_ops->task_capable(tsk, cred, cap, audit);
> if (rc)
> return rc;
>
> - return task_has_capability(tsk, cap, audit);
> + return task_has_capability(tsk, cred, cap, audit);
> }
>
> static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid)
> @@ -2037,7 +2050,7 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages)
> {
> int rc, cap_sys_admin = 0;
>
> - rc = selinux_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT);
> + rc = selinux_capable(CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT);
> if (rc == 0)
> cap_sys_admin = 1;
>
> @@ -2880,7 +2893,7 @@ static int selinux_inode_getsecurity(const struct inode *inode, const char *name
> * and lack of permission just means that we fall back to the
> * in-core context value, not a denial.
> */
> - error = selinux_capable(current, CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT);
> + error = selinux_capable(CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT);
> if (!error)
> error = security_sid_to_context_force(isec->sid, &context,
> &size);
> @@ -5568,6 +5581,7 @@ static struct security_operations selinux_ops = {
> .capset = selinux_capset,
> .sysctl = selinux_sysctl,
> .capable = selinux_capable,
> + .task_capable = selinux_task_capable,
> .quotactl = selinux_quotactl,
> .quota_on = selinux_quota_on,
> .syslog = selinux_syslog,
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 1b5551d..8e53d43 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -2628,6 +2628,7 @@ struct security_operations smack_ops = {
> .capget = cap_capget,
> .capset = cap_capset,
> .capable = cap_capable,
> + .task_capable = cap_task_capable,
> .syslog = smack_syslog,
> .settime = cap_settime,
> .vm_enough_memory = cap_vm_enough_memory,
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-02 16:48 ` [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2] J. Bruce Fields
@ 2009-01-02 19:18 ` David Howells
0 siblings, 0 replies; 41+ messages in thread
From: David Howells @ 2009-01-02 19:18 UTC (permalink / raw)
To: J. Bruce Fields
Cc: dhowells, Christoph Hellwig, jmorris, linux-kernel, linux-fsdevel
J. Bruce Fields <bfields@fieldses.org> wrote:
> > Fix a regression in cap_capable() due to:
> >
> > commit 5ff7711e635b32f0a1e558227d030c7e45b4a465
>
> By the way, this should be 3b11a1decef07c19443d24ae926982bc8ec9f4c0,
> which is the patch that was committed by James Morris and is in Linus's
> tree. I assume 5ff7711... is the commitid in your tree and that James
> applied it as a patch instead of pulling from you.
Erm... Yes. I'm not sure how my test tree has managed to come up with that
ID. The 3b11 one is indeed correct.
David
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2008-12-31 15:15 ` [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2] David Howells
` (2 preceding siblings ...)
2009-01-02 16:48 ` [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2] J. Bruce Fields
@ 2009-01-05 2:07 ` James Morris
2009-01-05 3:18 ` Serge E. Hallyn
2009-01-06 22:27 ` [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #3] David Howells
3 siblings, 2 replies; 41+ messages in thread
From: James Morris @ 2009-01-05 2:07 UTC (permalink / raw)
To: David Howells
Cc: Christoph Hellwig, linux-kernel, linux-fsdevel, Serge E. Hallyn,
linux-security-module, Stephen Rothwell
On Wed, 31 Dec 2008, David Howells wrote:
>
> Here's an improved patch. It differentiates the use of objective and
> subjective capabilities by making capable() only check current's subjective
> caps, but making has_capability() check only the objective caps of whatever
> process is specified.
>
> It's a bit more involved, but I think it's the right thing to do.
I think it's the right approach, too, and the patch seems ok to me. I've
applied it to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
and expect to push it to Linus in the next day or so. It's not a trivial
change, and could do with more review (Serge?).
It seems that more testing should be done in linux-next vs. waiting for
the merge window.
- James
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-05 2:07 ` James Morris
@ 2009-01-05 3:18 ` Serge E. Hallyn
2009-01-05 3:37 ` Serge E. Hallyn
2009-01-05 12:43 ` David Howells
2009-01-06 22:27 ` [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #3] David Howells
1 sibling, 2 replies; 41+ messages in thread
From: Serge E. Hallyn @ 2009-01-05 3:18 UTC (permalink / raw)
To: James Morris
Cc: David Howells, Christoph Hellwig, linux-kernel, linux-fsdevel,
linux-security-module, Stephen Rothwell, Andrew Morgan
Quoting James Morris (jmorris@namei.org):
> On Wed, 31 Dec 2008, David Howells wrote:
>
> >
> > Here's an improved patch. It differentiates the use of objective and
> > subjective capabilities by making capable() only check current's subjective
> > caps, but making has_capability() check only the objective caps of whatever
> > process is specified.
> >
> > It's a bit more involved, but I think it's the right thing to do.
>
> I think it's the right approach, too, and the patch seems ok to me. I've
> applied it to
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
>
> and expect to push it to Linus in the next day or so. It's not a trivial
> change, and could do with more review (Serge?).
(Andrew Morgan cc'd for more review)
Yes, I'm sorry, I've been staring at it on and off all weekend... From
a high level it looks right, but i think there's a problem with naming
here (which also could be said to have obfuscated the original bug).
The security_capable() should be security_capable_eff() or somesuch,
and security_task_capable() should be security_task_capable_real() or
something. In fact the current-as-implicit optimization could be put
off for a kernel release or so while we just have
security_capable_eff(tsk, cap) and security_real_eff(tsk, cap), or
just security_capable(tsk, cap, flag) where flag is CAP_EFF or CAP_REAL.
I've been holding off on saying this since sat night bc when i read it
back it looks petty, but every time i read the patch i'm more convinced
that the type of capability checked must be made explicit to make this
maintainable (maybe even reviewable).
I'm also not thrilled about security_task_capable() and
security_ops->task_capable() having different args and semantics, but
of course I see the reason for it and figure if there's a way to improve
on that we can do it later.
Anyway David the patch on its own doesn't look incorrect. So far
the only code which manipulates subjective caps is in fact
sys_faccessat through cap_set_effective() right? If so this at
least looks safe, looking through capable/has_capability callers.
Finally, may i just say that i love the fact that a syscall is
checking the real user's access rights and so sets eff creds to
have the caps of the real user :) Hmm, did you at one time call
the subjective creds 'working' or 'acting' creds? Might be a good
name. Subj/obj always makes me pause to think, and real/eff while
seemingly natural could be confusing in cases like this.
cap_set_acting() - i like it...
thanks,
-serge
> It seems that more testing should be done in linux-next vs. waiting for
> the merge window.
>
>
> - James
> --
> James Morris
> <jmorris@namei.org>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-05 3:18 ` Serge E. Hallyn
@ 2009-01-05 3:37 ` Serge E. Hallyn
2009-01-05 12:43 ` David Howells
1 sibling, 0 replies; 41+ messages in thread
From: Serge E. Hallyn @ 2009-01-05 3:37 UTC (permalink / raw)
To: James Morris
Cc: David Howells, Christoph Hellwig, linux-kernel, linux-fsdevel,
linux-security-module, Stephen Rothwell, Andrew Morgan
Quoting Serge E. Hallyn (serue@us.ibm.com):
> seemingly natural could be confusing in cases like this.
> cap_set_acting() - i like it...
(Yes, please ignore that. I'm going to sleep.)
-serge
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-05 3:18 ` Serge E. Hallyn
2009-01-05 3:37 ` Serge E. Hallyn
@ 2009-01-05 12:43 ` David Howells
2009-01-05 19:07 ` Serge E. Hallyn
1 sibling, 1 reply; 41+ messages in thread
From: David Howells @ 2009-01-05 12:43 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: dhowells, James Morris, Christoph Hellwig, linux-kernel,
linux-fsdevel, linux-security-module, Stephen Rothwell,
Andrew Morgan
Serge E. Hallyn <serue@us.ibm.com> wrote:
> Yes, I'm sorry, I've been staring at it on and off all weekend... From
> a high level it looks right, but i think there's a problem with naming
> here (which also could be said to have obfuscated the original bug).
> The security_capable() should be security_capable_eff() or somesuch,
> and security_task_capable() should be security_task_capable_real() or
> something.
Yes. We've got real and effective creds, each with effective caps. It lends
itself to confusion most readily:-).
> In fact the current-as-implicit optimization could be put off for a kernel
> release or so while we just have security_capable_eff(tsk, cap) and
> security_real_eff(tsk, cap), or just security_capable(tsk, cap, flag) where
> flag is CAP_EFF or CAP_REAL.
I'd prefer not to add an extra argument like this, especially as CAP_EFF
shouldn't be used if tsk != current. Furthermore, security_capable_eff() may
only be called with tsk == current.
OTOH, as I pointed out, the capable() op as I've modified it to be it is
superfluous given the task_capable() op since the cred pointer is sufficient
to distinguish which set of creds you want to observe.
Is the attached patch preferable, then? I would prefer to go for the
optimisation in the common case, especially as the common case is called
rather a lot, but maybe the naming is more important...
> I'm also not thrilled about security_task_capable() and
> security_ops->task_capable() having different args and semantics, but
> of course I see the reason for it and figure if there's a way to improve
> on that we can do it later.
Well, security_ops->task_capable() should not be called, except by
security_task_capable*() and other security_ops->task_capable(), so does that
matter?
The reason for the way I've done things is that the creds from the specified
process need pinning in some way. If I just pass the task pointer through,
then each function that needs to examine those creds must pin them for
itself. With SELinux, that is both SELinux itself and commoncaps.
One thing I'm wondering is can I ditch the audit flag argument to the
task_capable() sec op if I make it contingent on tsk != NULL instead? The
credentials are passed by cred pointer, so, currently, tsk is only needed for
auditing.
> Anyway David the patch on its own doesn't look incorrect. So far
> the only code which manipulates subjective caps is in fact
> sys_faccessat through cap_set_effective() right? If so this at
> least looks safe, looking through capable/has_capability callers.
fs/nfsd/auth.c also.
> Finally, may i just say that i love the fact that a syscall is
> checking the real user's access rights and so sets eff creds to
> have the caps of the real user :)
Yes, it's quite mad.
> Hmm, did you at one time call the subjective creds 'working' or 'acting'
> creds? Might be a good name. Subj/obj always makes me pause to think, and
> real/eff while seemingly natural could be confusing in cases like this.
> cap_set_acting() - i like it...
I was using acting and act_as.
David
---
diff --git a/include/linux/capability.h b/include/linux/capability.h
index e22f48c..02bdb76 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -529,8 +529,21 @@ extern const kernel_cap_t __cap_init_eff_set;
*
* Note that this does not set PF_SUPERPRIV on the task.
*/
-#define has_capability(t, cap) (security_capable((t), (cap)) == 0)
-#define has_capability_noaudit(t, cap) (security_capable_noaudit((t), (cap)) == 0)
+#define has_capability(t, cap) (security_real_capable((t), (cap)) == 0)
+
+/**
+ * has_capability_noaudit - Determine if a task has a superior capability available (unaudited)
+ * @t: The task in question
+ * @cap: The capability to be tested for
+ *
+ * Return true if the specified task has the given superior capability
+ * currently in effect, false if not, but don't write an audit message for the
+ * check.
+ *
+ * Note that this does not set PF_SUPERPRIV on the task.
+ */
+#define has_capability_noaudit(t, cap) \
+ (security_real_capable_noaudit((t), (cap)) == 0)
extern int capable(int cap);
diff --git a/include/linux/security.h b/include/linux/security.h
index b92b5e4..1f2ab63 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -48,7 +48,8 @@ struct audit_krule;
* These functions are in security/capability.c and are used
* as the default capabilities functions
*/
-extern int cap_capable(struct task_struct *tsk, int cap, int audit);
+extern int cap_capable(struct task_struct *tsk, const struct cred *cred,
+ int cap, int audit);
extern int cap_settime(struct timespec *ts, struct timezone *tz);
extern int cap_ptrace_may_access(struct task_struct *child, unsigned int mode);
extern int cap_ptrace_traceme(struct task_struct *parent);
@@ -1251,9 +1252,12 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
* @permitted contains the permitted capability set.
* Return 0 and update @new if permission is granted.
* @capable:
- * Check whether the @tsk process has the @cap capability.
+ * Check whether the @tsk process has the @cap capability in the indicated
+ * credentials.
* @tsk contains the task_struct for the process.
+ * @cred contains the credentials to use.
* @cap contains the capability <include/linux/capability.h>.
+ * @audit: Whether to write an audit message or not
* Return 0 if the capability is granted for @tsk.
* @acct:
* Check permission before enabling or disabling process accounting. If
@@ -1346,7 +1350,8 @@ struct security_operations {
const kernel_cap_t *effective,
const kernel_cap_t *inheritable,
const kernel_cap_t *permitted);
- int (*capable) (struct task_struct *tsk, int cap, int audit);
+ int (*capable) (struct task_struct *tsk, const struct cred *cred,
+ int cap, int audit);
int (*acct) (struct file *file);
int (*sysctl) (struct ctl_table *table, int op);
int (*quotactl) (int cmds, int type, int id, struct super_block *sb);
@@ -1628,8 +1633,9 @@ int security_capset(struct cred *new, const struct cred *old,
const kernel_cap_t *effective,
const kernel_cap_t *inheritable,
const kernel_cap_t *permitted);
-int security_capable(struct task_struct *tsk, int cap);
-int security_capable_noaudit(struct task_struct *tsk, int cap);
+int security_capable(int cap);
+int security_real_capable(struct task_struct *tsk, int cap);
+int security_real_capable_noaudit(struct task_struct *tsk, int cap);
int security_acct(struct file *file);
int security_sysctl(struct ctl_table *table, int op);
int security_quotactl(int cmds, int type, int id, struct super_block *sb);
@@ -1826,14 +1832,31 @@ static inline int security_capset(struct cred *new,
return cap_capset(new, old, effective, inheritable, permitted);
}
-static inline int security_capable(struct task_struct *tsk, int cap)
+static inline int security_capable(int cap)
{
- return cap_capable(tsk, cap, SECURITY_CAP_AUDIT);
+ return cap_capable(current, current_cred(), cap, SECURITY_CAP_AUDIT);
}
-static inline int security_capable_noaudit(struct task_struct *tsk, int cap)
+static inline int security_real_capable(struct task_struct *tsk, int cap)
{
- return cap_capable(tsk, cap, SECURITY_CAP_NOAUDIT);
+ int ret;
+
+ rcu_read_lock();
+ ret = cap_capable(tsk, __task_cred(tsk), cap, SECURITY_CAP_AUDIT);
+ rcu_read_unlock();
+ return ret;
+}
+
+static inline
+int security_real_capable_noaudit(struct task_struct *tsk, int cap)
+{
+ int ret;
+
+ rcu_read_lock();
+ ret = cap_capable(tsk, __task_cred(tsk), cap,
+ SECURITY_CAP_NOAUDIT);
+ rcu_read_unlock();
+ return ret;
}
static inline int security_acct(struct file *file)
diff --git a/kernel/capability.c b/kernel/capability.c
index c598d9d..688926e 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -306,7 +306,7 @@ int capable(int cap)
BUG();
}
- if (has_capability(current, cap)) {
+ if (security_capable(cap) == 0) {
current->flags |= PF_SUPERPRIV;
return 1;
}
diff --git a/security/commoncap.c b/security/commoncap.c
index 7971354..f0e671d 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -45,26 +45,22 @@ EXPORT_SYMBOL(cap_netlink_recv);
/**
* cap_capable - Determine whether a task has a particular effective capability
* @tsk: The task to query
+ * @cred: The credentials to use
* @cap: The capability to check for
* @audit: Whether to write an audit message or not
*
* Determine whether the nominated task has the specified capability amongst
* its effective set, returning 0 if it does, -ve if it does not.
*
- * NOTE WELL: cap_capable() cannot be used like the kernel's capable()
- * function. That is, it has the reverse semantics: cap_capable() returns 0
- * when a task has a capability, but the kernel's capable() returns 1 for this
- * case.
+ * NOTE WELL: cap_has_capability() cannot be used like the kernel's capable()
+ * and has_capability() functions. That is, it has the reverse semantics:
+ * cap_has_capability() returns 0 when a task has a capability, but the
+ * kernel's capable() and has_capability() returns 1 for this case.
*/
-int cap_capable(struct task_struct *tsk, int cap, int audit)
+int cap_capable(struct task_struct *tsk, const struct cred *cred, int cap,
+ int audit)
{
- __u32 cap_raised;
-
- /* Derived from include/linux/sched.h:capable. */
- rcu_read_lock();
- cap_raised = cap_raised(__task_cred(tsk)->cap_effective, cap);
- rcu_read_unlock();
- return cap_raised ? 0 : -EPERM;
+ return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM;
}
/**
@@ -160,7 +156,8 @@ static inline int cap_inh_is_capped(void)
/* they are so limited unless the current task has the CAP_SETPCAP
* capability
*/
- if (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) == 0)
+ if (cap_capable(current, current_cred(), CAP_SETPCAP,
+ SECURITY_CAP_AUDIT) == 0)
return 0;
#endif
return 1;
@@ -869,7 +866,8 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
& (new->securebits ^ arg2)) /*[1]*/
|| ((new->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/
|| (arg2 & ~(SECURE_ALL_LOCKS | SECURE_ALL_BITS)) /*[3]*/
- || (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) != 0) /*[4]*/
+ || (cap_capable(current, current_cred(), CAP_SETPCAP,
+ SECURITY_CAP_AUDIT) != 0) /*[4]*/
/*
* [1] no changing of bits that are locked
* [2] no unlocking of locks
@@ -950,7 +948,8 @@ int cap_vm_enough_memory(struct mm_struct *mm, long pages)
{
int cap_sys_admin = 0;
- if (cap_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT) == 0)
+ if (cap_capable(current, current_cred(), CAP_SYS_ADMIN,
+ SECURITY_CAP_NOAUDIT) == 0)
cap_sys_admin = 1;
return __vm_enough_memory(mm, pages, cap_sys_admin);
}
diff --git a/security/security.c b/security/security.c
index 678d4d0..c3586c0 100644
--- a/security/security.c
+++ b/security/security.c
@@ -154,14 +154,32 @@ int security_capset(struct cred *new, const struct cred *old,
effective, inheritable, permitted);
}
-int security_capable(struct task_struct *tsk, int cap)
+int security_capable(int cap)
{
- return security_ops->capable(tsk, cap, SECURITY_CAP_AUDIT);
+ return security_ops->capable(current, current_cred(), cap,
+ SECURITY_CAP_AUDIT);
}
-int security_capable_noaudit(struct task_struct *tsk, int cap)
+int security_real_capable(struct task_struct *tsk, int cap)
{
- return security_ops->capable(tsk, cap, SECURITY_CAP_NOAUDIT);
+ const struct cred *cred;
+ int ret;
+
+ cred = get_task_cred(tsk);
+ ret = security_ops->capable(tsk, cred, cap, SECURITY_CAP_AUDIT);
+ put_cred(cred);
+ return ret;
+}
+
+int security_real_capable_noaudit(struct task_struct *tsk, int cap)
+{
+ const struct cred *cred;
+ int ret;
+
+ cred = get_task_cred(tsk);
+ ret = security_ops->capable(tsk, cred, cap, SECURITY_CAP_NOAUDIT);
+ put_cred(cred);
+ return ret;
}
int security_acct(struct file *file)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index dbeaa78..e0cb106 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1433,12 +1433,13 @@ static int current_has_perm(const struct task_struct *tsk,
/* Check whether a task is allowed to use a capability. */
static int task_has_capability(struct task_struct *tsk,
+ const struct cred *cred,
int cap, int audit)
{
struct avc_audit_data ad;
struct av_decision avd;
u16 sclass;
- u32 sid = task_sid(tsk);
+ u32 sid = cred_sid(cred);
u32 av = CAP_TO_MASK(cap);
int rc;
@@ -1865,15 +1866,16 @@ static int selinux_capset(struct cred *new, const struct cred *old,
return cred_has_perm(old, new, PROCESS__SETCAP);
}
-static int selinux_capable(struct task_struct *tsk, int cap, int audit)
+static int selinux_capable(struct task_struct *tsk, const struct cred *cred,
+ int cap, int audit)
{
int rc;
- rc = secondary_ops->capable(tsk, cap, audit);
+ rc = secondary_ops->capable(tsk, cred, cap, audit);
if (rc)
return rc;
- return task_has_capability(tsk, cap, audit);
+ return task_has_capability(tsk, cred, cap, audit);
}
static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid)
@@ -2037,7 +2039,8 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages)
{
int rc, cap_sys_admin = 0;
- rc = selinux_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT);
+ rc = selinux_capable(current, current_cred(), CAP_SYS_ADMIN,
+ SECURITY_CAP_NOAUDIT);
if (rc == 0)
cap_sys_admin = 1;
@@ -2880,7 +2883,8 @@ static int selinux_inode_getsecurity(const struct inode *inode, const char *name
* and lack of permission just means that we fall back to the
* in-core context value, not a denial.
*/
- error = selinux_capable(current, CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT);
+ error = selinux_capable(current, current_cred(), CAP_MAC_ADMIN,
+ SECURITY_CAP_NOAUDIT);
if (!error)
error = security_sid_to_context_force(isec->sid, &context,
&size);
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-05 12:43 ` David Howells
@ 2009-01-05 19:07 ` Serge E. Hallyn
2009-01-05 21:12 ` David Howells
0 siblings, 1 reply; 41+ messages in thread
From: Serge E. Hallyn @ 2009-01-05 19:07 UTC (permalink / raw)
To: David Howells
Cc: James Morris, Christoph Hellwig, linux-kernel, linux-fsdevel,
linux-security-module, Stephen Rothwell, Andrew Morgan
Quoting David Howells (dhowells@redhat.com):
> Serge E. Hallyn <serue@us.ibm.com> wrote:
>
> > Yes, I'm sorry, I've been staring at it on and off all weekend... From
> > a high level it looks right, but i think there's a problem with naming
> > here (which also could be said to have obfuscated the original bug).
> > The security_capable() should be security_capable_eff() or somesuch,
> > and security_task_capable() should be security_task_capable_real() or
> > something.
>
> Yes. We've got real and effective creds, each with effective caps. It lends
> itself to confusion most readily:-).
>
> > In fact the current-as-implicit optimization could be put off for a kernel
> > release or so while we just have security_capable_eff(tsk, cap) and
> > security_real_eff(tsk, cap), or just security_capable(tsk, cap, flag) where
> > flag is CAP_EFF or CAP_REAL.
>
> I'd prefer not to add an extra argument like this, especially as CAP_EFF
> shouldn't be used if tsk != current. Furthermore, security_capable_eff() may
> only be called with tsk == current.
Ok.
> OTOH, as I pointed out, the capable() op as I've modified it to be it is
> superfluous given the task_capable() op since the cred pointer is sufficient
> to distinguish which set of creds you want to observe.
>
> Is the attached patch preferable, then? I would prefer to go for the
> optimisation in the common case, especially as the common case is called
> rather a lot, but maybe the naming is more important...
You have the 'acting_as' name for subj/eff, which I like. Is there
another name you could use in place of 'real' in the name
task_real_capable()?
I do find this version much easier to read. It seems easier to
track capable+current_cred() vs real_capable+get_task_cred(). Could
you do a few benchmarks to gauge whether the difference the
optimization makes?
> > I'm also not thrilled about security_task_capable() and
> > security_ops->task_capable() having different args and semantics, but
> > of course I see the reason for it and figure if there's a way to improve
> > on that we can do it later.
>
> Well, security_ops->task_capable() should not be called, except by
> security_task_capable*() and other security_ops->task_capable(), so does that
> matter?
It's just something that could cause confusion 6 months down the road
when someone who wasn't involved in this thread is trying to do some
routine LSM maintenance... But like I say I understand the reasons
and agree, so let's ignore it.
> The reason for the way I've done things is that the creds from the specified
> process need pinning in some way. If I just pass the task pointer through,
> then each function that needs to examine those creds must pin them for
> itself. With SELinux, that is both SELinux itself and commoncaps.
>
> One thing I'm wondering is can I ditch the audit flag argument to the
> task_capable() sec op if I make it contingent on tsk != NULL instead? The
> credentials are passed by cred pointer, so, currently, tsk is only needed for
> auditing.
I'm looking at a several-week-old linux-next, but only see one use of
capable on another task which audits, and that is in commoncap for
traceme, so it seems reasonable.
> > Anyway David the patch on its own doesn't look incorrect. So far
> > the only code which manipulates subjective caps is in fact
> > sys_faccessat through cap_set_effective() right? If so this at
> > least looks safe, looking through capable/has_capability callers.
>
> fs/nfsd/auth.c also.
So yeah, I do like this version better.
Acked-by: Serge Hallyn <serue@us.ibm.com>
> > Finally, may i just say that i love the fact that a syscall is
> > checking the real user's access rights and so sets eff creds to
> > have the caps of the real user :)
>
> Yes, it's quite mad.
>
> > Hmm, did you at one time call the subjective creds 'working' or 'acting'
> > creds? Might be a good name. Subj/obj always makes me pause to think, and
> > real/eff while seemingly natural could be confusing in cases like this.
> > cap_set_acting() - i like it...
>
> I was using acting and act_as.
>
> David
>
> ---
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index e22f48c..02bdb76 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -529,8 +529,21 @@ extern const kernel_cap_t __cap_init_eff_set;
> *
> * Note that this does not set PF_SUPERPRIV on the task.
> */
> -#define has_capability(t, cap) (security_capable((t), (cap)) == 0)
> -#define has_capability_noaudit(t, cap) (security_capable_noaudit((t), (cap)) == 0)
> +#define has_capability(t, cap) (security_real_capable((t), (cap)) == 0)
> +
> +/**
> + * has_capability_noaudit - Determine if a task has a superior capability available (unaudited)
> + * @t: The task in question
> + * @cap: The capability to be tested for
> + *
> + * Return true if the specified task has the given superior capability
> + * currently in effect, false if not, but don't write an audit message for the
> + * check.
> + *
> + * Note that this does not set PF_SUPERPRIV on the task.
> + */
> +#define has_capability_noaudit(t, cap) \
> + (security_real_capable_noaudit((t), (cap)) == 0)
>
> extern int capable(int cap);
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index b92b5e4..1f2ab63 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -48,7 +48,8 @@ struct audit_krule;
> * These functions are in security/capability.c and are used
> * as the default capabilities functions
> */
> -extern int cap_capable(struct task_struct *tsk, int cap, int audit);
> +extern int cap_capable(struct task_struct *tsk, const struct cred *cred,
> + int cap, int audit);
> extern int cap_settime(struct timespec *ts, struct timezone *tz);
> extern int cap_ptrace_may_access(struct task_struct *child, unsigned int mode);
> extern int cap_ptrace_traceme(struct task_struct *parent);
> @@ -1251,9 +1252,12 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
> * @permitted contains the permitted capability set.
> * Return 0 and update @new if permission is granted.
> * @capable:
> - * Check whether the @tsk process has the @cap capability.
> + * Check whether the @tsk process has the @cap capability in the indicated
> + * credentials.
> * @tsk contains the task_struct for the process.
> + * @cred contains the credentials to use.
> * @cap contains the capability <include/linux/capability.h>.
> + * @audit: Whether to write an audit message or not
> * Return 0 if the capability is granted for @tsk.
> * @acct:
> * Check permission before enabling or disabling process accounting. If
> @@ -1346,7 +1350,8 @@ struct security_operations {
> const kernel_cap_t *effective,
> const kernel_cap_t *inheritable,
> const kernel_cap_t *permitted);
> - int (*capable) (struct task_struct *tsk, int cap, int audit);
> + int (*capable) (struct task_struct *tsk, const struct cred *cred,
> + int cap, int audit);
> int (*acct) (struct file *file);
> int (*sysctl) (struct ctl_table *table, int op);
> int (*quotactl) (int cmds, int type, int id, struct super_block *sb);
> @@ -1628,8 +1633,9 @@ int security_capset(struct cred *new, const struct cred *old,
> const kernel_cap_t *effective,
> const kernel_cap_t *inheritable,
> const kernel_cap_t *permitted);
> -int security_capable(struct task_struct *tsk, int cap);
> -int security_capable_noaudit(struct task_struct *tsk, int cap);
> +int security_capable(int cap);
> +int security_real_capable(struct task_struct *tsk, int cap);
> +int security_real_capable_noaudit(struct task_struct *tsk, int cap);
> int security_acct(struct file *file);
> int security_sysctl(struct ctl_table *table, int op);
> int security_quotactl(int cmds, int type, int id, struct super_block *sb);
> @@ -1826,14 +1832,31 @@ static inline int security_capset(struct cred *new,
> return cap_capset(new, old, effective, inheritable, permitted);
> }
>
> -static inline int security_capable(struct task_struct *tsk, int cap)
> +static inline int security_capable(int cap)
> {
> - return cap_capable(tsk, cap, SECURITY_CAP_AUDIT);
> + return cap_capable(current, current_cred(), cap, SECURITY_CAP_AUDIT);
> }
>
> -static inline int security_capable_noaudit(struct task_struct *tsk, int cap)
> +static inline int security_real_capable(struct task_struct *tsk, int cap)
> {
> - return cap_capable(tsk, cap, SECURITY_CAP_NOAUDIT);
> + int ret;
> +
> + rcu_read_lock();
> + ret = cap_capable(tsk, __task_cred(tsk), cap, SECURITY_CAP_AUDIT);
> + rcu_read_unlock();
> + return ret;
> +}
> +
> +static inline
> +int security_real_capable_noaudit(struct task_struct *tsk, int cap)
> +{
> + int ret;
> +
> + rcu_read_lock();
> + ret = cap_capable(tsk, __task_cred(tsk), cap,
> + SECURITY_CAP_NOAUDIT);
> + rcu_read_unlock();
> + return ret;
> }
>
> static inline int security_acct(struct file *file)
> diff --git a/kernel/capability.c b/kernel/capability.c
> index c598d9d..688926e 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -306,7 +306,7 @@ int capable(int cap)
> BUG();
> }
>
> - if (has_capability(current, cap)) {
> + if (security_capable(cap) == 0) {
> current->flags |= PF_SUPERPRIV;
> return 1;
> }
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 7971354..f0e671d 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -45,26 +45,22 @@ EXPORT_SYMBOL(cap_netlink_recv);
> /**
> * cap_capable - Determine whether a task has a particular effective capability
> * @tsk: The task to query
> + * @cred: The credentials to use
> * @cap: The capability to check for
> * @audit: Whether to write an audit message or not
> *
> * Determine whether the nominated task has the specified capability amongst
> * its effective set, returning 0 if it does, -ve if it does not.
> *
> - * NOTE WELL: cap_capable() cannot be used like the kernel's capable()
> - * function. That is, it has the reverse semantics: cap_capable() returns 0
> - * when a task has a capability, but the kernel's capable() returns 1 for this
> - * case.
> + * NOTE WELL: cap_has_capability() cannot be used like the kernel's capable()
> + * and has_capability() functions. That is, it has the reverse semantics:
> + * cap_has_capability() returns 0 when a task has a capability, but the
> + * kernel's capable() and has_capability() returns 1 for this case.
> */
> -int cap_capable(struct task_struct *tsk, int cap, int audit)
> +int cap_capable(struct task_struct *tsk, const struct cred *cred, int cap,
> + int audit)
> {
> - __u32 cap_raised;
> -
> - /* Derived from include/linux/sched.h:capable. */
> - rcu_read_lock();
> - cap_raised = cap_raised(__task_cred(tsk)->cap_effective, cap);
> - rcu_read_unlock();
> - return cap_raised ? 0 : -EPERM;
> + return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM;
> }
>
> /**
> @@ -160,7 +156,8 @@ static inline int cap_inh_is_capped(void)
> /* they are so limited unless the current task has the CAP_SETPCAP
> * capability
> */
> - if (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) == 0)
> + if (cap_capable(current, current_cred(), CAP_SETPCAP,
> + SECURITY_CAP_AUDIT) == 0)
> return 0;
> #endif
> return 1;
> @@ -869,7 +866,8 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> & (new->securebits ^ arg2)) /*[1]*/
> || ((new->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/
> || (arg2 & ~(SECURE_ALL_LOCKS | SECURE_ALL_BITS)) /*[3]*/
> - || (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) != 0) /*[4]*/
> + || (cap_capable(current, current_cred(), CAP_SETPCAP,
> + SECURITY_CAP_AUDIT) != 0) /*[4]*/
> /*
> * [1] no changing of bits that are locked
> * [2] no unlocking of locks
> @@ -950,7 +948,8 @@ int cap_vm_enough_memory(struct mm_struct *mm, long pages)
> {
> int cap_sys_admin = 0;
>
> - if (cap_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT) == 0)
> + if (cap_capable(current, current_cred(), CAP_SYS_ADMIN,
> + SECURITY_CAP_NOAUDIT) == 0)
> cap_sys_admin = 1;
> return __vm_enough_memory(mm, pages, cap_sys_admin);
> }
> diff --git a/security/security.c b/security/security.c
> index 678d4d0..c3586c0 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -154,14 +154,32 @@ int security_capset(struct cred *new, const struct cred *old,
> effective, inheritable, permitted);
> }
>
> -int security_capable(struct task_struct *tsk, int cap)
> +int security_capable(int cap)
> {
> - return security_ops->capable(tsk, cap, SECURITY_CAP_AUDIT);
> + return security_ops->capable(current, current_cred(), cap,
> + SECURITY_CAP_AUDIT);
> }
>
> -int security_capable_noaudit(struct task_struct *tsk, int cap)
> +int security_real_capable(struct task_struct *tsk, int cap)
> {
> - return security_ops->capable(tsk, cap, SECURITY_CAP_NOAUDIT);
> + const struct cred *cred;
> + int ret;
> +
> + cred = get_task_cred(tsk);
> + ret = security_ops->capable(tsk, cred, cap, SECURITY_CAP_AUDIT);
> + put_cred(cred);
> + return ret;
> +}
> +
> +int security_real_capable_noaudit(struct task_struct *tsk, int cap)
> +{
> + const struct cred *cred;
> + int ret;
> +
> + cred = get_task_cred(tsk);
> + ret = security_ops->capable(tsk, cred, cap, SECURITY_CAP_NOAUDIT);
> + put_cred(cred);
> + return ret;
> }
>
> int security_acct(struct file *file)
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index dbeaa78..e0cb106 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1433,12 +1433,13 @@ static int current_has_perm(const struct task_struct *tsk,
>
> /* Check whether a task is allowed to use a capability. */
> static int task_has_capability(struct task_struct *tsk,
> + const struct cred *cred,
> int cap, int audit)
> {
> struct avc_audit_data ad;
> struct av_decision avd;
> u16 sclass;
> - u32 sid = task_sid(tsk);
> + u32 sid = cred_sid(cred);
> u32 av = CAP_TO_MASK(cap);
> int rc;
>
> @@ -1865,15 +1866,16 @@ static int selinux_capset(struct cred *new, const struct cred *old,
> return cred_has_perm(old, new, PROCESS__SETCAP);
> }
>
> -static int selinux_capable(struct task_struct *tsk, int cap, int audit)
> +static int selinux_capable(struct task_struct *tsk, const struct cred *cred,
> + int cap, int audit)
> {
> int rc;
>
> - rc = secondary_ops->capable(tsk, cap, audit);
> + rc = secondary_ops->capable(tsk, cred, cap, audit);
> if (rc)
> return rc;
>
> - return task_has_capability(tsk, cap, audit);
> + return task_has_capability(tsk, cred, cap, audit);
> }
>
> static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid)
> @@ -2037,7 +2039,8 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages)
> {
> int rc, cap_sys_admin = 0;
>
> - rc = selinux_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT);
> + rc = selinux_capable(current, current_cred(), CAP_SYS_ADMIN,
> + SECURITY_CAP_NOAUDIT);
> if (rc == 0)
> cap_sys_admin = 1;
>
> @@ -2880,7 +2883,8 @@ static int selinux_inode_getsecurity(const struct inode *inode, const char *name
> * and lack of permission just means that we fall back to the
> * in-core context value, not a denial.
> */
> - error = selinux_capable(current, CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT);
> + error = selinux_capable(current, current_cred(), CAP_MAC_ADMIN,
> + SECURITY_CAP_NOAUDIT);
> if (!error)
> error = security_sid_to_context_force(isec->sid, &context,
> &size);
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-05 19:07 ` Serge E. Hallyn
@ 2009-01-05 21:12 ` David Howells
2009-01-06 16:47 ` Serge E. Hallyn
0 siblings, 1 reply; 41+ messages in thread
From: David Howells @ 2009-01-05 21:12 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: dhowells, James Morris, Christoph Hellwig, linux-kernel,
linux-fsdevel, linux-security-module, Stephen Rothwell,
Andrew Morgan
Serge E. Hallyn <serue@us.ibm.com> wrote:
> You have the 'acting_as' name for subj/eff, which I like. Is there
> another name you could use in place of 'real' in the name
> task_real_capable()?
Ummm... 'Actual' or 'Assigned' perhaps?
> I do find this version much easier to read. It seems easier to
> track capable+current_cred() vs real_capable+get_task_cred(). Could
> you do a few benchmarks to gauge whether the difference the
> optimization makes?
Yeah... My main objection is passing around two or three superfluous arguments
in the common case. Most of the time, the only necessary argument to
sec->capable():
int (*capable) (struct task_struct *tsk, const struct cred *cred,
int cap, int audit);
is cap; tsk, cred and audit are all superfluous in the (very) common case.
How about:
int (*fast_capable) (int cap);
which assumes current, current_cred() and SECURITY_CAP_AUDIT?
Benchmarking is tricky, given that the individual savings will be relatively
small in comparison to the code that calls them.
However, if I can get rid of three arguments passed into each of
security_capable(), selinux_capable() and cap_capable(), that really should
speed things up if you call it enough times, especially as current is held in a
register on some archs.
I'll see what I can do.
> I'm looking at a several-week-old linux-next, but only see one use of
> capable on another task which audits, and that is in commoncap for
> traceme, so it seems reasonable.
Should has_capability() be out of lines and have security_real_capable() merged
into it? And the same for has_capability_noaudit() and
security_real_capable_noaudit()?
> So yeah, I do like this version better.
Perhaps a separate patch to optimise capable(). As I said, I'll see about
benchmarking it.
David
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-05 21:12 ` David Howells
@ 2009-01-06 16:47 ` Serge E. Hallyn
2009-01-06 20:39 ` David Howells
0 siblings, 1 reply; 41+ messages in thread
From: Serge E. Hallyn @ 2009-01-06 16:47 UTC (permalink / raw)
To: David Howells
Cc: James Morris, Christoph Hellwig, linux-kernel, linux-fsdevel,
linux-security-module, Stephen Rothwell, Andrew Morgan
Quoting David Howells (dhowells@redhat.com):
> Serge E. Hallyn <serue@us.ibm.com> wrote:
>
> > You have the 'acting_as' name for subj/eff, which I like. Is there
> > another name you could use in place of 'real' in the name
> > task_real_capable()?
>
> Ummm... 'Actual' or 'Assigned' perhaps?
>
> > I do find this version much easier to read. It seems easier to
> > track capable+current_cred() vs real_capable+get_task_cred(). Could
> > you do a few benchmarks to gauge whether the difference the
> > optimization makes?
>
> Yeah... My main objection is passing around two or three superfluous arguments
> in the common case. Most of the time, the only necessary argument to
> sec->capable():
>
> int (*capable) (struct task_struct *tsk, const struct cred *cred,
> int cap, int audit);
>
> is cap; tsk, cred and audit are all superfluous in the (very) common case.
>
> How about:
>
> int (*fast_capable) (int cap);
>
> which assumes current, current_cred() and SECURITY_CAP_AUDIT?
Well I'd rather it be called acting_capable() or self_acting_capable(),
but the realy issue is how to make that work through the security_ops()
layer without needless code duplication. It'd be ideal if it's doable,
I agree.
> Benchmarking is tricky, given that the individual savings will be relatively
> small in comparison to the code that calls them.
>
> However, if I can get rid of three arguments passed into each of
> security_capable(), selinux_capable() and cap_capable(), that really should
> speed things up if you call it enough times, especially as current is held in a
> register on some archs.
>
> I'll see what I can do.
>
> > I'm looking at a several-week-old linux-next, but only see one use of
> > capable on another task which audits, and that is in commoncap for
> > traceme, so it seems reasonable.
>
> Should has_capability() be out of lines and have security_real_capable() merged
> into it? And the same for has_capability_noaudit() and
> security_real_capable_noaudit()?
>
> > So yeah, I do like this version better.
>
> Perhaps a separate patch to optimise capable(). As I said, I'll see about
> benchmarking it.
Cool, thanks. In the meantime, I guess your first patch is in
security-next anyway, right?
-serge
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-06 16:47 ` Serge E. Hallyn
@ 2009-01-06 20:39 ` David Howells
2009-01-06 20:56 ` Serge E. Hallyn
0 siblings, 1 reply; 41+ messages in thread
From: David Howells @ 2009-01-06 20:39 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: dhowells, James Morris, Christoph Hellwig, linux-kernel,
linux-fsdevel, linux-security-module, Stephen Rothwell,
Andrew Morgan
Serge E. Hallyn <serue@us.ibm.com> wrote:
> Well I'd rather it be called acting_capable() or self_acting_capable(),
> but the realy issue is how to make that work through the security_ops()
> layer without needless code duplication.
Given that the code duplication permits heavy optimisation of the most often
called path (by a fair stretch), I'd say that the duplication isn't precisely
'needless'. You'd be trading code space in favour of execution time, and I
think the amount of code is small enough that it'd be reasonable.
I'd prefer to call the security ops 'capable()' and 'has_capability()', I
think, but I can't do that as long as has_capability() is a macro. Changing
it to an inline function isn't straightforward, though.
> Cool, thanks. In the meantime, I guess your first patch is in security-next
> anyway, right?
Yes.
Anyway, how about the attached? I've updated the description a bit.
David
---
From: David Howells <dhowells@redhat.com>
Subject: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat()
Fix a regression in cap_capable() due to:
commit 3b11a1decef07c19443d24ae926982bc8ec9f4c0
Author: David Howells <dhowells@redhat.com>
Date: Fri Nov 14 10:39:26 2008 +1100
CRED: Differentiate objective and effective subjective credentials on a task
The problem is that the above patch allows a process to have two sets of
credentials, and for the most part uses the subjective credentials when
accessing current's creds.
There is, however, one exception: cap_capable(), and thus capable(), uses the
real/objective credentials of the target task, whether or not it is the current
task.
Ordinarily this doesn't matter, since usually the two cred pointers in current
point to the same set of creds. However, sys_faccessat() makes use of this
facility to override the credentials of the calling process to make its test,
without affecting the creds as seen from other processes.
One of the things sys_faccessat() does is to make an adjustment to the
effective capabilities mask, which cap_capable(), as it stands, then ignores.
The affected capability check is in generic_permission():
if (!(mask & MAY_EXEC) || execute_ok(inode))
if (capable(CAP_DAC_OVERRIDE))
return 0;
This change passes the set of credentials to be tested down into the commoncap
and SELinux code. The security functions called by capable() and
has_capability() select the appropriate set of credentials from the process
being checked.
This can be tested by compiling the following program from the XFS testsuite:
/*
* t_access_root.c - trivial test program to show permission bug.
*
* Written by Michael Kerrisk - copyright ownership not pursued.
* Sourced from: http://linux.derkeiler.com/Mailing-Lists/Kernel/2003-10/6030.html
*/
#include <limits.h>
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <sys/stat.h>
#define UID 500
#define GID 100
#define PERM 0
#define TESTPATH "/tmp/t_access"
static void
errExit(char *msg)
{
perror(msg);
exit(EXIT_FAILURE);
} /* errExit */
static void
accessTest(char *file, int mask, char *mstr)
{
printf("access(%s, %s) returns %d\n", file, mstr, access(file, mask));
} /* accessTest */
int
main(int argc, char *argv[])
{
int fd, perm, uid, gid;
char *testpath;
char cmd[PATH_MAX + 20];
testpath = (argc > 1) ? argv[1] : TESTPATH;
perm = (argc > 2) ? strtoul(argv[2], NULL, 8) : PERM;
uid = (argc > 3) ? atoi(argv[3]) : UID;
gid = (argc > 4) ? atoi(argv[4]) : GID;
unlink(testpath);
fd = open(testpath, O_RDWR | O_CREAT, 0);
if (fd == -1) errExit("open");
if (fchown(fd, uid, gid) == -1) errExit("fchown");
if (fchmod(fd, perm) == -1) errExit("fchmod");
close(fd);
snprintf(cmd, sizeof(cmd), "ls -l %s", testpath);
system(cmd);
if (seteuid(uid) == -1) errExit("seteuid");
accessTest(testpath, 0, "0");
accessTest(testpath, R_OK, "R_OK");
accessTest(testpath, W_OK, "W_OK");
accessTest(testpath, X_OK, "X_OK");
accessTest(testpath, R_OK | W_OK, "R_OK | W_OK");
accessTest(testpath, R_OK | X_OK, "R_OK | X_OK");
accessTest(testpath, W_OK | X_OK, "W_OK | X_OK");
accessTest(testpath, R_OK | W_OK | X_OK, "R_OK | W_OK | X_OK");
exit(EXIT_SUCCESS);
} /* main */
This can be run against an Ext3 filesystem as well as against an XFS
filesystem. If successful, it will show:
[root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043
---------- 1 dhowells dhowells 0 2008-12-31 03:00 /tmp/xxx
access(/tmp/xxx, 0) returns 0
access(/tmp/xxx, R_OK) returns 0
access(/tmp/xxx, W_OK) returns 0
access(/tmp/xxx, X_OK) returns -1
access(/tmp/xxx, R_OK | W_OK) returns 0
access(/tmp/xxx, R_OK | X_OK) returns -1
access(/tmp/xxx, W_OK | X_OK) returns -1
access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1
If unsuccessful, it will show:
[root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043
---------- 1 dhowells dhowells 0 2008-12-31 02:56 /tmp/xxx
access(/tmp/xxx, 0) returns 0
access(/tmp/xxx, R_OK) returns -1
access(/tmp/xxx, W_OK) returns -1
access(/tmp/xxx, X_OK) returns -1
access(/tmp/xxx, R_OK | W_OK) returns -1
access(/tmp/xxx, R_OK | X_OK) returns -1
access(/tmp/xxx, W_OK | X_OK) returns -1
access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1
I've also tested the fix with the SELinux and syscalls LTP testsuites.
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: J. Bruce Fields <bfields@citi.umich.edu>
Acked-by: Serge Hallyn <serue@us.ibm.com>
---
include/linux/capability.h | 17 +++++++++++++++--
include/linux/security.h | 41 ++++++++++++++++++++++++++++++++---------
kernel/capability.c | 2 +-
security/commoncap.c | 29 ++++++++++++++---------------
security/security.c | 26 ++++++++++++++++++++++----
security/selinux/hooks.c | 16 ++++++++++------
6 files changed, 94 insertions(+), 37 deletions(-)
diff --git a/include/linux/capability.h b/include/linux/capability.h
index e22f48c..02bdb76 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -529,8 +529,21 @@ extern const kernel_cap_t __cap_init_eff_set;
*
* Note that this does not set PF_SUPERPRIV on the task.
*/
-#define has_capability(t, cap) (security_capable((t), (cap)) == 0)
-#define has_capability_noaudit(t, cap) (security_capable_noaudit((t), (cap)) == 0)
+#define has_capability(t, cap) (security_real_capable((t), (cap)) == 0)
+
+/**
+ * has_capability_noaudit - Determine if a task has a superior capability available (unaudited)
+ * @t: The task in question
+ * @cap: The capability to be tested for
+ *
+ * Return true if the specified task has the given superior capability
+ * currently in effect, false if not, but don't write an audit message for the
+ * check.
+ *
+ * Note that this does not set PF_SUPERPRIV on the task.
+ */
+#define has_capability_noaudit(t, cap) \
+ (security_real_capable_noaudit((t), (cap)) == 0)
extern int capable(int cap);
diff --git a/include/linux/security.h b/include/linux/security.h
index b92b5e4..1f2ab63 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -48,7 +48,8 @@ struct audit_krule;
* These functions are in security/capability.c and are used
* as the default capabilities functions
*/
-extern int cap_capable(struct task_struct *tsk, int cap, int audit);
+extern int cap_capable(struct task_struct *tsk, const struct cred *cred,
+ int cap, int audit);
extern int cap_settime(struct timespec *ts, struct timezone *tz);
extern int cap_ptrace_may_access(struct task_struct *child, unsigned int mode);
extern int cap_ptrace_traceme(struct task_struct *parent);
@@ -1251,9 +1252,12 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
* @permitted contains the permitted capability set.
* Return 0 and update @new if permission is granted.
* @capable:
- * Check whether the @tsk process has the @cap capability.
+ * Check whether the @tsk process has the @cap capability in the indicated
+ * credentials.
* @tsk contains the task_struct for the process.
+ * @cred contains the credentials to use.
* @cap contains the capability <include/linux/capability.h>.
+ * @audit: Whether to write an audit message or not
* Return 0 if the capability is granted for @tsk.
* @acct:
* Check permission before enabling or disabling process accounting. If
@@ -1346,7 +1350,8 @@ struct security_operations {
const kernel_cap_t *effective,
const kernel_cap_t *inheritable,
const kernel_cap_t *permitted);
- int (*capable) (struct task_struct *tsk, int cap, int audit);
+ int (*capable) (struct task_struct *tsk, const struct cred *cred,
+ int cap, int audit);
int (*acct) (struct file *file);
int (*sysctl) (struct ctl_table *table, int op);
int (*quotactl) (int cmds, int type, int id, struct super_block *sb);
@@ -1628,8 +1633,9 @@ int security_capset(struct cred *new, const struct cred *old,
const kernel_cap_t *effective,
const kernel_cap_t *inheritable,
const kernel_cap_t *permitted);
-int security_capable(struct task_struct *tsk, int cap);
-int security_capable_noaudit(struct task_struct *tsk, int cap);
+int security_capable(int cap);
+int security_real_capable(struct task_struct *tsk, int cap);
+int security_real_capable_noaudit(struct task_struct *tsk, int cap);
int security_acct(struct file *file);
int security_sysctl(struct ctl_table *table, int op);
int security_quotactl(int cmds, int type, int id, struct super_block *sb);
@@ -1826,14 +1832,31 @@ static inline int security_capset(struct cred *new,
return cap_capset(new, old, effective, inheritable, permitted);
}
-static inline int security_capable(struct task_struct *tsk, int cap)
+static inline int security_capable(int cap)
{
- return cap_capable(tsk, cap, SECURITY_CAP_AUDIT);
+ return cap_capable(current, current_cred(), cap, SECURITY_CAP_AUDIT);
}
-static inline int security_capable_noaudit(struct task_struct *tsk, int cap)
+static inline int security_real_capable(struct task_struct *tsk, int cap)
{
- return cap_capable(tsk, cap, SECURITY_CAP_NOAUDIT);
+ int ret;
+
+ rcu_read_lock();
+ ret = cap_capable(tsk, __task_cred(tsk), cap, SECURITY_CAP_AUDIT);
+ rcu_read_unlock();
+ return ret;
+}
+
+static inline
+int security_real_capable_noaudit(struct task_struct *tsk, int cap)
+{
+ int ret;
+
+ rcu_read_lock();
+ ret = cap_capable(tsk, __task_cred(tsk), cap,
+ SECURITY_CAP_NOAUDIT);
+ rcu_read_unlock();
+ return ret;
}
static inline int security_acct(struct file *file)
diff --git a/kernel/capability.c b/kernel/capability.c
index c598d9d..688926e 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -306,7 +306,7 @@ int capable(int cap)
BUG();
}
- if (has_capability(current, cap)) {
+ if (security_capable(cap) == 0) {
current->flags |= PF_SUPERPRIV;
return 1;
}
diff --git a/security/commoncap.c b/security/commoncap.c
index 7971354..f0e671d 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -45,26 +45,22 @@ EXPORT_SYMBOL(cap_netlink_recv);
/**
* cap_capable - Determine whether a task has a particular effective capability
* @tsk: The task to query
+ * @cred: The credentials to use
* @cap: The capability to check for
* @audit: Whether to write an audit message or not
*
* Determine whether the nominated task has the specified capability amongst
* its effective set, returning 0 if it does, -ve if it does not.
*
- * NOTE WELL: cap_capable() cannot be used like the kernel's capable()
- * function. That is, it has the reverse semantics: cap_capable() returns 0
- * when a task has a capability, but the kernel's capable() returns 1 for this
- * case.
+ * NOTE WELL: cap_has_capability() cannot be used like the kernel's capable()
+ * and has_capability() functions. That is, it has the reverse semantics:
+ * cap_has_capability() returns 0 when a task has a capability, but the
+ * kernel's capable() and has_capability() returns 1 for this case.
*/
-int cap_capable(struct task_struct *tsk, int cap, int audit)
+int cap_capable(struct task_struct *tsk, const struct cred *cred, int cap,
+ int audit)
{
- __u32 cap_raised;
-
- /* Derived from include/linux/sched.h:capable. */
- rcu_read_lock();
- cap_raised = cap_raised(__task_cred(tsk)->cap_effective, cap);
- rcu_read_unlock();
- return cap_raised ? 0 : -EPERM;
+ return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM;
}
/**
@@ -160,7 +156,8 @@ static inline int cap_inh_is_capped(void)
/* they are so limited unless the current task has the CAP_SETPCAP
* capability
*/
- if (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) == 0)
+ if (cap_capable(current, current_cred(), CAP_SETPCAP,
+ SECURITY_CAP_AUDIT) == 0)
return 0;
#endif
return 1;
@@ -869,7 +866,8 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
& (new->securebits ^ arg2)) /*[1]*/
|| ((new->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/
|| (arg2 & ~(SECURE_ALL_LOCKS | SECURE_ALL_BITS)) /*[3]*/
- || (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) != 0) /*[4]*/
+ || (cap_capable(current, current_cred(), CAP_SETPCAP,
+ SECURITY_CAP_AUDIT) != 0) /*[4]*/
/*
* [1] no changing of bits that are locked
* [2] no unlocking of locks
@@ -950,7 +948,8 @@ int cap_vm_enough_memory(struct mm_struct *mm, long pages)
{
int cap_sys_admin = 0;
- if (cap_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT) == 0)
+ if (cap_capable(current, current_cred(), CAP_SYS_ADMIN,
+ SECURITY_CAP_NOAUDIT) == 0)
cap_sys_admin = 1;
return __vm_enough_memory(mm, pages, cap_sys_admin);
}
diff --git a/security/security.c b/security/security.c
index 678d4d0..c3586c0 100644
--- a/security/security.c
+++ b/security/security.c
@@ -154,14 +154,32 @@ int security_capset(struct cred *new, const struct cred *old,
effective, inheritable, permitted);
}
-int security_capable(struct task_struct *tsk, int cap)
+int security_capable(int cap)
{
- return security_ops->capable(tsk, cap, SECURITY_CAP_AUDIT);
+ return security_ops->capable(current, current_cred(), cap,
+ SECURITY_CAP_AUDIT);
}
-int security_capable_noaudit(struct task_struct *tsk, int cap)
+int security_real_capable(struct task_struct *tsk, int cap)
{
- return security_ops->capable(tsk, cap, SECURITY_CAP_NOAUDIT);
+ const struct cred *cred;
+ int ret;
+
+ cred = get_task_cred(tsk);
+ ret = security_ops->capable(tsk, cred, cap, SECURITY_CAP_AUDIT);
+ put_cred(cred);
+ return ret;
+}
+
+int security_real_capable_noaudit(struct task_struct *tsk, int cap)
+{
+ const struct cred *cred;
+ int ret;
+
+ cred = get_task_cred(tsk);
+ ret = security_ops->capable(tsk, cred, cap, SECURITY_CAP_NOAUDIT);
+ put_cred(cred);
+ return ret;
}
int security_acct(struct file *file)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index dbeaa78..e0cb106 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1433,12 +1433,13 @@ static int current_has_perm(const struct task_struct *tsk,
/* Check whether a task is allowed to use a capability. */
static int task_has_capability(struct task_struct *tsk,
+ const struct cred *cred,
int cap, int audit)
{
struct avc_audit_data ad;
struct av_decision avd;
u16 sclass;
- u32 sid = task_sid(tsk);
+ u32 sid = cred_sid(cred);
u32 av = CAP_TO_MASK(cap);
int rc;
@@ -1865,15 +1866,16 @@ static int selinux_capset(struct cred *new, const struct cred *old,
return cred_has_perm(old, new, PROCESS__SETCAP);
}
-static int selinux_capable(struct task_struct *tsk, int cap, int audit)
+static int selinux_capable(struct task_struct *tsk, const struct cred *cred,
+ int cap, int audit)
{
int rc;
- rc = secondary_ops->capable(tsk, cap, audit);
+ rc = secondary_ops->capable(tsk, cred, cap, audit);
if (rc)
return rc;
- return task_has_capability(tsk, cap, audit);
+ return task_has_capability(tsk, cred, cap, audit);
}
static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid)
@@ -2037,7 +2039,8 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages)
{
int rc, cap_sys_admin = 0;
- rc = selinux_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT);
+ rc = selinux_capable(current, current_cred(), CAP_SYS_ADMIN,
+ SECURITY_CAP_NOAUDIT);
if (rc == 0)
cap_sys_admin = 1;
@@ -2880,7 +2883,8 @@ static int selinux_inode_getsecurity(const struct inode *inode, const char *name
* and lack of permission just means that we fall back to the
* in-core context value, not a denial.
*/
- error = selinux_capable(current, CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT);
+ error = selinux_capable(current, current_cred(), CAP_MAC_ADMIN,
+ SECURITY_CAP_NOAUDIT);
if (!error)
error = security_sid_to_context_force(isec->sid, &context,
&size);
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]
2009-01-06 20:39 ` David Howells
@ 2009-01-06 20:56 ` Serge E. Hallyn
0 siblings, 0 replies; 41+ messages in thread
From: Serge E. Hallyn @ 2009-01-06 20:56 UTC (permalink / raw)
To: David Howells
Cc: James Morris, Christoph Hellwig, linux-kernel, linux-fsdevel,
linux-security-module, Stephen Rothwell, Andrew Morgan
Quoting David Howells (dhowells@redhat.com):
> Serge E. Hallyn <serue@us.ibm.com> wrote:
>
> > Well I'd rather it be called acting_capable() or self_acting_capable(),
> > but the realy issue is how to make that work through the security_ops()
> > layer without needless code duplication.
>
> Given that the code duplication permits heavy optimisation of the most often
> called path (by a fair stretch), I'd say that the duplication isn't precisely
> 'needless'. You'd be trading code space in favour of execution time, and I
> think the amount of code is small enough that it'd be reasonable.
>
> I'd prefer to call the security ops 'capable()' and 'has_capability()', I
> think, but I can't do that as long as has_capability() is a macro. Changing
> it to an inline function isn't straightforward, though.
I'm ok with that naming. At least the names are different enough that
it will be easy to remember that capable() means "really capable given
acting_as creds" and has_capability() means "has the real creds."
> > Cool, thanks. In the meantime, I guess your first patch is in security-next
> > anyway, right?
>
> Yes.
>
> Anyway, how about the attached? I've updated the description a bit.
It's already on the patch, but just to confirm:
Acked-by: Serge Hallyn <serue@us.ibm.com>
> David
> ---
> From: David Howells <dhowells@redhat.com>
> Subject: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat()
>
> Fix a regression in cap_capable() due to:
>
> commit 3b11a1decef07c19443d24ae926982bc8ec9f4c0
> Author: David Howells <dhowells@redhat.com>
> Date: Fri Nov 14 10:39:26 2008 +1100
>
> CRED: Differentiate objective and effective subjective credentials on a task
>
>
> The problem is that the above patch allows a process to have two sets of
> credentials, and for the most part uses the subjective credentials when
> accessing current's creds.
>
> There is, however, one exception: cap_capable(), and thus capable(), uses the
> real/objective credentials of the target task, whether or not it is the current
> task.
>
> Ordinarily this doesn't matter, since usually the two cred pointers in current
> point to the same set of creds. However, sys_faccessat() makes use of this
> facility to override the credentials of the calling process to make its test,
> without affecting the creds as seen from other processes.
>
> One of the things sys_faccessat() does is to make an adjustment to the
> effective capabilities mask, which cap_capable(), as it stands, then ignores.
>
> The affected capability check is in generic_permission():
>
> if (!(mask & MAY_EXEC) || execute_ok(inode))
> if (capable(CAP_DAC_OVERRIDE))
> return 0;
>
>
> This change passes the set of credentials to be tested down into the commoncap
> and SELinux code. The security functions called by capable() and
> has_capability() select the appropriate set of credentials from the process
> being checked.
>
>
> This can be tested by compiling the following program from the XFS testsuite:
>
> /*
> * t_access_root.c - trivial test program to show permission bug.
> *
> * Written by Michael Kerrisk - copyright ownership not pursued.
> * Sourced from: http://linux.derkeiler.com/Mailing-Lists/Kernel/2003-10/6030.html
> */
> #include <limits.h>
> #include <unistd.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <fcntl.h>
> #include <sys/stat.h>
>
> #define UID 500
> #define GID 100
> #define PERM 0
> #define TESTPATH "/tmp/t_access"
>
> static void
> errExit(char *msg)
> {
> perror(msg);
> exit(EXIT_FAILURE);
> } /* errExit */
>
> static void
> accessTest(char *file, int mask, char *mstr)
> {
> printf("access(%s, %s) returns %d\n", file, mstr, access(file, mask));
> } /* accessTest */
>
> int
> main(int argc, char *argv[])
> {
> int fd, perm, uid, gid;
> char *testpath;
> char cmd[PATH_MAX + 20];
>
> testpath = (argc > 1) ? argv[1] : TESTPATH;
> perm = (argc > 2) ? strtoul(argv[2], NULL, 8) : PERM;
> uid = (argc > 3) ? atoi(argv[3]) : UID;
> gid = (argc > 4) ? atoi(argv[4]) : GID;
>
> unlink(testpath);
>
> fd = open(testpath, O_RDWR | O_CREAT, 0);
> if (fd == -1) errExit("open");
>
> if (fchown(fd, uid, gid) == -1) errExit("fchown");
> if (fchmod(fd, perm) == -1) errExit("fchmod");
> close(fd);
>
> snprintf(cmd, sizeof(cmd), "ls -l %s", testpath);
> system(cmd);
>
> if (seteuid(uid) == -1) errExit("seteuid");
>
> accessTest(testpath, 0, "0");
> accessTest(testpath, R_OK, "R_OK");
> accessTest(testpath, W_OK, "W_OK");
> accessTest(testpath, X_OK, "X_OK");
> accessTest(testpath, R_OK | W_OK, "R_OK | W_OK");
> accessTest(testpath, R_OK | X_OK, "R_OK | X_OK");
> accessTest(testpath, W_OK | X_OK, "W_OK | X_OK");
> accessTest(testpath, R_OK | W_OK | X_OK, "R_OK | W_OK | X_OK");
>
> exit(EXIT_SUCCESS);
> } /* main */
>
>
> This can be run against an Ext3 filesystem as well as against an XFS
> filesystem. If successful, it will show:
>
> [root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043
> ---------- 1 dhowells dhowells 0 2008-12-31 03:00 /tmp/xxx
> access(/tmp/xxx, 0) returns 0
> access(/tmp/xxx, R_OK) returns 0
> access(/tmp/xxx, W_OK) returns 0
> access(/tmp/xxx, X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK) returns 0
> access(/tmp/xxx, R_OK | X_OK) returns -1
> access(/tmp/xxx, W_OK | X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1
>
> If unsuccessful, it will show:
>
> [root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043
> ---------- 1 dhowells dhowells 0 2008-12-31 02:56 /tmp/xxx
> access(/tmp/xxx, 0) returns 0
> access(/tmp/xxx, R_OK) returns -1
> access(/tmp/xxx, W_OK) returns -1
> access(/tmp/xxx, X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK) returns -1
> access(/tmp/xxx, R_OK | X_OK) returns -1
> access(/tmp/xxx, W_OK | X_OK) returns -1
> access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1
>
> I've also tested the fix with the SELinux and syscalls LTP testsuites.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Tested-by: J. Bruce Fields <bfields@citi.umich.edu>
> Acked-by: Serge Hallyn <serue@us.ibm.com>
> ---
>
> include/linux/capability.h | 17 +++++++++++++++--
> include/linux/security.h | 41 ++++++++++++++++++++++++++++++++---------
> kernel/capability.c | 2 +-
> security/commoncap.c | 29 ++++++++++++++---------------
> security/security.c | 26 ++++++++++++++++++++++----
> security/selinux/hooks.c | 16 ++++++++++------
> 6 files changed, 94 insertions(+), 37 deletions(-)
>
>
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index e22f48c..02bdb76 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -529,8 +529,21 @@ extern const kernel_cap_t __cap_init_eff_set;
> *
> * Note that this does not set PF_SUPERPRIV on the task.
> */
> -#define has_capability(t, cap) (security_capable((t), (cap)) == 0)
> -#define has_capability_noaudit(t, cap) (security_capable_noaudit((t), (cap)) == 0)
> +#define has_capability(t, cap) (security_real_capable((t), (cap)) == 0)
> +
> +/**
> + * has_capability_noaudit - Determine if a task has a superior capability available (unaudited)
> + * @t: The task in question
> + * @cap: The capability to be tested for
> + *
> + * Return true if the specified task has the given superior capability
> + * currently in effect, false if not, but don't write an audit message for the
> + * check.
> + *
> + * Note that this does not set PF_SUPERPRIV on the task.
> + */
> +#define has_capability_noaudit(t, cap) \
> + (security_real_capable_noaudit((t), (cap)) == 0)
>
> extern int capable(int cap);
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index b92b5e4..1f2ab63 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -48,7 +48,8 @@ struct audit_krule;
> * These functions are in security/capability.c and are used
> * as the default capabilities functions
> */
> -extern int cap_capable(struct task_struct *tsk, int cap, int audit);
> +extern int cap_capable(struct task_struct *tsk, const struct cred *cred,
> + int cap, int audit);
> extern int cap_settime(struct timespec *ts, struct timezone *tz);
> extern int cap_ptrace_may_access(struct task_struct *child, unsigned int mode);
> extern int cap_ptrace_traceme(struct task_struct *parent);
> @@ -1251,9 +1252,12 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
> * @permitted contains the permitted capability set.
> * Return 0 and update @new if permission is granted.
> * @capable:
> - * Check whether the @tsk process has the @cap capability.
> + * Check whether the @tsk process has the @cap capability in the indicated
> + * credentials.
> * @tsk contains the task_struct for the process.
> + * @cred contains the credentials to use.
> * @cap contains the capability <include/linux/capability.h>.
> + * @audit: Whether to write an audit message or not
> * Return 0 if the capability is granted for @tsk.
> * @acct:
> * Check permission before enabling or disabling process accounting. If
> @@ -1346,7 +1350,8 @@ struct security_operations {
> const kernel_cap_t *effective,
> const kernel_cap_t *inheritable,
> const kernel_cap_t *permitted);
> - int (*capable) (struct task_struct *tsk, int cap, int audit);
> + int (*capable) (struct task_struct *tsk, const struct cred *cred,
> + int cap, int audit);
> int (*acct) (struct file *file);
> int (*sysctl) (struct ctl_table *table, int op);
> int (*quotactl) (int cmds, int type, int id, struct super_block *sb);
> @@ -1628,8 +1633,9 @@ int security_capset(struct cred *new, const struct cred *old,
> const kernel_cap_t *effective,
> const kernel_cap_t *inheritable,
> const kernel_cap_t *permitted);
> -int security_capable(struct task_struct *tsk, int cap);
> -int security_capable_noaudit(struct task_struct *tsk, int cap);
> +int security_capable(int cap);
> +int security_real_capable(struct task_struct *tsk, int cap);
> +int security_real_capable_noaudit(struct task_struct *tsk, int cap);
> int security_acct(struct file *file);
> int security_sysctl(struct ctl_table *table, int op);
> int security_quotactl(int cmds, int type, int id, struct super_block *sb);
> @@ -1826,14 +1832,31 @@ static inline int security_capset(struct cred *new,
> return cap_capset(new, old, effective, inheritable, permitted);
> }
>
> -static inline int security_capable(struct task_struct *tsk, int cap)
> +static inline int security_capable(int cap)
> {
> - return cap_capable(tsk, cap, SECURITY_CAP_AUDIT);
> + return cap_capable(current, current_cred(), cap, SECURITY_CAP_AUDIT);
> }
>
> -static inline int security_capable_noaudit(struct task_struct *tsk, int cap)
> +static inline int security_real_capable(struct task_struct *tsk, int cap)
> {
> - return cap_capable(tsk, cap, SECURITY_CAP_NOAUDIT);
> + int ret;
> +
> + rcu_read_lock();
> + ret = cap_capable(tsk, __task_cred(tsk), cap, SECURITY_CAP_AUDIT);
> + rcu_read_unlock();
> + return ret;
> +}
> +
> +static inline
> +int security_real_capable_noaudit(struct task_struct *tsk, int cap)
> +{
> + int ret;
> +
> + rcu_read_lock();
> + ret = cap_capable(tsk, __task_cred(tsk), cap,
> + SECURITY_CAP_NOAUDIT);
> + rcu_read_unlock();
> + return ret;
> }
>
> static inline int security_acct(struct file *file)
> diff --git a/kernel/capability.c b/kernel/capability.c
> index c598d9d..688926e 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -306,7 +306,7 @@ int capable(int cap)
> BUG();
> }
>
> - if (has_capability(current, cap)) {
> + if (security_capable(cap) == 0) {
> current->flags |= PF_SUPERPRIV;
> return 1;
> }
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 7971354..f0e671d 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -45,26 +45,22 @@ EXPORT_SYMBOL(cap_netlink_recv);
> /**
> * cap_capable - Determine whether a task has a particular effective capability
> * @tsk: The task to query
> + * @cred: The credentials to use
> * @cap: The capability to check for
> * @audit: Whether to write an audit message or not
> *
> * Determine whether the nominated task has the specified capability amongst
> * its effective set, returning 0 if it does, -ve if it does not.
> *
> - * NOTE WELL: cap_capable() cannot be used like the kernel's capable()
> - * function. That is, it has the reverse semantics: cap_capable() returns 0
> - * when a task has a capability, but the kernel's capable() returns 1 for this
> - * case.
> + * NOTE WELL: cap_has_capability() cannot be used like the kernel's capable()
> + * and has_capability() functions. That is, it has the reverse semantics:
> + * cap_has_capability() returns 0 when a task has a capability, but the
> + * kernel's capable() and has_capability() returns 1 for this case.
> */
> -int cap_capable(struct task_struct *tsk, int cap, int audit)
> +int cap_capable(struct task_struct *tsk, const struct cred *cred, int cap,
> + int audit)
> {
> - __u32 cap_raised;
> -
> - /* Derived from include/linux/sched.h:capable. */
> - rcu_read_lock();
> - cap_raised = cap_raised(__task_cred(tsk)->cap_effective, cap);
> - rcu_read_unlock();
> - return cap_raised ? 0 : -EPERM;
> + return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM;
> }
>
> /**
> @@ -160,7 +156,8 @@ static inline int cap_inh_is_capped(void)
> /* they are so limited unless the current task has the CAP_SETPCAP
> * capability
> */
> - if (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) == 0)
> + if (cap_capable(current, current_cred(), CAP_SETPCAP,
> + SECURITY_CAP_AUDIT) == 0)
> return 0;
> #endif
> return 1;
> @@ -869,7 +866,8 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> & (new->securebits ^ arg2)) /*[1]*/
> || ((new->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/
> || (arg2 & ~(SECURE_ALL_LOCKS | SECURE_ALL_BITS)) /*[3]*/
> - || (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) != 0) /*[4]*/
> + || (cap_capable(current, current_cred(), CAP_SETPCAP,
> + SECURITY_CAP_AUDIT) != 0) /*[4]*/
> /*
> * [1] no changing of bits that are locked
> * [2] no unlocking of locks
> @@ -950,7 +948,8 @@ int cap_vm_enough_memory(struct mm_struct *mm, long pages)
> {
> int cap_sys_admin = 0;
>
> - if (cap_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT) == 0)
> + if (cap_capable(current, current_cred(), CAP_SYS_ADMIN,
> + SECURITY_CAP_NOAUDIT) == 0)
> cap_sys_admin = 1;
> return __vm_enough_memory(mm, pages, cap_sys_admin);
> }
> diff --git a/security/security.c b/security/security.c
> index 678d4d0..c3586c0 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -154,14 +154,32 @@ int security_capset(struct cred *new, const struct cred *old,
> effective, inheritable, permitted);
> }
>
> -int security_capable(struct task_struct *tsk, int cap)
> +int security_capable(int cap)
> {
> - return security_ops->capable(tsk, cap, SECURITY_CAP_AUDIT);
> + return security_ops->capable(current, current_cred(), cap,
> + SECURITY_CAP_AUDIT);
> }
>
> -int security_capable_noaudit(struct task_struct *tsk, int cap)
> +int security_real_capable(struct task_struct *tsk, int cap)
> {
> - return security_ops->capable(tsk, cap, SECURITY_CAP_NOAUDIT);
> + const struct cred *cred;
> + int ret;
> +
> + cred = get_task_cred(tsk);
> + ret = security_ops->capable(tsk, cred, cap, SECURITY_CAP_AUDIT);
> + put_cred(cred);
> + return ret;
> +}
> +
> +int security_real_capable_noaudit(struct task_struct *tsk, int cap)
> +{
> + const struct cred *cred;
> + int ret;
> +
> + cred = get_task_cred(tsk);
> + ret = security_ops->capable(tsk, cred, cap, SECURITY_CAP_NOAUDIT);
> + put_cred(cred);
> + return ret;
> }
>
> int security_acct(struct file *file)
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index dbeaa78..e0cb106 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1433,12 +1433,13 @@ static int current_has_perm(const struct task_struct *tsk,
>
> /* Check whether a task is allowed to use a capability. */
> static int task_has_capability(struct task_struct *tsk,
> + const struct cred *cred,
> int cap, int audit)
> {
> struct avc_audit_data ad;
> struct av_decision avd;
> u16 sclass;
> - u32 sid = task_sid(tsk);
> + u32 sid = cred_sid(cred);
> u32 av = CAP_TO_MASK(cap);
> int rc;
>
> @@ -1865,15 +1866,16 @@ static int selinux_capset(struct cred *new, const struct cred *old,
> return cred_has_perm(old, new, PROCESS__SETCAP);
> }
>
> -static int selinux_capable(struct task_struct *tsk, int cap, int audit)
> +static int selinux_capable(struct task_struct *tsk, const struct cred *cred,
> + int cap, int audit)
> {
> int rc;
>
> - rc = secondary_ops->capable(tsk, cap, audit);
> + rc = secondary_ops->capable(tsk, cred, cap, audit);
> if (rc)
> return rc;
>
> - return task_has_capability(tsk, cap, audit);
> + return task_has_capability(tsk, cred, cap, audit);
> }
>
> static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid)
> @@ -2037,7 +2039,8 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages)
> {
> int rc, cap_sys_admin = 0;
>
> - rc = selinux_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT);
> + rc = selinux_capable(current, current_cred(), CAP_SYS_ADMIN,
> + SECURITY_CAP_NOAUDIT);
> if (rc == 0)
> cap_sys_admin = 1;
>
> @@ -2880,7 +2883,8 @@ static int selinux_inode_getsecurity(const struct inode *inode, const char *name
> * and lack of permission just means that we fall back to the
> * in-core context value, not a denial.
> */
> - error = selinux_capable(current, CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT);
> + error = selinux_capable(current, current_cred(), CAP_MAC_ADMIN,
> + SECURITY_CAP_NOAUDIT);
> if (!error)
> error = security_sid_to_context_force(isec->sid, &context,
> &size);
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #3]
2009-01-05 2:07 ` James Morris
2009-01-05 3:18 ` Serge E. Hallyn
@ 2009-01-06 22:27 ` David Howells
2009-01-06 22:53 ` James Morris
1 sibling, 1 reply; 41+ messages in thread
From: David Howells @ 2009-01-06 22:27 UTC (permalink / raw)
To: James Morris
Cc: dhowells, Christoph Hellwig, linux-kernel, linux-fsdevel,
Serge E. Hallyn, linux-security-module, Stephen Rothwell, bfields
Fix a regression in cap_capable() due to:
commit 3b11a1decef07c19443d24ae926982bc8ec9f4c0
Author: David Howells <dhowells@redhat.com>
Date: Fri Nov 14 10:39:26 2008 +1100
CRED: Differentiate objective and effective subjective credentials on a task
The problem is that the above patch allows a process to have two sets of
credentials, and for the most part uses the subjective credentials when
accessing current's creds.
There is, however, one exception: cap_capable(), and thus capable(), uses the
real/objective credentials of the target task, whether or not it is the current
task.
Ordinarily this doesn't matter, since usually the two cred pointers in current
point to the same set of creds. However, sys_faccessat() makes use of this
facility to override the credentials of the calling process to make its test,
without affecting the creds as seen from other processes.
One of the things sys_faccessat() does is to make an adjustment to the
effective capabilities mask, which cap_capable(), as it stands, then ignores.
The affected capability check is in generic_permission():
if (!(mask & MAY_EXEC) || execute_ok(inode))
if (capable(CAP_DAC_OVERRIDE))
return 0;
This change passes the set of credentials to be tested down into the commoncap
and SELinux code. The security functions called by capable() and
has_capability() select the appropriate set of credentials from the process
being checked.
This can be tested by compiling the following program from the XFS testsuite:
/*
* t_access_root.c - trivial test program to show permission bug.
*
* Written by Michael Kerrisk - copyright ownership not pursued.
* Sourced from: http://linux.derkeiler.com/Mailing-Lists/Kernel/2003-10/6030.html
*/
#include <limits.h>
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <sys/stat.h>
#define UID 500
#define GID 100
#define PERM 0
#define TESTPATH "/tmp/t_access"
static void
errExit(char *msg)
{
perror(msg);
exit(EXIT_FAILURE);
} /* errExit */
static void
accessTest(char *file, int mask, char *mstr)
{
printf("access(%s, %s) returns %d\n", file, mstr, access(file, mask));
} /* accessTest */
int
main(int argc, char *argv[])
{
int fd, perm, uid, gid;
char *testpath;
char cmd[PATH_MAX + 20];
testpath = (argc > 1) ? argv[1] : TESTPATH;
perm = (argc > 2) ? strtoul(argv[2], NULL, 8) : PERM;
uid = (argc > 3) ? atoi(argv[3]) : UID;
gid = (argc > 4) ? atoi(argv[4]) : GID;
unlink(testpath);
fd = open(testpath, O_RDWR | O_CREAT, 0);
if (fd == -1) errExit("open");
if (fchown(fd, uid, gid) == -1) errExit("fchown");
if (fchmod(fd, perm) == -1) errExit("fchmod");
close(fd);
snprintf(cmd, sizeof(cmd), "ls -l %s", testpath);
system(cmd);
if (seteuid(uid) == -1) errExit("seteuid");
accessTest(testpath, 0, "0");
accessTest(testpath, R_OK, "R_OK");
accessTest(testpath, W_OK, "W_OK");
accessTest(testpath, X_OK, "X_OK");
accessTest(testpath, R_OK | W_OK, "R_OK | W_OK");
accessTest(testpath, R_OK | X_OK, "R_OK | X_OK");
accessTest(testpath, W_OK | X_OK, "W_OK | X_OK");
accessTest(testpath, R_OK | W_OK | X_OK, "R_OK | W_OK | X_OK");
exit(EXIT_SUCCESS);
} /* main */
This can be run against an Ext3 filesystem as well as against an XFS
filesystem. If successful, it will show:
[root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043
---------- 1 dhowells dhowells 0 2008-12-31 03:00 /tmp/xxx
access(/tmp/xxx, 0) returns 0
access(/tmp/xxx, R_OK) returns 0
access(/tmp/xxx, W_OK) returns 0
access(/tmp/xxx, X_OK) returns -1
access(/tmp/xxx, R_OK | W_OK) returns 0
access(/tmp/xxx, R_OK | X_OK) returns -1
access(/tmp/xxx, W_OK | X_OK) returns -1
access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1
If unsuccessful, it will show:
[root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043
---------- 1 dhowells dhowells 0 2008-12-31 02:56 /tmp/xxx
access(/tmp/xxx, 0) returns 0
access(/tmp/xxx, R_OK) returns -1
access(/tmp/xxx, W_OK) returns -1
access(/tmp/xxx, X_OK) returns -1
access(/tmp/xxx, R_OK | W_OK) returns -1
access(/tmp/xxx, R_OK | X_OK) returns -1
access(/tmp/xxx, W_OK | X_OK) returns -1
access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1
I've also tested the fix with the SELinux and syscalls LTP testsuites.
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: J. Bruce Fields <bfields@citi.umich.edu>
Acked-by: Serge Hallyn <serue@us.ibm.com>
---
include/linux/capability.h | 17 +++++++++++++++--
include/linux/security.h | 41 ++++++++++++++++++++++++++++++++---------
kernel/capability.c | 2 +-
security/commoncap.c | 29 ++++++++++++++---------------
security/security.c | 26 ++++++++++++++++++++++----
security/selinux/hooks.c | 16 ++++++++++------
6 files changed, 94 insertions(+), 37 deletions(-)
diff --git a/include/linux/capability.h b/include/linux/capability.h
index e22f48c..02bdb76 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -529,8 +529,21 @@ extern const kernel_cap_t __cap_init_eff_set;
*
* Note that this does not set PF_SUPERPRIV on the task.
*/
-#define has_capability(t, cap) (security_capable((t), (cap)) == 0)
-#define has_capability_noaudit(t, cap) (security_capable_noaudit((t), (cap)) == 0)
+#define has_capability(t, cap) (security_real_capable((t), (cap)) == 0)
+
+/**
+ * has_capability_noaudit - Determine if a task has a superior capability available (unaudited)
+ * @t: The task in question
+ * @cap: The capability to be tested for
+ *
+ * Return true if the specified task has the given superior capability
+ * currently in effect, false if not, but don't write an audit message for the
+ * check.
+ *
+ * Note that this does not set PF_SUPERPRIV on the task.
+ */
+#define has_capability_noaudit(t, cap) \
+ (security_real_capable_noaudit((t), (cap)) == 0)
extern int capable(int cap);
diff --git a/include/linux/security.h b/include/linux/security.h
index b92b5e4..1f2ab63 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -48,7 +48,8 @@ struct audit_krule;
* These functions are in security/capability.c and are used
* as the default capabilities functions
*/
-extern int cap_capable(struct task_struct *tsk, int cap, int audit);
+extern int cap_capable(struct task_struct *tsk, const struct cred *cred,
+ int cap, int audit);
extern int cap_settime(struct timespec *ts, struct timezone *tz);
extern int cap_ptrace_may_access(struct task_struct *child, unsigned int mode);
extern int cap_ptrace_traceme(struct task_struct *parent);
@@ -1251,9 +1252,12 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
* @permitted contains the permitted capability set.
* Return 0 and update @new if permission is granted.
* @capable:
- * Check whether the @tsk process has the @cap capability.
+ * Check whether the @tsk process has the @cap capability in the indicated
+ * credentials.
* @tsk contains the task_struct for the process.
+ * @cred contains the credentials to use.
* @cap contains the capability <include/linux/capability.h>.
+ * @audit: Whether to write an audit message or not
* Return 0 if the capability is granted for @tsk.
* @acct:
* Check permission before enabling or disabling process accounting. If
@@ -1346,7 +1350,8 @@ struct security_operations {
const kernel_cap_t *effective,
const kernel_cap_t *inheritable,
const kernel_cap_t *permitted);
- int (*capable) (struct task_struct *tsk, int cap, int audit);
+ int (*capable) (struct task_struct *tsk, const struct cred *cred,
+ int cap, int audit);
int (*acct) (struct file *file);
int (*sysctl) (struct ctl_table *table, int op);
int (*quotactl) (int cmds, int type, int id, struct super_block *sb);
@@ -1628,8 +1633,9 @@ int security_capset(struct cred *new, const struct cred *old,
const kernel_cap_t *effective,
const kernel_cap_t *inheritable,
const kernel_cap_t *permitted);
-int security_capable(struct task_struct *tsk, int cap);
-int security_capable_noaudit(struct task_struct *tsk, int cap);
+int security_capable(int cap);
+int security_real_capable(struct task_struct *tsk, int cap);
+int security_real_capable_noaudit(struct task_struct *tsk, int cap);
int security_acct(struct file *file);
int security_sysctl(struct ctl_table *table, int op);
int security_quotactl(int cmds, int type, int id, struct super_block *sb);
@@ -1826,14 +1832,31 @@ static inline int security_capset(struct cred *new,
return cap_capset(new, old, effective, inheritable, permitted);
}
-static inline int security_capable(struct task_struct *tsk, int cap)
+static inline int security_capable(int cap)
{
- return cap_capable(tsk, cap, SECURITY_CAP_AUDIT);
+ return cap_capable(current, current_cred(), cap, SECURITY_CAP_AUDIT);
}
-static inline int security_capable_noaudit(struct task_struct *tsk, int cap)
+static inline int security_real_capable(struct task_struct *tsk, int cap)
{
- return cap_capable(tsk, cap, SECURITY_CAP_NOAUDIT);
+ int ret;
+
+ rcu_read_lock();
+ ret = cap_capable(tsk, __task_cred(tsk), cap, SECURITY_CAP_AUDIT);
+ rcu_read_unlock();
+ return ret;
+}
+
+static inline
+int security_real_capable_noaudit(struct task_struct *tsk, int cap)
+{
+ int ret;
+
+ rcu_read_lock();
+ ret = cap_capable(tsk, __task_cred(tsk), cap,
+ SECURITY_CAP_NOAUDIT);
+ rcu_read_unlock();
+ return ret;
}
static inline int security_acct(struct file *file)
diff --git a/kernel/capability.c b/kernel/capability.c
index c598d9d..688926e 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -306,7 +306,7 @@ int capable(int cap)
BUG();
}
- if (has_capability(current, cap)) {
+ if (security_capable(cap) == 0) {
current->flags |= PF_SUPERPRIV;
return 1;
}
diff --git a/security/commoncap.c b/security/commoncap.c
index 7971354..f0e671d 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -45,26 +45,22 @@ EXPORT_SYMBOL(cap_netlink_recv);
/**
* cap_capable - Determine whether a task has a particular effective capability
* @tsk: The task to query
+ * @cred: The credentials to use
* @cap: The capability to check for
* @audit: Whether to write an audit message or not
*
* Determine whether the nominated task has the specified capability amongst
* its effective set, returning 0 if it does, -ve if it does not.
*
- * NOTE WELL: cap_capable() cannot be used like the kernel's capable()
- * function. That is, it has the reverse semantics: cap_capable() returns 0
- * when a task has a capability, but the kernel's capable() returns 1 for this
- * case.
+ * NOTE WELL: cap_has_capability() cannot be used like the kernel's capable()
+ * and has_capability() functions. That is, it has the reverse semantics:
+ * cap_has_capability() returns 0 when a task has a capability, but the
+ * kernel's capable() and has_capability() returns 1 for this case.
*/
-int cap_capable(struct task_struct *tsk, int cap, int audit)
+int cap_capable(struct task_struct *tsk, const struct cred *cred, int cap,
+ int audit)
{
- __u32 cap_raised;
-
- /* Derived from include/linux/sched.h:capable. */
- rcu_read_lock();
- cap_raised = cap_raised(__task_cred(tsk)->cap_effective, cap);
- rcu_read_unlock();
- return cap_raised ? 0 : -EPERM;
+ return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM;
}
/**
@@ -160,7 +156,8 @@ static inline int cap_inh_is_capped(void)
/* they are so limited unless the current task has the CAP_SETPCAP
* capability
*/
- if (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) == 0)
+ if (cap_capable(current, current_cred(), CAP_SETPCAP,
+ SECURITY_CAP_AUDIT) == 0)
return 0;
#endif
return 1;
@@ -869,7 +866,8 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
& (new->securebits ^ arg2)) /*[1]*/
|| ((new->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/
|| (arg2 & ~(SECURE_ALL_LOCKS | SECURE_ALL_BITS)) /*[3]*/
- || (cap_capable(current, CAP_SETPCAP, SECURITY_CAP_AUDIT) != 0) /*[4]*/
+ || (cap_capable(current, current_cred(), CAP_SETPCAP,
+ SECURITY_CAP_AUDIT) != 0) /*[4]*/
/*
* [1] no changing of bits that are locked
* [2] no unlocking of locks
@@ -950,7 +948,8 @@ int cap_vm_enough_memory(struct mm_struct *mm, long pages)
{
int cap_sys_admin = 0;
- if (cap_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT) == 0)
+ if (cap_capable(current, current_cred(), CAP_SYS_ADMIN,
+ SECURITY_CAP_NOAUDIT) == 0)
cap_sys_admin = 1;
return __vm_enough_memory(mm, pages, cap_sys_admin);
}
diff --git a/security/security.c b/security/security.c
index 678d4d0..c3586c0 100644
--- a/security/security.c
+++ b/security/security.c
@@ -154,14 +154,32 @@ int security_capset(struct cred *new, const struct cred *old,
effective, inheritable, permitted);
}
-int security_capable(struct task_struct *tsk, int cap)
+int security_capable(int cap)
{
- return security_ops->capable(tsk, cap, SECURITY_CAP_AUDIT);
+ return security_ops->capable(current, current_cred(), cap,
+ SECURITY_CAP_AUDIT);
}
-int security_capable_noaudit(struct task_struct *tsk, int cap)
+int security_real_capable(struct task_struct *tsk, int cap)
{
- return security_ops->capable(tsk, cap, SECURITY_CAP_NOAUDIT);
+ const struct cred *cred;
+ int ret;
+
+ cred = get_task_cred(tsk);
+ ret = security_ops->capable(tsk, cred, cap, SECURITY_CAP_AUDIT);
+ put_cred(cred);
+ return ret;
+}
+
+int security_real_capable_noaudit(struct task_struct *tsk, int cap)
+{
+ const struct cred *cred;
+ int ret;
+
+ cred = get_task_cred(tsk);
+ ret = security_ops->capable(tsk, cred, cap, SECURITY_CAP_NOAUDIT);
+ put_cred(cred);
+ return ret;
}
int security_acct(struct file *file)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index dbeaa78..e0cb106 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1433,12 +1433,13 @@ static int current_has_perm(const struct task_struct *tsk,
/* Check whether a task is allowed to use a capability. */
static int task_has_capability(struct task_struct *tsk,
+ const struct cred *cred,
int cap, int audit)
{
struct avc_audit_data ad;
struct av_decision avd;
u16 sclass;
- u32 sid = task_sid(tsk);
+ u32 sid = cred_sid(cred);
u32 av = CAP_TO_MASK(cap);
int rc;
@@ -1865,15 +1866,16 @@ static int selinux_capset(struct cred *new, const struct cred *old,
return cred_has_perm(old, new, PROCESS__SETCAP);
}
-static int selinux_capable(struct task_struct *tsk, int cap, int audit)
+static int selinux_capable(struct task_struct *tsk, const struct cred *cred,
+ int cap, int audit)
{
int rc;
- rc = secondary_ops->capable(tsk, cap, audit);
+ rc = secondary_ops->capable(tsk, cred, cap, audit);
if (rc)
return rc;
- return task_has_capability(tsk, cap, audit);
+ return task_has_capability(tsk, cred, cap, audit);
}
static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid)
@@ -2037,7 +2039,8 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages)
{
int rc, cap_sys_admin = 0;
- rc = selinux_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT);
+ rc = selinux_capable(current, current_cred(), CAP_SYS_ADMIN,
+ SECURITY_CAP_NOAUDIT);
if (rc == 0)
cap_sys_admin = 1;
@@ -2880,7 +2883,8 @@ static int selinux_inode_getsecurity(const struct inode *inode, const char *name
* and lack of permission just means that we fall back to the
* in-core context value, not a denial.
*/
- error = selinux_capable(current, CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT);
+ error = selinux_capable(current, current_cred(), CAP_MAC_ADMIN,
+ SECURITY_CAP_NOAUDIT);
if (!error)
error = security_sid_to_context_force(isec->sid, &context,
&size);
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #3]
2009-01-06 22:27 ` [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #3] David Howells
@ 2009-01-06 22:53 ` James Morris
2009-01-06 23:57 ` J. Bruce Fields
0 siblings, 1 reply; 41+ messages in thread
From: James Morris @ 2009-01-06 22:53 UTC (permalink / raw)
To: David Howells
Cc: Christoph Hellwig, linux-kernel, linux-fsdevel, Serge E. Hallyn,
linux-security-module, Stephen Rothwell, bfields
On Tue, 6 Jan 2009, David Howells wrote:
>
> Fix a regression in cap_capable() due to:
>
> commit 3b11a1decef07c19443d24ae926982bc8ec9f4c0
> Author: David Howells <dhowells@redhat.com>
> Date: Fri Nov 14 10:39:26 2008 +1100
>
> CRED: Differentiate objective and effective subjective credentials on a task
>
Applied to:
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #3]
2009-01-06 22:53 ` James Morris
@ 2009-01-06 23:57 ` J. Bruce Fields
2009-01-07 0:09 ` James Morris
0 siblings, 1 reply; 41+ messages in thread
From: J. Bruce Fields @ 2009-01-06 23:57 UTC (permalink / raw)
To: James Morris
Cc: David Howells, Christoph Hellwig, linux-kernel, linux-fsdevel,
Serge E. Hallyn, linux-security-module, Stephen Rothwell
On Wed, Jan 07, 2009 at 09:53:48AM +1100, James Morris wrote:
> On Tue, 6 Jan 2009, David Howells wrote:
>
> >
> > Fix a regression in cap_capable() due to:
> >
> > commit 3b11a1decef07c19443d24ae926982bc8ec9f4c0
> > Author: David Howells <dhowells@redhat.com>
> > Date: Fri Nov 14 10:39:26 2008 +1100
> >
> > CRED: Differentiate objective and effective subjective credentials on a task
> >
>
> Applied to:
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
Thanks! When should that be in mainline?
--b.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #3]
2009-01-06 23:57 ` J. Bruce Fields
@ 2009-01-07 0:09 ` James Morris
0 siblings, 0 replies; 41+ messages in thread
From: James Morris @ 2009-01-07 0:09 UTC (permalink / raw)
To: J. Bruce Fields
Cc: David Howells, Christoph Hellwig, linux-kernel, linux-fsdevel,
Serge E. Hallyn, linux-security-module, Stephen Rothwell
On Tue, 6 Jan 2009, J. Bruce Fields wrote:
> On Wed, Jan 07, 2009 at 09:53:48AM +1100, James Morris wrote:
> > On Tue, 6 Jan 2009, David Howells wrote:
> >
> > >
> > > Fix a regression in cap_capable() due to:
> > >
> > > commit 3b11a1decef07c19443d24ae926982bc8ec9f4c0
> > > Author: David Howells <dhowells@redhat.com>
> > > Date: Fri Nov 14 10:39:26 2008 +1100
> > >
> > > CRED: Differentiate objective and effective subjective credentials on a task
> > >
> >
> > Applied to:
> > git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
>
> Thanks! When should that be in mainline?
As soon as Linus pulls.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 41+ messages in thread