Help!

[PATCH] sysfs: Don't leak secdata when a sysfs_dirent is f..

 
  

Post new topic   General Reply to Topic (not reply to a specific post)    Forums Home -> Kernel RSS
Next:  Accepted g2clib 1.1.9-1 (source i386)  
Author Message
Eric W. Biederman
External


Since: May 19, 2006
Posts: 1134



PostPosted: Wed Nov 04, 2009 7:10 am    Post subject: [PATCH] sysfs: Don't leak secdata when a sysfs_dirent is freed.
Archived from groups: linux>kernel (more info?)

While refreshing my sysfs patches I noticed a leak in the secdata
implementation. We don't free the secdata when we free the
sysfs dirent.

This is a bug in 2.6.32-rc5 that we really should close.

Signed-off-by: Eric W. Biederman <ebiederm.RemoveThis@aristanetworks.com>
---

fs/sysfs/dir.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 5fad489..e020183 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -21,6 +21,7 @@
#include <linux/completion.h>
#include <linux/mutex.h>
#include <linux/slab.h>
+#include <linux/security.h>
#include "sysfs.h"

DEFINE_MUTEX(sysfs_mutex);
@@ -285,6 +286,9 @@ void release_sysfs_dirent(struct sysfs_dirent * sd)
sysfs_put(sd->s_symlink.target_sd);
if (sysfs_type(sd) & SYSFS_COPY_NAME)
kfree(sd->s_name);
+ if (sd->s_iattr && sd->s_iattr->ia_secdata)
+ security_release_secctx(sd->s_iattr->ia_secdata,
+ sd->s_iattr->ia_secdata_len);
kfree(sd->s_iattr);
sysfs_free_ino(sd->s_ino);
kmem_cache_free(sysfs_dir_cachep, sd);
--
1.6.5.2.143.g8cc62

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Serge E. Hallyn
External


Since: May 18, 2006
Posts: 335



PostPosted: Wed Nov 04, 2009 10:10 am    Post subject: Re: [PATCH] sysfs: Don't leak secdata when a sysfs_dirent is freed. [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

Quoting Eric W. Biederman (ebiederm@xmission.com):
>
> While refreshing my sysfs patches I noticed a leak in the secdata
> implementation. We don't free the secdata when we free the
> sysfs dirent.
>
> This is a bug in 2.6.32-rc5 that we really should close.
>
> Signed-off-by: Eric W. Biederman <ebiederm DeleteThis @aristanetworks.com>

I'm surprised no kmemleak or anything has found this - but I guess
not many sites hand-set custom selinux contexts on sysfs?

Acked-by: Serge Hallyn <serue DeleteThis @us.ibm.com>

> ---
>
> fs/sysfs/dir.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
> index 5fad489..e020183 100644
> --- a/fs/sysfs/dir.c
> +++ b/fs/sysfs/dir.c
> @@ -21,6 +21,7 @@
> #include <linux/completion.h>
> #include <linux/mutex.h>
> #include <linux/slab.h>
> +#include <linux/security.h>
> #include "sysfs.h"
>
> DEFINE_MUTEX(sysfs_mutex);
> @@ -285,6 +286,9 @@ void release_sysfs_dirent(struct sysfs_dirent * sd)
> sysfs_put(sd->s_symlink.target_sd);
> if (sysfs_type(sd) & SYSFS_COPY_NAME)
> kfree(sd->s_name);
> + if (sd->s_iattr && sd->s_iattr->ia_secdata)
> + security_release_secctx(sd->s_iattr->ia_secdata,
> + sd->s_iattr->ia_secdata_len);
> kfree(sd->s_iattr);
> sysfs_free_ino(sd->s_ino);
> kmem_cache_free(sysfs_dir_cachep, sd);
> --
> 1.6.5.2.143.g8cc62
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo DeleteThis @vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Eric W. Biederman
External


Since: May 19, 2006
Posts: 1134



PostPosted: Wed Nov 04, 2009 10:10 am    Post subject: Re: [PATCH] sysfs: Don't leak secdata when a sysfs_dirent is freed. [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

"Serge E. Hallyn" <serue RemoveThis @us.ibm.com> writes:

> Quoting Eric W. Biederman (ebiederm@xmission.com):
>>
>> While refreshing my sysfs patches I noticed a leak in the secdata
>> implementation. We don't free the secdata when we free the
>> sysfs dirent.
>>
>> This is a bug in 2.6.32-rc5 that we really should close.
>>
>> Signed-off-by: Eric W. Biederman <ebiederm RemoveThis @aristanetworks.com>
>
> I'm surprised no kmemleak or anything has found this - but I guess
> not many sites hand-set custom selinux contexts on sysfs?

I don't expect so since you couldn't customize it in 2.6.31.
This is brand new code, and this corner case was just overlooked
in the review.

Eric


> Acked-by: Serge Hallyn <serue RemoveThis @us.ibm.com>
>
>> ---
>>
>> fs/sysfs/dir.c | 4 ++++
>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
>> index 5fad489..e020183 100644
>> --- a/fs/sysfs/dir.c
>> +++ b/fs/sysfs/dir.c
>> @@ -21,6 +21,7 @@
>> #include <linux/completion.h>
>> #include <linux/mutex.h>
>> #include <linux/slab.h>
>> +#include <linux/security.h>
>> #include "sysfs.h"
>>
>> DEFINE_MUTEX(sysfs_mutex);
>> @@ -285,6 +286,9 @@ void release_sysfs_dirent(struct sysfs_dirent * sd)
>> sysfs_put(sd->s_symlink.target_sd);
>> if (sysfs_type(sd) & SYSFS_COPY_NAME)
>> kfree(sd->s_name);
>> + if (sd->s_iattr && sd->s_iattr->ia_secdata)
>> + security_release_secctx(sd->s_iattr->ia_secdata,
>> + sd->s_iattr->ia_secdata_len);
>> kfree(sd->s_iattr);
>> sysfs_free_ino(sd->s_ino);
>> kmem_cache_free(sysfs_dir_cachep, sd);
>> --
>> 1.6.5.2.143.g8cc62
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
>> the body of a message to majordomo RemoveThis @vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
James Morris
External


Since: May 16, 2006
Posts: 193



PostPosted: Thu Nov 05, 2009 7:10 am    Post subject: [GIT] sysfs: Don't leak secdata when a sysfs_dirent is freed. [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

Hi Linus,

This fixes a memory leak introduced in 2.6.32-rc5, please pull.


The following changes since commit 91d3f9bacdb4950d2f79fe2ba296aa249f60d06c:
Linus Torvalds (1):
Merge branch 'for-linus' of git://git.kernel.org/.../anholt/drm-intel

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6 for-linus

Eric W. Biederman (1):
sysfs: Don't leak secdata when a sysfs_dirent is freed.

fs/sysfs/dir.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)


--
James Morris
<jmorris.DeleteThis@namei.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Display posts from previous:   
Post new topic   General Reply to Topic (not reply to a specific post)    Forums Home -> Kernel All times are: Eastern Time (US & Canada) (change)
Page 1 of 1

 
You can post new topics in this forum
You can reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum