[v4,4/3] LSM: Improve context interface for proc attrs

Commit Message

Casey Schaufler June 24, 2016, 4:38 p.m. UTC

Subject: [PATCH v4 4/3] LSM: Improve context interface for proc attrs

Replace kzalloc ... sprintf with kasprintf in the
"context" procfs attr code.

Signed-off-by: Casey Schaufler <[e-mail]>

---
 security/apparmor/lsm.c    | 47 +++++++++++++++++++++++-----------------------
 security/selinux/hooks.c   |  4 +---
 security/smack/smack_lsm.c |  5 +----
 3 files changed, 26 insertions(+), 30 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Kees Cook June 24, 2016, 5:48 p.m. UTC | #1

On Fri, Jun 24, 2016 at 9:38 AM, Casey Schaufler <[e-mail]> wrote:
> Subject: [PATCH v4 4/3] LSM: Improve context interface for proc attrs
>
> Replace kzalloc ... sprintf with kasprintf in the
> "context" procfs attr code.
>
> Signed-off-by: Casey Schaufler <[e-mail]>

Great! For this and the other 3:

Acked-by: Kees Cook <[e-mail]>

-Kees

Patch

diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 3790a7d..5cac15f 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -476,6 +476,8 @@  static int apparmor_getprocattr(struct task_struct *task, char *name,
 	const struct cred *cred = get_task_cred(task);
 	struct aa_task_cxt *cxt = cred_cxt(cred);
 	struct aa_profile *profile = NULL;
+	char *vp;
+	char *np;
 
 	if (strcmp(name, "current") == 0)
 		profile = aa_get_newest_profile(cxt->profile);
@@ -488,30 +490,29 @@  static int apparmor_getprocattr(struct task_struct *task, char *name,
 	else
 		error = -EINVAL;
 
-	if (profile) {
-		if (strcmp(name, "context") == 0) {
-			char *vp;
-			char *np;
-
-			error = aa_getprocattr(profile, &vp);
-			if (error > 0) {
-				error += 12;
-				*value = kzalloc(error, GFP_KERNEL);
-				if (*value == NULL)
-					error = -ENOMEM;
-				else {
-					sprintf(*value, "apparmor='%s'", vp);
-					np = strchr(*value, '\n');
-					if (np != NULL) {
-						np[0] = '\'';
-						np[1] = '\0';
-					}
-				}
-			}
-		} else
-			error = aa_getprocattr(profile, value);
-	}
+	if (profile == NULL)
+		goto put_out;
+
+	error = aa_getprocattr(profile, &vp);
+	if (error < 0)
+		goto put_out;
+
+	if (strcmp(name, "context") == 0) {
+		*value = kasprintf(GFP_KERNEL, "apparmor='%s'", vp);
+		if (*value == NULL) {
+			error = -ENOMEM;
+			goto put_out;
+		}
+		np = strchr(*value, '\n');
+		if (np != NULL) {
+			np[0] = '\'';
+			np[1] = '\0';
+		}
+		error = strlen(*value);
+	} else
+		*value = vp;
 
+put_out:
 	aa_put_profile(profile);
 	put_cred(cred);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 3a21c2b..6397721 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5737,11 +5737,9 @@  static int selinux_getprocattr(struct task_struct *p,
 
 		error = security_sid_to_context(sid, &vp, &len);
 		if (!error) {
-			*value = kzalloc(len + 10, GFP_KERNEL);
+			*value = kasprintf(GFP_KERNEL, "selinux='%s'", vp);
 			if (*value == NULL)
 				error = -ENOMEM;
-			else
-				sprintf(*value, "selinux='%s'", vp);
 		}
 	}
 
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index d2d8624..92e66f8 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3574,18 +3574,15 @@  static int smack_getprocattr(struct task_struct *p, char *name, char **value)
 {
 	struct smack_known *skp = smk_of_task_struct(p);
 	char *cp;
-	int slen;
 
 	if (strcmp(name, "current") == 0) {
 		cp = kstrdup(skp->smk_known, GFP_KERNEL);
 		if (cp == NULL)
 			return -ENOMEM;
 	} else if (strcmp(name, "context") == 0) {
-		slen = strlen(skp->smk_known) + 9;
-		cp = kzalloc(slen, GFP_KERNEL);
+		cp = kasprintf(GFP_KERNEL, "smack='%s'", skp->smk_known);
 		if (cp == NULL)
 			return -ENOMEM;
-		sprintf(cp, "smack='%s'", skp->smk_known);
 	} else
 		return -EINVAL;