* [PATCH] Fix memory leak in sysfs_sd_setsecdata().
@ 2012-02-20 22:43 Masami Ichikawa
[not found] ` <m11up2zu03.fsf@fess.ebiederm.org>
0 siblings, 1 reply; 4+ messages in thread
From: Masami Ichikawa @ 2012-02-20 22:43 UTC (permalink / raw)
To: gregkh; +Cc: linux-kernel, Masami Ichikawa
This patch fixies follwing two memory leak patterns that reported by kmemleak.
sysfs_sd_setsecdata() is called during sys_lsetxattr() operation.
It checks sd->s_iattr is NULL or not. Then if it is NULL, it calls
sysfs_init_inode_attrs() to allocate memory.
That code is this.
iattrs = sd->s_iattr;
if (!iattrs)
iattrs = sysfs_init_inode_attrs(sd);
The iattrs recieves sysfs_init_inode_attrs()'s result, but sd->s_iattr
doesn't know the address. so it needs to set correct address to
sd->s_iattr to free memory in other function.
unreferenced object 0xffff880250b73e60 (size 32):
comm "systemd", pid 1, jiffies 4294683888 (age 94.553s)
hex dump (first 32 bytes):
73 79 73 74 65 6d 5f 75 3a 6f 62 6a 65 63 74 5f system_u:object_
72 3a 73 79 73 66 73 5f 74 3a 73 30 00 00 00 00 r:sysfs_t:s0....
backtrace:
[<ffffffff814cb1d0>] kmemleak_alloc+0x73/0x98
[<ffffffff811270ab>] __kmalloc+0x100/0x12c
[<ffffffff8120775a>] context_struct_to_string+0x106/0x210
[<ffffffff81207cc1>] security_sid_to_context_core+0x10b/0x129
[<ffffffff812090ef>] security_sid_to_context+0x10/0x12
[<ffffffff811fb0da>] selinux_inode_getsecurity+0x7d/0xa8
[<ffffffff811fb127>] selinux_inode_getsecctx+0x22/0x2e
[<ffffffff811f4d62>] security_inode_getsecctx+0x16/0x18
[<ffffffff81191dad>] sysfs_setxattr+0x96/0x117
[<ffffffff811542f0>] __vfs_setxattr_noperm+0x73/0xd9
[<ffffffff811543d9>] vfs_setxattr+0x83/0xa1
[<ffffffff811544c6>] setxattr+0xcf/0x101
[<ffffffff81154745>] sys_lsetxattr+0x6a/0x8f
[<ffffffff814efda9>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
unreferenced object 0xffff88024163c5a0 (size 96):
comm "systemd", pid 1, jiffies 4294683888 (age 94.553s)
hex dump (first 32 bytes):
00 00 00 00 ed 41 00 00 00 00 00 00 00 00 00 00 .....A..........
00 00 00 00 00 00 00 00 0c 64 42 4f 00 00 00 00 .........dBO....
backtrace:
[<ffffffff814cb1d0>] kmemleak_alloc+0x73/0x98
[<ffffffff81127402>] kmem_cache_alloc_trace+0xc4/0xee
[<ffffffff81191cbe>] sysfs_init_inode_attrs+0x2a/0x83
[<ffffffff81191dd6>] sysfs_setxattr+0xbf/0x117
[<ffffffff811542f0>] __vfs_setxattr_noperm+0x73/0xd9
[<ffffffff811543d9>] vfs_setxattr+0x83/0xa1
[<ffffffff811544c6>] setxattr+0xcf/0x101
[<ffffffff81154745>] sys_lsetxattr+0x6a/0x8f
[<ffffffff814efda9>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
`
Signed-off-by: Masami Ichikawa <masami256@gmail.com>
---
fs/sysfs/inode.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index 85eb816..feb2d69 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -136,12 +136,13 @@ static int sysfs_sd_setsecdata(struct sysfs_dirent *sd, void **secdata, u32 *sec
void *old_secdata;
size_t old_secdata_len;
- iattrs = sd->s_iattr;
- if (!iattrs)
- iattrs = sysfs_init_inode_attrs(sd);
- if (!iattrs)
- return -ENOMEM;
+ if (!sd->s_iattr) {
+ sd->s_iattr = sysfs_init_inode_attrs(sd);
+ if (!sd->s_iattr)
+ return -ENOMEM;
+ }
+ iattrs = sd->s_iattr;
old_secdata = iattrs->ia_secdata;
old_secdata_len = iattrs->ia_secdata_len;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix memory leak in sysfs_sd_setsecdata().
[not found] ` <m11up2zu03.fsf@fess.ebiederm.org>
@ 2012-03-08 21:14 ` Greg KH
2012-03-08 21:43 ` Eric W. Biederman
0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2012-03-08 21:14 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: Masami Ichikawa, linux-kernel, stable
On Thu, Mar 08, 2012 at 01:02:20PM -0800, Eric W. Biederman wrote:
> Masami Ichikawa <masami256@gmail.com> writes:
>
> > This patch fixies follwing two memory leak patterns that reported by kmemleak.
> > sysfs_sd_setsecdata() is called during sys_lsetxattr() operation.
> > It checks sd->s_iattr is NULL or not. Then if it is NULL, it calls
> > sysfs_init_inode_attrs() to allocate memory.
> > That code is this.
>
> I don't know how you count two memory leaks. But there is definitely a
> leak here sd->s_iattr is allocated and then never assigned. It looks
> like I introduced that leak when I re-factored the code to protect
> the code with sysfs_mutex at the end of 2009.
>
> I am surprise the securlity label crowd has not been screaming about
> selinux protection not working on sysfs for the last two years.
>
> I have reviewed the code and the fix looks obvious and correct.
>
> Greg can you pick this up?
I applied it a while ago to my tree already :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix memory leak in sysfs_sd_setsecdata().
2012-03-08 21:14 ` Greg KH
@ 2012-03-08 21:43 ` Eric W. Biederman
2012-03-08 22:33 ` Masami Ichikawa
0 siblings, 1 reply; 4+ messages in thread
From: Eric W. Biederman @ 2012-03-08 21:43 UTC (permalink / raw)
To: Greg KH; +Cc: Masami Ichikawa, linux-kernel, stable
Greg KH <gregkh@linuxfoundation.org> writes:
> On Thu, Mar 08, 2012 at 01:02:20PM -0800, Eric W. Biederman wrote:
>> Masami Ichikawa <masami256@gmail.com> writes:
>>
>> > This patch fixies follwing two memory leak patterns that reported by kmemleak.
>> > sysfs_sd_setsecdata() is called during sys_lsetxattr() operation.
>> > It checks sd->s_iattr is NULL or not. Then if it is NULL, it calls
>> > sysfs_init_inode_attrs() to allocate memory.
>> > That code is this.
>>
>> I don't know how you count two memory leaks. But there is definitely a
>> leak here sd->s_iattr is allocated and then never assigned. It looks
>> like I introduced that leak when I re-factored the code to protect
>> the code with sysfs_mutex at the end of 2009.
>>
>> I am surprise the securlity label crowd has not been screaming about
>> selinux protection not working on sysfs for the last two years.
>>
>> I have reviewed the code and the fix looks obvious and correct.
>>
>> Greg can you pick this up?
>
> I applied it a while ago to my tree already :)
Odd I didn't see it linux-next when I looked an hour or so ago.
As long as it is there and it makes it to stable.
Eric
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix memory leak in sysfs_sd_setsecdata().
2012-03-08 21:43 ` Eric W. Biederman
@ 2012-03-08 22:33 ` Masami Ichikawa
0 siblings, 0 replies; 4+ messages in thread
From: Masami Ichikawa @ 2012-03-08 22:33 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: Greg KH, linux-kernel, stable
Hi,
Eric, thank you for the review.
Greg, thenk you for apply my patch.
On Fri, Mar 9, 2012 at 6:43 AM, Eric W. Biederman <ebiederm@xmission.com> wrote:
> Greg KH <gregkh@linuxfoundation.org> writes:
>
>> On Thu, Mar 08, 2012 at 01:02:20PM -0800, Eric W. Biederman wrote:
>>> Masami Ichikawa <masami256@gmail.com> writes:
>>>
>>> > This patch fixies follwing two memory leak patterns that reported by kmemleak.
>>> > sysfs_sd_setsecdata() is called during sys_lsetxattr() operation.
>>> > It checks sd->s_iattr is NULL or not. Then if it is NULL, it calls
>>> > sysfs_init_inode_attrs() to allocate memory.
>>> > That code is this.
>>>
>>> I don't know how you count two memory leaks. But there is definitely a
>>> leak here sd->s_iattr is allocated and then never assigned. It looks
>>> like I introduced that leak when I re-factored the code to protect
>>> the code with sysfs_mutex at the end of 2009.
>>>
>>> I am surprise the securlity label crowd has not been screaming about
>>> selinux protection not working on sysfs for the last two years.
>>>
>>> I have reviewed the code and the fix looks obvious and correct.
>>>
>>> Greg can you pick this up?
>>
>> I applied it a while ago to my tree already :)
>
> Odd I didn't see it linux-next when I looked an hour or so ago.
>
> As long as it is there and it makes it to stable.
>
> Eric
>
I saw my patch in linux-next tree.
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=93518dd2ebafcc761a8637b2877008cfd748c202
Cheers,
--
Masami Ichikawa
gmail: masami256@gmail.com
Fedora project: masami@fedoraproject.org
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-03-08 22:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-20 22:43 [PATCH] Fix memory leak in sysfs_sd_setsecdata() Masami Ichikawa
[not found] ` <m11up2zu03.fsf@fess.ebiederm.org>
2012-03-08 21:14 ` Greg KH
2012-03-08 21:43 ` Eric W. Biederman
2012-03-08 22:33 ` Masami Ichikawa
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).