linux-kernel - Re: [PATCH 1/4] security: filesystem capabilities bugfix1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 30 Jun 2008 14:30:35 +0100
From:	David Howells <dhowells@...hat.com>
To:	unlisted-recipients:; (no To-header on input)
Cc:	dhowells@...hat.com, "Andrew G. Morgan" <morgan@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"Serge E. Hallyn" <serue@...ibm.com>,
	lkml <linux-kernel@...r.kernel.org>,
	Linux Security Modules List 
	<linux-security-module@...r.kernel.org>
Subject: Re: [PATCH 1/4] security: filesystem capabilities bugfix1

David Howells <dhowells@...hat.com> wrote:

> > +kernel_cap_t cap_set_effective(const kernel_cap_t pE_new)
> 
> Hmmm... kernel_cap_t is a structure that might not fit into a single register.
> It occurs to me that you might be better off returing the old caps through a
> pointer argument.

Apply something like the attached, perhaps?  (Note the attached patch is
missing the change from linux/capability.h because my version is not
compatible with your submitted patch).

David

diff --git a/fs/open.c b/fs/open.c
index 3b53948..24b4a11 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -451,9 +451,9 @@ asmlinkage long sys_faccessat(int dfd, const char __user *filename, int mode)
 		 */
 #endif /* ndef CONFIG_SECURITY_FILE_CAPABILITIES */
 		if (current->uid)
-			old_cap = cap_set_effective(__cap_empty_set);
+			cap_set_effective(&__cap_empty_set, &old_cap);
 		else
-			old_cap = cap_set_effective(current->cap_permitted);
+			cap_set_effective(&current->cap_permitted, &old_cap);
 	}
 
 	res = __user_walk_fd(dfd, filename, LOOKUP_FOLLOW|LOOKUP_ACCESS, &nd);
@@ -484,9 +484,8 @@ out:
 	current->fsuid = old_fsuid;
 	current->fsgid = old_fsgid;
 
-	if (!issecure(SECURE_NO_SETUID_FIXUP)) {
-		(void) cap_set_effective(old_cap);
-	}
+	if (!issecure(SECURE_NO_SETUID_FIXUP))
+		cap_set_effective(&old_cap, NULL);
 
 	return res;
 }
diff --git a/kernel/capability.c b/kernel/capability.c
index c3bf957..13c496a 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -126,18 +126,16 @@ static int cap_validate_magic(cap_user_header_t header, unsigned *tocopy)
  * value. No permission check is performed here - it is assumed that the
  * caller is permitted to set the desired effective capabilities.
  */
-kernel_cap_t cap_set_effective(const kernel_cap_t pE_new)
+void cap_set_effective(const kernel_cap_t *pE_new,
+		       kernel_cap_t *_pE_old)
 {
-	kernel_cap_t pE_old;
-
 	spin_lock(&task_capability_lock);
 
-	pE_old = current->cap_effective;
-	current->cap_effective = pE_new;
+	if (_pE_old)
+		*_pE_old = current->cap_effective;
+	current->cap_effective = *pE_new;
 
 	spin_unlock(&task_capability_lock);
-
-	return pE_old;
 }
 
 EXPORT_SYMBOL(cap_set_effective);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/