mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* BUG memory leakage at ./security/selinux/hooks.c
@ 2009-08-10  8:57 iceberg
  2009-08-10 11:53 ` James Morris
  0 siblings, 1 reply; 6+ messages in thread
From: iceberg @ 2009-08-10  8:57 UTC (permalink / raw)
  To: linux-kernel

        KERNEL_VERSION: 2.6.30.4
        SUBJECT:  in function inode_doinit_with_dentry memory is not released 
on error  path (if rc<0).
        DESCRIBE:
        In ./security/selinux/hooks.c in function inode_doinit_with_dentry:
1. If in the line 1278 we successfully allocate memory and assign it to 
context variablehen
2. if in the line 1284 getxattr returns -ERANGE and
3. if in the line 1288 getxattr returns rc<0
then we go to out_unlock without releasing memory pointed to by context 
variable.

1278                 context = kmalloc(len, GFP_NOFS);
1279                 if (!context) {
1280                         rc = -ENOMEM;
1281                         dput(dentry);
1282                         goto out_unlock;
1283                 }
1284                 rc = inode->i_op->getxattr(dentry, XATTR_NAME_SELINUX,
1285                                            context, len);
1286                 if (rc == -ERANGE) {
1287                         /* Need a larger buffer.  Query for the
right size. */
1288                         rc = inode->i_op->getxattr(dentry,
XATTR_NAME_SELINUX,
1289                                                    NULL, 0);
1290                         if (rc < 0) {
1291                                 dput(dentry);
1292                                 goto out_unlock;
1293                         }
1294                         kfree(context);
1295                         len = rc;
1296                         context = kmalloc(len, GFP_NOFS);



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

* Re: BUG memory leakage at ./security/selinux/hooks.c
  2009-08-10  8:57 BUG memory leakage at ./security/selinux/hooks.c iceberg
@ 2009-08-10 11:53 ` James Morris
  2009-08-10 12:17   ` [PATCH] SELinux: re. " James Morris
  0 siblings, 1 reply; 6+ messages in thread
From: James Morris @ 2009-08-10 11:53 UTC (permalink / raw)
  To: iceberg; +Cc: linux-kernel, selinux, Eric Paris, Stephen Smalley

Thanks for the report.  (Cc'd to the SELinux list).


On Mon, 10 Aug 2009, iceberg wrote:

>         KERNEL_VERSION: 2.6.30.4
>         SUBJECT:  in function inode_doinit_with_dentry memory is not released 
> on error  path (if rc<0).
>         DESCRIBE:
>         In ./security/selinux/hooks.c in function inode_doinit_with_dentry:
> 1. If in the line 1278 we successfully allocate memory and assign it to 
> context variablehen
> 2. if in the line 1284 getxattr returns -ERANGE and
> 3. if in the line 1288 getxattr returns rc<0
> then we go to out_unlock without releasing memory pointed to by context 
> variable.
> 
> 1278                 context = kmalloc(len, GFP_NOFS);
> 1279                 if (!context) {
> 1280                         rc = -ENOMEM;
> 1281                         dput(dentry);
> 1282                         goto out_unlock;
> 1283                 }
> 1284                 rc = inode->i_op->getxattr(dentry, XATTR_NAME_SELINUX,
> 1285                                            context, len);
> 1286                 if (rc == -ERANGE) {
> 1287                         /* Need a larger buffer.  Query for the
> right size. */
> 1288                         rc = inode->i_op->getxattr(dentry,
> XATTR_NAME_SELINUX,
> 1289                                                    NULL, 0);
> 1290                         if (rc < 0) {
> 1291                                 dput(dentry);
> 1292                                 goto out_unlock;
> 1293                         }
> 1294                         kfree(context);
> 1295                         len = rc;
> 1296                         context = kmalloc(len, GFP_NOFS);
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
James Morris
<jmorris@namei.org>

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

* [PATCH] SELinux: re. BUG memory leakage at ./security/selinux/hooks.c
  2009-08-10 11:53 ` James Morris
@ 2009-08-10 12:17   ` James Morris
  2009-08-10 12:24     ` Stephen Smalley
  0 siblings, 1 reply; 6+ messages in thread
From: James Morris @ 2009-08-10 12:17 UTC (permalink / raw)
  To: iceberg; +Cc: linux-kernel, selinux, Eric Paris, Stephen Smalley

Please review.

Author: James Morris <jmorris@namei.org>
Date:   Mon Aug 10 22:00:13 2009 +1000

    SELinux: fix memory leakage in /security/selinux/hooks.c
    
    Fix memory leakage in /security/selinux/hooks.c
    
    The buffer always needs to be freed here; we either error
    out or allocate more memory.
    
    Reported-by: iceberg <strakh@ispras.ru>
    Signed-off-by: James Morris <jmorris@namei.org>

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 15c2a08..1e8cfc4 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1285,6 +1285,8 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
 		rc = inode->i_op->getxattr(dentry, XATTR_NAME_SELINUX,
 					   context, len);
 		if (rc == -ERANGE) {
+			kfree(context);
+
 			/* Need a larger buffer.  Query for the right size. */
 			rc = inode->i_op->getxattr(dentry, XATTR_NAME_SELINUX,
 						   NULL, 0);
@@ -1292,7 +1294,6 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
 				dput(dentry);
 				goto out_unlock;
 			}
-			kfree(context);
 			len = rc;
 			context = kmalloc(len+1, GFP_NOFS);
 			if (!context) {

-- 
James Morris
<jmorris@namei.org>

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

* Re: [PATCH] SELinux: re. BUG memory leakage at ./security/selinux/hooks.c
  2009-08-10 12:17   ` [PATCH] SELinux: re. " James Morris
@ 2009-08-10 12:24     ` Stephen Smalley
  0 siblings, 0 replies; 6+ messages in thread
From: Stephen Smalley @ 2009-08-10 12:24 UTC (permalink / raw)
  To: James Morris; +Cc: iceberg, linux-kernel, selinux, Eric Paris

On Mon, 2009-08-10 at 22:17 +1000, James Morris wrote:
> Please review.
> 
> Author: James Morris <jmorris@namei.org>
> Date:   Mon Aug 10 22:00:13 2009 +1000
> 
>     SELinux: fix memory leakage in /security/selinux/hooks.c
>     
>     Fix memory leakage in /security/selinux/hooks.c
>     
>     The buffer always needs to be freed here; we either error
>     out or allocate more memory.
>     
>     Reported-by: iceberg <strakh@ispras.ru>
>     Signed-off-by: James Morris <jmorris@namei.org>

Acked-by:  Stephen Smalley <sds@tycho.nsa.gov>

> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 15c2a08..1e8cfc4 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1285,6 +1285,8 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
>  		rc = inode->i_op->getxattr(dentry, XATTR_NAME_SELINUX,
>  					   context, len);
>  		if (rc == -ERANGE) {
> +			kfree(context);
> +
>  			/* Need a larger buffer.  Query for the right size. */
>  			rc = inode->i_op->getxattr(dentry, XATTR_NAME_SELINUX,
>  						   NULL, 0);
> @@ -1292,7 +1294,6 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
>  				dput(dentry);
>  				goto out_unlock;
>  			}
> -			kfree(context);
>  			len = rc;
>  			context = kmalloc(len+1, GFP_NOFS);
>  			if (!context) {
> 
-- 
Stephen Smalley
National Security Agency


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

* Re: BUG memory leakage at ./security/selinux/hooks.c
  2009-08-10  8:53 iceberg
@ 2009-08-15  7:17 ` Jeremy Fitzhardinge
  0 siblings, 0 replies; 6+ messages in thread
From: Jeremy Fitzhardinge @ 2009-08-15  7:17 UTC (permalink / raw)
  To: iceberg; +Cc: linux-kernel, James Morris

Added cc:

On 08/10/09 01:53, iceberg wrote:
> ????????KERNEL_VERSION: 2.6.30.4??
>   

Looks like you got some kind of charset conversion problem going on.

> ????????SUBJECT: ?in function inode_doinit_with_dentry memory is not released 
> on error  path (if rc<0).
> ????????DESCRIBE:
> ????????In ./security/selinux/hooks.c in function inode_doinit_with_dentry:??
> 1. If in the line 1278 we successfully allocate memory and assign it to 
> context variablehen 
> 2. if in the line 1284 getxattr returns -ERANGE and 
> 3. if in the line 1288 getxattr returns rc<0 
> then we go to out_unlock without releasing memory pointed to by context 
> variable.
>
> 1278 ????????????????context?=?kmalloc(len,?GFP_NOFS);
> 1279 ????????????????if?(!context)?{
> 1280 ????????????????????????rc?=?-ENOMEM;
> 1281 ????????????????????????dput(dentry);
> 1282 ????????????????????????goto?out_unlock;
> 1283 ????????????????}
> 1284 ????????????????rc?=?inode->i_op->getxattr(dentry,?XATTR_NAME_SELINUX,
> 1285 ???????????????????????????????????????????context,?len);
> 1286 ????????????????if?(rc?==?-ERANGE)?{
> 1287 ????????????????????????/*?Need?a?larger?buffer.??Query?for?the
> right?size.?*/
> 1288 ????????????????????????rc?=?inode->i_op->getxattr(dentry,
> XATTR_NAME_SELINUX,
> 1289 ???????????????????????????????????????????????????NULL,?0);
> 1290 ????????????????????????if?(rc?<?0)?{
> 1291 ????????????????????????????????dput(dentry);
> 1292 ????????????????????????????????goto?out_unlock;
> 1293 ????????????????????????}
> 1294 ????????????????????????kfree(context);
> 1295 ????????????????????????len?=?rc;
> 1296 ????????????????????????context?=?kmalloc(len,?GFP_NOFS);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
>   


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

* BUG memory leakage at ./security/selinux/hooks.c
@ 2009-08-10  8:53 iceberg
  2009-08-15  7:17 ` Jeremy Fitzhardinge
  0 siblings, 1 reply; 6+ messages in thread
From: iceberg @ 2009-08-10  8:53 UTC (permalink / raw)
  To: linux-kernel

????????KERNEL_VERSION: 2.6.30.4??
????????SUBJECT: ?in function inode_doinit_with_dentry memory is not released 
on error  path (if rc<0).
????????DESCRIBE:
????????In ./security/selinux/hooks.c in function inode_doinit_with_dentry:??
1. If in the line 1278 we successfully allocate memory and assign it to 
context variablehen 
2. if in the line 1284 getxattr returns -ERANGE and 
3. if in the line 1288 getxattr returns rc<0 
then we go to out_unlock without releasing memory pointed to by context 
variable.

1278 ????????????????context?=?kmalloc(len,?GFP_NOFS);
1279 ????????????????if?(!context)?{
1280 ????????????????????????rc?=?-ENOMEM;
1281 ????????????????????????dput(dentry);
1282 ????????????????????????goto?out_unlock;
1283 ????????????????}
1284 ????????????????rc?=?inode->i_op->getxattr(dentry,?XATTR_NAME_SELINUX,
1285 ???????????????????????????????????????????context,?len);
1286 ????????????????if?(rc?==?-ERANGE)?{
1287 ????????????????????????/*?Need?a?larger?buffer.??Query?for?the
right?size.?*/
1288 ????????????????????????rc?=?inode->i_op->getxattr(dentry,
XATTR_NAME_SELINUX,
1289 ???????????????????????????????????????????????????NULL,?0);
1290 ????????????????????????if?(rc?<?0)?{
1291 ????????????????????????????????dput(dentry);
1292 ????????????????????????????????goto?out_unlock;
1293 ????????????????????????}
1294 ????????????????????????kfree(context);
1295 ????????????????????????len?=?rc;
1296 ????????????????????????context?=?kmalloc(len,?GFP_NOFS);

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

end of thread, other threads:[~2009-08-15  7:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-10  8:57 BUG memory leakage at ./security/selinux/hooks.c iceberg
2009-08-10 11:53 ` James Morris
2009-08-10 12:17   ` [PATCH] SELinux: re. " James Morris
2009-08-10 12:24     ` Stephen Smalley
  -- strict thread matches above, loose matches on Subject: below --
2009-08-10  8:53 iceberg
2009-08-15  7:17 ` Jeremy Fitzhardinge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome