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