linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Smack: setprocattr memory leak fix
@ 2012-08-22 18:44 Casey Schaufler
  2012-08-23 12:48 ` David Howells
  2012-08-23 16:51 ` Casey Schaufler
  0 siblings, 2 replies; 5+ messages in thread
From: Casey Schaufler @ 2012-08-22 18:44 UTC (permalink / raw)
  To: LKLM, LSM, Casey Schaufler, David Howells

From: Casey Schaufler <casey@schaufler-ca.com>
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 <casey@schaufler-ca.com>

---

 security/smack/smack_lsm.c |   14 ++------------
 1 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index ce9273a..2874c73 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -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;
 }


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] Smack: setprocattr memory leak fix
  2012-08-22 18:44 [PATCH] Smack: setprocattr memory leak fix Casey Schaufler
@ 2012-08-23 12:48 ` David Howells
  2012-08-23 15:58   ` Casey Schaufler
  2012-08-23 16:51 ` Casey Schaufler
  1 sibling, 1 reply; 5+ messages in thread
From: David Howells @ 2012-08-23 12:48 UTC (permalink / raw)
  To: Casey Schaufler; +Cc: dhowells, LKLM, LSM

Casey Schaufler <casey@schaufler-ca.com> wrote:

> From: Casey Schaufler <casey@schaufler-ca.com>
> 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 <casey@schaufler-ca.com>

Acked-by: David Howells <dhowells@redhat.com>

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Smack: setprocattr memory leak fix
  2012-08-23 12:48 ` David Howells
@ 2012-08-23 15:58   ` Casey Schaufler
  0 siblings, 0 replies; 5+ messages in thread
From: Casey Schaufler @ 2012-08-23 15:58 UTC (permalink / raw)
  To: David Howells; +Cc: LKLM, LSM, Casey Schaufler

On 8/23/2012 5:48 AM, David Howells wrote:
> Casey Schaufler <casey@schaufler-ca.com> wrote:
>
>> From: Casey Schaufler <casey@schaufler-ca.com>
>> 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 <casey@schaufler-ca.com>
> Acked-by: David Howells <dhowells@redhat.com>

Thanks for the review.

> 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.

>
>   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
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Smack: setprocattr memory leak fix
  2012-08-22 18:44 [PATCH] Smack: setprocattr memory leak fix Casey Schaufler
  2012-08-23 12:48 ` David Howells
@ 2012-08-23 16:51 ` Casey Schaufler
  1 sibling, 0 replies; 5+ messages in thread
From: Casey Schaufler @ 2012-08-23 16:51 UTC (permalink / raw)
  To: LSM; +Cc: Casey Schaufler, LKLM, David Howells

On 8/22/2012 11:44 AM, Casey Schaufler wrote:

> From: Casey Schaufler <casey@schaufler-ca.com>
> 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 <casey@schaufler-ca.com>

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

 

>
> ---
>
>  security/smack/smack_lsm.c |   14 ++------------
>  1 files changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index ce9273a..2874c73 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] Smack: setprocattr memory leak fix
@ 2012-08-22 21:49 Casey Schaufler
  0 siblings, 0 replies; 5+ messages in thread
From: Casey Schaufler @ 2012-08-22 21:49 UTC (permalink / raw)
  To: LKLM, LSM, Casey Schaufler, David Howells

From: Casey Schaufler <casey@schaufler-ca.com>
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 <casey@schaufler-ca.com>

---

 security/smack/smack_lsm.c |   14 ++------------
 1 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index ce9273a..2874c73 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -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;
 }


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-08-23 16:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-22 18:44 [PATCH] Smack: setprocattr memory leak fix Casey Schaufler
2012-08-23 12:48 ` David Howells
2012-08-23 15:58   ` Casey Schaufler
2012-08-23 16:51 ` Casey Schaufler
2012-08-22 21:49 Casey Schaufler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).