rpms/kernel/devel linux-2.6-selinux-deffered-context-mapping-no-sleep.patch, NONE, 1.1 linux-2.6-selinux-generic-ioctl.patch, NONE, 1.1 kernel.spec, 1.631, 1.632

[Eric Paris (eparis)]

Author: eparis

Update of /cvs/pkgs/rpms/kernel/devel
In directory cvs-int.fedora.redhat.com:/tmp/cvs-serv32335

Modified Files:
	kernel.spec 
Added Files:
	linux-2.6-selinux-deffered-context-mapping-no-sleep.patch 
	linux-2.6-selinux-generic-ioctl.patch 
Log Message:
* Wed May 14 2008 Eric Paris <eparis at redhat.com>
- fix may sleep in allocation for previous deffered context patch
- replace selinux specific knowledge of ioctls with a generic ioctl implementation


linux-2.6-selinux-deffered-context-mapping-no-sleep.patch:

--- NEW FILE linux-2.6-selinux-deffered-context-mapping-no-sleep.patch ---
Fix a sleeping function called from invalid context bug by moving allocation
to the callers prior to taking the policy rdlock.

Signed-off-by:  Stephen Smalley <sds at tycho.nsa.gov>

---

 security/selinux/ss/services.c |   70 +++++++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 30 deletions(-)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index b86ac9d..40255f6 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -730,15 +730,16 @@ int security_sid_to_context_force(u32 sid, char **scontext, u32 *scontext_len)
 	return security_sid_to_context_core(sid, scontext, scontext_len, 1);
 }
 
+/*
+ * Caveat:  Mutates scontext.
+ */
 static int string_to_context_struct(struct policydb *pol,
 				    struct sidtab *sidtabp,
-				    const char *scontext,
+				    char *scontext,
 				    u32 scontext_len,
 				    struct context *ctx,
-				    u32 def_sid,
-				    gfp_t gfp_flags)
+				    u32 def_sid)
 {
-	char *scontext2 = NULL;
 	struct role_datum *role;
 	struct type_datum *typdatum;
 	struct user_datum *usrdatum;
@@ -747,19 +748,10 @@ static int string_to_context_struct(struct policydb *pol,
 
 	context_init(ctx);
 
-	/* Copy the string so that we can modify the copy as we parse it. */
-	scontext2 = kmalloc(scontext_len+1, gfp_flags);
-	if (!scontext2) {
-		rc = -ENOMEM;
-		goto out;
-	}
-	memcpy(scontext2, scontext, scontext_len);
-	scontext2[scontext_len] = 0;
-
 	/* Parse the security context. */
 
 	rc = -EINVAL;
-	scontextp = (char *) scontext2;
+	scontextp = (char *) scontext;
 
 	/* Extract the user. */
 	p = scontextp;
@@ -809,7 +801,7 @@ static int string_to_context_struct(struct policydb *pol,
 	if (rc)
 		goto out;
 
-	if ((p - scontext2) < scontext_len) {
+	if ((p - scontext) < scontext_len) {
 		rc = -EINVAL;
 		goto out;
 	}
@@ -822,7 +814,6 @@ static int string_to_context_struct(struct policydb *pol,
 	}
 	rc = 0;
 out:
-	kfree(scontext2);
 	return rc;
 }
 
@@ -830,6 +821,7 @@ static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
 					u32 *sid, u32 def_sid, gfp_t gfp_flags,
 					int force)
 {
+	char *scontext2, *str = NULL;
 	struct context context;
 	int rc = 0;
 
@@ -839,27 +831,38 @@ static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
 		for (i = 1; i < SECINITSID_NUM; i++) {
 			if (!strcmp(initial_sid_to_string[i], scontext)) {
 				*sid = i;
-				goto out;
+				return 0;
 			}
 		}
 		*sid = SECINITSID_KERNEL;
-		goto out;
+		return 0;
 	}
 	*sid = SECSID_NULL;
 
+	/* Copy the string so that we can modify the copy as we parse it. */
+	scontext2 = kmalloc(scontext_len+1, gfp_flags);
+	if (!scontext2)
+		return -ENOMEM;
+	memcpy(scontext2, scontext, scontext_len);
+	scontext2[scontext_len] = 0;
+
+	if (force) {
+		/* Save another copy for storing in uninterpreted form */
+		str = kstrdup(scontext2, gfp_flags);
+		if (!str) {
+			kfree(scontext2);
+			return -ENOMEM;
+		}
+	}
+
 	POLICY_RDLOCK;
 	rc = string_to_context_struct(&policydb, &sidtab,
-				      scontext, scontext_len,
-				      &context, def_sid, gfp_flags);
+				      scontext2, scontext_len,
+				      &context, def_sid);
 	if (rc == -EINVAL && force) {
-		context.str = kmalloc(scontext_len+1, gfp_flags);
-		if (!context.str) {
-			rc = -ENOMEM;
-			goto out;
-		}
-		memcpy(context.str, scontext, scontext_len);
-		context.str[scontext_len] = 0;
+		context.str = str;
 		context.len = scontext_len;
+		str = NULL;
 	} else if (rc)
 		goto out;
 	rc = sidtab_context_to_sid(&sidtab, &context, sid);
@@ -867,6 +870,8 @@ static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
 		context_destroy(&context);
 out:
 	POLICY_RDUNLOCK;
+	kfree(scontext2);
+	kfree(str);
 	return rc;
 }
 
@@ -1339,9 +1344,14 @@ static int convert_context(u32 key,
 
 	if (c->str) {
 		struct context ctx;
-		rc = string_to_context_struct(args->newp, NULL, c->str,
-					      c->len, &ctx, SECSID_NULL,
-					      GFP_KERNEL);
+		s = kstrdup(c->str, GFP_KERNEL);
+		if (!s) {
+			rc = -ENOMEM;
+			goto out;
+		}
+		rc = string_to_context_struct(args->newp, NULL, s,
+					      c->len, &ctx, SECSID_NULL);
+		kfree(s);
 		if (!rc) {
 			printk(KERN_INFO
 		       "SELinux:  Context %s became valid (mapped).\n",

-- 
Stephen Smalley
National Security Agency


linux-2.6-selinux-generic-ioctl.patch:

--- NEW FILE linux-2.6-selinux-generic-ioctl.patch ---
This is an RFC for changing the SELinux ioctl checking.  There have been
prior discussions and patches for this in the past, but they've never
been brought to conclusion.  This does yield a change in behavior, but
I'm hoping to avoid having to introduce yet another policy capability or
compatibility knob for it unless we find that it actually yields
breakage with existing policies.  I was able to boot and login with this
patch applied without any denials, both as an unconfined_u user and as a
user_u user.  

A few comments on the patch:
1) It completely removes all knowledge of individual ioctl commands.
Alternatively, we could retain specific knowledge of the vfs layer
generic ioctl commands while dropping the specific driver and filesystem
ioctl commands (which represent a layering/encapsulation violation
presently).  But I'm not sure we want to deal with tracking any
individual ioctls vs. just using the IOC_DIR for everything.

2) It uses read vs. write permissions rather than getattr vs. setattr
permissions.  With the prior patches, we found that mapping IOC_WRITE to
SETATTR caused real breakage with existing policies as we tend to be
stingy with setattr permission in policy to specifically limit
chmod/chown and friends, and not everything marked with IOC_WRITE truly
corresponds to a SETATTR.  If we feel we need SETATTR checking on
specific ioctl commands, then I think we need to introduce
security_inode_setattr() hook calls in the corresponding driver and/or
filesystem code rather than trying to catch them here since we are
otherwise violating layering.

3) It removes ext2-specific handling that is a legacy of the original
SELinux kernel patches that was never updated to cover ext3 ioctls.  Yet
another reason for doing it the general way instead.

4) It removes the special checking of KDSKBENT/KDSKBSENT that was
introduced into SELinux a long time ago.  I believe that checking
CAP_SYS_TTY_CONFIG was added to the vt code (see
drivers/char/vt_ioctl.c) a while back for those ioctls, such that the
SELinux check is now redundant.  Also another example of a layering
violation.

5) If the ioctl command access mode bits are clear, then it still falls
through to checking the IOCTL permission. So all operations remain
mediated.

Review and wider testing requested.

---<snip>---

Simplify and improve the robustness of the SELinux ioctl checking by 
using the "access mode" bits of the ioctl command to determine the 
permission check rather than dealing with individual command values.
This removes any knowledge of specific ioctl commands from SELinux
and follows the same guidance we gave to Smack earlier.

---

 security/selinux/hooks.c |   49 +++++++----------------------------------------
 1 file changed, 8 insertions(+), 41 deletions(-)

--- linux-2.6.25.x86_64/security/selinux/hooks.c.pre.ioctl	2008-05-14 15:47:10.000000000 -0400
+++ linux-2.6.25.x86_64/security/selinux/hooks.c	2008-05-14 15:50:50.000000000 -0400
@@ -41,9 +41,7 @@
 #include <linux/file.h>
 #include <linux/namei.h>
 #include <linux/mount.h>
-#include <linux/ext2_fs.h>
 #include <linux/proc_fs.h>
-#include <linux/kd.h>
 #include <linux/netfilter_ipv4.h>
 #include <linux/netfilter_ipv6.h>
 #include <linux/tty.h>
@@ -2821,47 +2819,16 @@ static void selinux_file_free_security(s
 static int selinux_file_ioctl(struct file *file, unsigned int cmd,
 			      unsigned long arg)
 {
-	int error = 0;
-
-	switch (cmd) {
-		case FIONREAD:
-		/* fall through */
-		case FIBMAP:
-		/* fall through */
-		case FIGETBSZ:
-		/* fall through */
-		case EXT2_IOC_GETFLAGS:
-		/* fall through */
-		case EXT2_IOC_GETVERSION:
-			error = file_has_perm(current, file, FILE__GETATTR);
-			break;
-
-		case EXT2_IOC_SETFLAGS:
-		/* fall through */
-		case EXT2_IOC_SETVERSION:
-			error = file_has_perm(current, file, FILE__SETATTR);
-			break;
-
-		/* sys_ioctl() checks */
-		case FIONBIO:
-		/* fall through */
-		case FIOASYNC:
-			error = file_has_perm(current, file, 0);
-			break;
-
-	        case KDSKBENT:
-	        case KDSKBSENT:
-			error = task_has_capability(current,CAP_SYS_TTY_CONFIG);
-			break;
+	u32 av = 0;
 
-		/* default case assumes that the command will go
-		 * to the file's ioctl() function.
-		 */
-		default:
-			error = file_has_perm(current, file, FILE__IOCTL);
+	if (_IOC_DIR(cmd) & _IOC_WRITE)
+		av |= FILE__WRITE;
+	if (_IOC_DIR(cmd) & _IOC_READ)
+		av |= FILE__READ;
+	if (!av)
+		av = FILE__IOCTL;
 
-	}
-	return error;
+	return file_has_perm(current, file, av);
 }
 
 static int file_map_prot_check(struct file *file, unsigned long prot, int shared)


Index: kernel.spec
===================================================================
RCS file: /cvs/pkgs/rpms/kernel/devel/kernel.spec,v
retrieving revision 1.631
retrieving revision 1.632
diff -u -r1.631 -r1.632
--- kernel.spec	12 May 2008 15:11:04 -0000	1.631
+++ kernel.spec	14 May 2008 20:13:11 -0000	1.632
@@ -622,6 +622,8 @@
 
 # SELinux patches, will go upstream in .27
 Patch800: linux-2.6-selinux-deffered-context-mapping.patch
+Patch801: linux-2.6-selinux-deffered-context-mapping-no-sleep.patch
+Patch802: linux-2.6-selinux-generic-ioctl.patch
 #
 
 Patch1101: linux-2.6-default-mmf_dump_elf_headers.patch
@@ -1146,6 +1148,8 @@
 
 # Allow selinux to defer validation of contexts, aka: rpm can write illegal labels
 ApplyPatch linux-2.6-selinux-deffered-context-mapping.patch
+ApplyPatch linux-2.6-selinux-deffered-context-mapping-no-sleep.patch
+ApplyPatch linux-2.6-selinux-generic-ioctl.patch
 
 # wireless patches headed for 2.6.26
 ApplyPatch linux-2.6-wireless.patch
@@ -1796,6 +1800,10 @@
 %kernel_variant_files -a /%{image_install_path}/xen*-%{KVERREL}.xen -e /etc/ld.so.conf.d/kernelcap-%{KVERREL}.xen.conf %{with_xen} xen
 
 %changelog
+* Wed May 14 2008 Eric Paris <eparis at redhat.com>
+- fix may sleep in allocation for previous deffered context patch
+- replace selinux specific knowledge of ioctls with a generic ioctl implementation
+
 * Mon May 12 2008 Kyle McMartin <kmcmartin at redhat.com>
 - Linux 2.6.25.3