The patch titled
Fix setting of PF_SUPERPRIV by __capable()
has been added to the -mm tree. Its filename is
fix-setting-of-pf_superpriv-by-__capable.patch
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/SubmitChecklist when testing your code ***
See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
out what to do about this
The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
------------------------------------------------------
Subject: Fix setting of PF_SUPERPRIV by __capable()
From: David Howells <dhowells@xxxxxxxxxx>
Fix the setting of PF_SUPERPRIV by __capable() as it could corrupt the
flags the target process if that is not the current process and it is
trying to change its own flags in a different way at the same time.
__capable() is using neither atomic ops nor locking to protect t->flags.
This patch changes __capable() so it will only set PF_SUPERPRIV if current
is checking its own capabilities.
Possibly we should be setting PF_SUPERPRIV on current, regardless of the
target pointer; however there are cases where you probably don't want to
do this.
Two of the instances of __capable() actually only act on current, and so have
been changed to calls to capable().
Of those that remain:
(1) The OOM killer calls __capable() thrice when weighing the killability of a
process. I think it would be reasonable for this not to set PF_SUPERPRIV
at all.
(2) cap_ptrace() and smack_ptrace() are called in two different ways:
(a) The parent pointer is current, in which case it's securing PT_ATTACH,
and setting PF_SUPERPRIV is correct.
(b) The parent pointer is current->parent, in which case it's securing
PT_TRACEME, in which case not setting PF_SUPERPRIV is probably
reasonable as no-one has actually used a privileged operation as yet,
merely verified that one can be used.
(3) In smack_file_send_sigiotask(), we need to allow privileged processes to
receive SIGIO on files they're manipulating. Setting PF_SUPERPRIV on
current is wrong, but setting it on the target might be correct.
(4) In smack_task_wait(), we let a process wait for a privileged process,
whether or not the process doing the waiting is privileged. I'm not sure
whether this should set PF_SUPERPRIV, and if so on whom.
Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
Cc: James Morris <jmorris@xxxxxxxxx>
Acked-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx>
Acked-by: Serge Hallyn <serue@xxxxxxxxxx>
Cc: Stephen Smalley <sds@xxxxxxxxxxxxx>
Cc: Chris Wright <chrisw@xxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
kernel/capability.c | 3 ++-
security/commoncap.c | 2 +-
security/smack/smack_lsm.c | 6 +++---
3 files changed, 6 insertions(+), 5 deletions(-)
diff -puN kernel/capability.c~fix-setting-of-pf_superpriv-by-__capable kernel/capability.c
--- a/kernel/capability.c~fix-setting-of-pf_superpriv-by-__capable
+++ a/kernel/capability.c
@@ -364,7 +364,8 @@ out:
int __capable(struct task_struct *t, int cap)
{
if (security_capable(t, cap) == 0) {
- t->flags |= PF_SUPERPRIV;
+ if (t == current)
+ t->flags |= PF_SUPERPRIV;
return 1;
}
return 0;
diff -puN security/commoncap.c~fix-setting-of-pf_superpriv-by-__capable security/commoncap.c
--- a/security/commoncap.c~fix-setting-of-pf_superpriv-by-__capable
+++ a/security/commoncap.c
@@ -528,7 +528,7 @@ int cap_task_post_setuid (uid_t old_ruid
static inline int cap_safe_nice(struct task_struct *p)
{
if (!cap_issubset(p->cap_permitted, current->cap_permitted) &&
- !__capable(current, CAP_SYS_NICE))
+ !capable(CAP_SYS_NICE))
return -EPERM;
return 0;
}
diff -puN security/smack/smack_lsm.c~fix-setting-of-pf_superpriv-by-__capable security/smack/smack_lsm.c
--- a/security/smack/smack_lsm.c~fix-setting-of-pf_superpriv-by-__capable
+++ a/security/smack/smack_lsm.c
@@ -2038,9 +2038,6 @@ static int smack_setprocattr(struct task
{
char *newsmack;
- if (!__capable(p, CAP_MAC_ADMIN))
- return -EPERM;
-
/*
* Changing another process' Smack value is too dangerous
* and supports no sane use case.
@@ -2048,6 +2045,9 @@ static int smack_setprocattr(struct task
if (p != current)
return -EPERM;
+ if (!capable(CAP_MAC_ADMIN))
+ return -EPERM;
+
if (value == NULL || size == 0 || size >= SMK_LABELLEN)
return -EINVAL;
_
Patches currently in -mm which might be from dhowells@xxxxxxxxxx are
git-unionfs.patch
fix-setting-of-pf_superpriv-by-__capable.patch
frv-use-the-common-ascii-hex-helpers.patch
mn10300-use-the-common-ascii-hex-helpers.patch
mutex-subsystem-synchro-test-module.patch
--
To unsubscribe from this list: send the line "unsubscribe mm-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
