* [PATCH] nfs: fix memory leak in nfs_get_sb with CONFIG_NFS_V4
@ 2010-04-22 10:56 Xiaotian Feng
2010-04-22 16:09 ` Chuck Lever
0 siblings, 1 reply; 4+ messages in thread
From: Xiaotian Feng @ 2010-04-22 10:56 UTC (permalink / raw)
To: linux-nfs
Cc: linux-kernel, Xiaotian Feng, Trond Myklebust, Chuck Lever,
Benny Halevy, Al Viro, Andy Adamson
With CONFIG_NFS_V4 and data version 4, nfs_get_sb will allocate memory for
export_path in nfs4_validate_text_mount_data, so we need to free it then.
This is addressed in following kmemleak report:
unreferenced object 0xffff88016bf48a50 (size 16):
comm "mount.nfs", pid 22567, jiffies 4651574704 (age 175471.200s)
hex dump (first 16 bytes):
2f 6f 70 74 2f 77 6f 72 6b 00 6b 6b 6b 6b 6b a5 /opt/work.kkkkk.
backtrace:
[<ffffffff814b34f9>] kmemleak_alloc+0x60/0xa7
[<ffffffff81102c76>] kmemleak_alloc_recursive.clone.5+0x1b/0x1d
[<ffffffff811046b3>] __kmalloc_track_caller+0x18f/0x1b7
[<ffffffff810e1b08>] kstrndup+0x37/0x54
[<ffffffffa0336971>] nfs_parse_devname+0x152/0x204 [nfs]
[<ffffffffa0336af3>] nfs4_validate_text_mount_data+0xd0/0xdc [nfs]
[<ffffffffa0338deb>] nfs_get_sb+0x325/0x736 [nfs]
[<ffffffff81113671>] vfs_kern_mount+0xbd/0x17c
[<ffffffff81113798>] do_kern_mount+0x4d/0xed
[<ffffffff81129a87>] do_mount+0x787/0x7fe
[<ffffffff81129b86>] sys_mount+0x88/0xc2
[<ffffffff81009b42>] system_call_fastpath+0x16/0x1b
Signed-off-by: Xiaotian Feng <dfeng@redhat.com>
Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Chuck Lever <chuck.lever@oracle.com>
Cc: Benny Halevy <bhalevy@panasas.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Andy Adamson <andros@netapp.com>
---
fs/nfs/super.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index e016372..44e6567 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2187,6 +2187,7 @@ static int nfs_get_sb(struct file_system_type *fs_type,
if (data->version == 4) {
error = nfs4_try_mount(flags, dev_name, data, mnt);
kfree(data->client_address);
+ kfree(data->nfs_server.export_path);
goto out;
}
#endif /* CONFIG_NFS_V4 */
--
1.7.0.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] nfs: fix memory leak in nfs_get_sb with CONFIG_NFS_V4
2010-04-22 10:56 [PATCH] nfs: fix memory leak in nfs_get_sb with CONFIG_NFS_V4 Xiaotian Feng
@ 2010-04-22 16:09 ` Chuck Lever
2010-04-23 2:29 ` Xiaotian Feng
0 siblings, 1 reply; 4+ messages in thread
From: Chuck Lever @ 2010-04-22 16:09 UTC (permalink / raw)
To: Xiaotian Feng
Cc: linux-nfs, linux-kernel, Trond Myklebust, Benny Halevy, Al Viro,
Andy Adamson
On 04/22/2010 06:56 AM, Xiaotian Feng wrote:
> With CONFIG_NFS_V4 and data version 4, nfs_get_sb will allocate memory for
> export_path in nfs4_validate_text_mount_data, so we need to free it then.
> This is addressed in following kmemleak report:
>
> unreferenced object 0xffff88016bf48a50 (size 16):
> comm "mount.nfs", pid 22567, jiffies 4651574704 (age 175471.200s)
> hex dump (first 16 bytes):
> 2f 6f 70 74 2f 77 6f 72 6b 00 6b 6b 6b 6b 6b a5 /opt/work.kkkkk.
> backtrace:
> [<ffffffff814b34f9>] kmemleak_alloc+0x60/0xa7
> [<ffffffff81102c76>] kmemleak_alloc_recursive.clone.5+0x1b/0x1d
> [<ffffffff811046b3>] __kmalloc_track_caller+0x18f/0x1b7
> [<ffffffff810e1b08>] kstrndup+0x37/0x54
> [<ffffffffa0336971>] nfs_parse_devname+0x152/0x204 [nfs]
> [<ffffffffa0336af3>] nfs4_validate_text_mount_data+0xd0/0xdc [nfs]
> [<ffffffffa0338deb>] nfs_get_sb+0x325/0x736 [nfs]
> [<ffffffff81113671>] vfs_kern_mount+0xbd/0x17c
> [<ffffffff81113798>] do_kern_mount+0x4d/0xed
> [<ffffffff81129a87>] do_mount+0x787/0x7fe
> [<ffffffff81129b86>] sys_mount+0x88/0xc2
> [<ffffffff81009b42>] system_call_fastpath+0x16/0x1b
>
> Signed-off-by: Xiaotian Feng<dfeng@redhat.com>
> Cc: Trond Myklebust<Trond.Myklebust@netapp.com>
> Cc: Chuck Lever<chuck.lever@oracle.com>
> Cc: Benny Halevy<bhalevy@panasas.com>
> Cc: Al Viro<viro@ZenIV.linux.org.uk>
> Cc: Andy Adamson<andros@netapp.com>
> ---
> fs/nfs/super.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index e016372..44e6567 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2187,6 +2187,7 @@ static int nfs_get_sb(struct file_system_type *fs_type,
> if (data->version == 4) {
> error = nfs4_try_mount(flags, dev_name, data, mnt);
> kfree(data->client_address);
> + kfree(data->nfs_server.export_path);
nfs4_try_mount's other call site in fs/nfs/super.c also frees
data->nfs_server.hostname and data->fscache_uniq. Does that need to
happen here too?
> goto out;
> }
> #endif /* CONFIG_NFS_V4 */
--
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nfs: fix memory leak in nfs_get_sb with CONFIG_NFS_V4
2010-04-22 16:09 ` Chuck Lever
@ 2010-04-23 2:29 ` Xiaotian Feng
2010-04-23 15:24 ` Chuck Lever
0 siblings, 1 reply; 4+ messages in thread
From: Xiaotian Feng @ 2010-04-23 2:29 UTC (permalink / raw)
To: Chuck Lever
Cc: linux-nfs, linux-kernel, Trond Myklebust, Benny Halevy, Al Viro,
Andy Adamson
On 04/23/2010 12:09 AM, Chuck Lever wrote:
> On 04/22/2010 06:56 AM, Xiaotian Feng wrote:
>> With CONFIG_NFS_V4 and data version 4, nfs_get_sb will allocate
>> memory for
>> export_path in nfs4_validate_text_mount_data, so we need to free it
>> then.
>> This is addressed in following kmemleak report:
>>
>> unreferenced object 0xffff88016bf48a50 (size 16):
>> comm "mount.nfs", pid 22567, jiffies 4651574704 (age 175471.200s)
>> hex dump (first 16 bytes):
>> 2f 6f 70 74 2f 77 6f 72 6b 00 6b 6b 6b 6b 6b a5 /opt/work.kkkkk.
>> backtrace:
>> [<ffffffff814b34f9>] kmemleak_alloc+0x60/0xa7
>> [<ffffffff81102c76>] kmemleak_alloc_recursive.clone.5+0x1b/0x1d
>> [<ffffffff811046b3>] __kmalloc_track_caller+0x18f/0x1b7
>> [<ffffffff810e1b08>] kstrndup+0x37/0x54
>> [<ffffffffa0336971>] nfs_parse_devname+0x152/0x204 [nfs]
>> [<ffffffffa0336af3>] nfs4_validate_text_mount_data+0xd0/0xdc [nfs]
>> [<ffffffffa0338deb>] nfs_get_sb+0x325/0x736 [nfs]
>> [<ffffffff81113671>] vfs_kern_mount+0xbd/0x17c
>> [<ffffffff81113798>] do_kern_mount+0x4d/0xed
>> [<ffffffff81129a87>] do_mount+0x787/0x7fe
>> [<ffffffff81129b86>] sys_mount+0x88/0xc2
>> [<ffffffff81009b42>] system_call_fastpath+0x16/0x1b
>>
>> Signed-off-by: Xiaotian Feng<dfeng@redhat.com>
>> Cc: Trond Myklebust<Trond.Myklebust@netapp.com>
>> Cc: Chuck Lever<chuck.lever@oracle.com>
>> Cc: Benny Halevy<bhalevy@panasas.com>
>> Cc: Al Viro<viro@ZenIV.linux.org.uk>
>> Cc: Andy Adamson<andros@netapp.com>
>> ---
>> fs/nfs/super.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index e016372..44e6567 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -2187,6 +2187,7 @@ static int nfs_get_sb(struct file_system_type
>> *fs_type,
>> if (data->version == 4) {
>> error = nfs4_try_mount(flags, dev_name, data, mnt);
>> kfree(data->client_address);
>> + kfree(data->nfs_server.export_path);
>
> nfs4_try_mount's other call site in fs/nfs/super.c also frees
> data->nfs_server.hostname and data->fscache_uniq. Does that need to
> happen here too?
I don't think we need to free data->nfs_server.hostname and
data->fscache_uniq, they were freed in the out label ;-)
>
>> goto out;
>> }
out:
kfree(data->nfs_server.hostname);
kfree(data->mount_server.hostname);
kfree(data->fscache_uniq);
security_free_mnt_opts(&data->lsm_opts);
>> #endif /* CONFIG_NFS_V4 */
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nfs: fix memory leak in nfs_get_sb with CONFIG_NFS_V4
2010-04-23 2:29 ` Xiaotian Feng
@ 2010-04-23 15:24 ` Chuck Lever
0 siblings, 0 replies; 4+ messages in thread
From: Chuck Lever @ 2010-04-23 15:24 UTC (permalink / raw)
To: Xiaotian Feng
Cc: linux-nfs, linux-kernel, Trond Myklebust, Benny Halevy, Al Viro,
Andy Adamson
On 04/22/2010 10:29 PM, Xiaotian Feng wrote:
> On 04/23/2010 12:09 AM, Chuck Lever wrote:
>> On 04/22/2010 06:56 AM, Xiaotian Feng wrote:
>>> With CONFIG_NFS_V4 and data version 4, nfs_get_sb will allocate
>>> memory for
>>> export_path in nfs4_validate_text_mount_data, so we need to free it
>>> then.
>>> This is addressed in following kmemleak report:
>>>
>>> unreferenced object 0xffff88016bf48a50 (size 16):
>>> comm "mount.nfs", pid 22567, jiffies 4651574704 (age 175471.200s)
>>> hex dump (first 16 bytes):
>>> 2f 6f 70 74 2f 77 6f 72 6b 00 6b 6b 6b 6b 6b a5 /opt/work.kkkkk.
>>> backtrace:
>>> [<ffffffff814b34f9>] kmemleak_alloc+0x60/0xa7
>>> [<ffffffff81102c76>] kmemleak_alloc_recursive.clone.5+0x1b/0x1d
>>> [<ffffffff811046b3>] __kmalloc_track_caller+0x18f/0x1b7
>>> [<ffffffff810e1b08>] kstrndup+0x37/0x54
>>> [<ffffffffa0336971>] nfs_parse_devname+0x152/0x204 [nfs]
>>> [<ffffffffa0336af3>] nfs4_validate_text_mount_data+0xd0/0xdc [nfs]
>>> [<ffffffffa0338deb>] nfs_get_sb+0x325/0x736 [nfs]
>>> [<ffffffff81113671>] vfs_kern_mount+0xbd/0x17c
>>> [<ffffffff81113798>] do_kern_mount+0x4d/0xed
>>> [<ffffffff81129a87>] do_mount+0x787/0x7fe
>>> [<ffffffff81129b86>] sys_mount+0x88/0xc2
>>> [<ffffffff81009b42>] system_call_fastpath+0x16/0x1b
>>>
>>> Signed-off-by: Xiaotian Feng<dfeng@redhat.com>
>>> Cc: Trond Myklebust<Trond.Myklebust@netapp.com>
>>> Cc: Chuck Lever<chuck.lever@oracle.com>
>>> Cc: Benny Halevy<bhalevy@panasas.com>
>>> Cc: Al Viro<viro@ZenIV.linux.org.uk>
>>> Cc: Andy Adamson<andros@netapp.com>
>>> ---
>>> fs/nfs/super.c | 1 +
>>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>> index e016372..44e6567 100644
>>> --- a/fs/nfs/super.c
>>> +++ b/fs/nfs/super.c
>>> @@ -2187,6 +2187,7 @@ static int nfs_get_sb(struct file_system_type
>>> *fs_type,
>>> if (data->version == 4) {
>>> error = nfs4_try_mount(flags, dev_name, data, mnt);
>>> kfree(data->client_address);
>>> + kfree(data->nfs_server.export_path);
>>
>> nfs4_try_mount's other call site in fs/nfs/super.c also frees
>> data->nfs_server.hostname and data->fscache_uniq. Does that need to
>> happen here too?
>
> I don't think we need to free data->nfs_server.hostname and
> data->fscache_uniq, they were freed in the out label ;-)
Oy. Eventually, the NFSv2/3 and NFSv4 mount paths need to be made
simpler and more consistent with each other.
Since there can be potentially many dynamically allocated strings
hanging off of "data," it might be cleaner and safer to create a partner
for nfs_alloc_parsed_mount_data() that properly manages the deallocation
of the parsed mount data. Both nfs_get_sb() and nfs4_get_sb() can use that.
>>
>>> goto out;
>>> }
>
> out:
> kfree(data->nfs_server.hostname);
> kfree(data->mount_server.hostname);
> kfree(data->fscache_uniq);
> security_free_mnt_opts(&data->lsm_opts);
>>> #endif /* CONFIG_NFS_V4 */
>>
>>
--
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-04-23 15:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-22 10:56 [PATCH] nfs: fix memory leak in nfs_get_sb with CONFIG_NFS_V4 Xiaotian Feng
2010-04-22 16:09 ` Chuck Lever
2010-04-23 2:29 ` Xiaotian Feng
2010-04-23 15:24 ` Chuck Lever
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox