Skip to contentSkip to conversation

Smack: setprocattr memory leak fix

PATCH Applied Casey Schaufler 2 participants 4 messages Aug 22, 2012 – Aug 23, 2012

Subsystems (2)
Security (LSM) Smack security module
lore
Share & tools
lore

Casey Schaufler

author maintainer
#0 lore raw

PATCH 1/1 Smack: setprocattr memory leak fix

From: Casey Schaufler <[email protected]>
Subject: [PATCH] Smack: setprocattr memory leak fix

The data structure allocations being done in prepare_creds are duplicated in smack_setprocattr. This results in the structure allocated in prepare_creds being orphaned and never freed. The duplicate code is removed from smack_setprocattr.

Targeted for git://git.gitorious.org/smack-next/kernel.git

Signed-off-by Casey Schaufler <[email protected]>

---

 security/smack/smack_lsm.c |   14 ++------------
 1 files changed, 2 insertions(+), 12 deletions(-)
security/smack/smack_lsm.c +2 -12
index ce9273a..2874c73 100644
@@ -2684,9 +2684,7 @@ static int smack_getprocattr(struct task_struct *p, char *name, char **value)
 static int smack_setprocattr(struct task_struct *p, char *name,
 			     void *value, size_t size)
 {
-	int rc;
 	struct task_smack *tsp;
-	struct task_smack *oldtsp;
 	struct cred *new;
 	char *newsmack;
 

@@ -2716,21 +2714,13 @@ static int smack_setprocattr(struct task_struct *p, char *name,
 	if (newsmack == smack_known_web.smk_known)
 		return -EPERM;
 
-	oldtsp = p->cred->security;
 	new = prepare_creds();
 	if (new == NULL)
 		return -ENOMEM;
 
-	tsp = new_task_smack(newsmack, oldtsp->smk_forked, GFP_KERNEL);
-	if (tsp == NULL) {
-		kfree(new);
-		return -ENOMEM;
-	}
-	rc = smk_copy_rules(&tsp->smk_rules, &oldtsp->smk_rules, GFP_KERNEL);
-	if (rc != 0)
-		return rc;
+	tsp = new->security;
+	tsp->smk_task = newsmack;
 
-	new->security = tsp;
 	commit_creds(new);
 	return size;
 }
Acked-by on v1 1/1
#1 lore raw
Quoted 9 lines · Casey Schaufler

From: Casey Schaufler <[email protected]>
Subject: [PATCH] Smack: setprocattr memory leak fix

The data structure allocations being done in prepare_creds are duplicated in smack_setprocattr. This results in the structure allocated in prepare_creds being orphaned and never freed. The duplicate code is removed from smack_setprocattr.

Quoted 3 lines

Targeted for git://git.gitorious.org/smack-next/kernel.git

Signed-off-by Casey Schaufler <[email protected]>
Acked-by David Howells <[email protected]>

Note that I do have a patch to make Smack share rule lists - I've posted it a number of times, but I take it you're not interested.

  5707  06/13 [To:casey@schauf]  [PATCH 1/3] Smack: Constify some pointers<<Mak
  5708  06/13   [To:casey@schauf]  [PATCH 2/3] Smack: Further constification<<M
  5709  06/13   [To:casey@schauf]  [PATCH 3/3] Smack: Use rule list tail sharin

David

author maintainer
#2 lore raw
Quoted 11 lines · David Howells
Quoted 9 lines · Casey Schaufler

From: Casey Schaufler <[email protected]>
Subject: [PATCH] Smack: setprocattr memory leak fix

The data structure allocations being done in prepare_creds are duplicated in smack_setprocattr. This results in the structure allocated in prepare_creds being orphaned and never freed. The duplicate code is removed from smack_setprocattr.

Quoted 4 lines
Quoted 3 lines

Targeted for git://git.gitorious.org/smack-next/kernel.git

Signed-off-by Casey Schaufler <[email protected]>
Acked-by David Howells <[email protected]>

Thanks for the review.

Quoted 2 lines

Note that I do have a patch to make Smack share rule lists - I've posted it a number of times, but I take it you're not interested.

The patch is a big chunk of code and the feature isn't getting used to the extent that the people who requested it seemed certain it would be. I don't see there being value in optimizing the per-task rule lists given the amount of use they get.

Quoted 7 lines
  5707  06/13 [To:casey@schauf]  [PATCH 1/3] Smack: Constify some pointers<<Mak
  5708  06/13   [To:casey@schauf]  [PATCH 2/3] Smack: Further constification<<M
  5709  06/13   [To:casey@schauf]  [PATCH 3/3] Smack: Use rule list tail sharin

David

author maintainer Applied
#3 lore raw
In reply to Casey Schaufler
Quoted 9 lines · Casey Schaufler

From: Casey Schaufler <[email protected]>
Subject: [PATCH] Smack: setprocattr memory leak fix

The data structure allocations being done in prepare_creds are duplicated in smack_setprocattr. This results in the structure allocated in prepare_creds being orphaned and never freed. The duplicate code is removed from smack_setprocattr.

Quoted 3 lines

Targeted for git://git.gitorious.org/smack-next/kernel.git

Signed-off-by Casey Schaufler <[email protected]>

Applied to git://git.gitorious.org/smack-next/kernel.git

Quoted 50 lines

---

 security/smack/smack_lsm.c |   14 ++------------
 1 files changed, 2 insertions(+), 12 deletions(-)
security/smack/smack_lsm.c +2 -13
index ce9273a..2874c73 100644
@@ -2684,9 +2684,7 @@ static int smack_getprocattr(struct task_struct *p, char *name, char **value)
 static int smack_setprocattr(struct task_struct *p, char *name,
 			     void *value, size_t size)
 {
-	int rc;
 	struct task_smack *tsp;
-	struct task_smack *oldtsp;
 	struct cred *new;
 	char *newsmack;
 

@@ -2716,21 +2714,13 @@ static int smack_setprocattr(struct task_struct *p, char *name,
 	if (newsmack == smack_known_web.smk_known)
 		return -EPERM;
 
-	oldtsp = p->cred->security;
 	new = prepare_creds();
 	if (new == NULL)
 		return -ENOMEM;
 
-	tsp = new_task_smack(newsmack, oldtsp->smk_forked, GFP_KERNEL);
-	if (tsp == NULL) {
-		kfree(new);
-		return -ENOMEM;
-	}
-	rc = smk_copy_rules(&tsp->smk_rules, &oldtsp->smk_rules, GFP_KERNEL);
-	if (rc != 0)
-		return rc;
+	tsp = new->security;
+	tsp->smk_task = newsmack;
 
-	new->security = tsp;
 	commit_creds(new);
 	return size;
 }
 
--

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