* [PATCH] nilfs2: fix memory leak in nilfs_sysfs_create_device_group
@ 2021-09-06 4:13 Dongliang Mu
2021-09-06 5:43 ` Dongliang Mu
0 siblings, 1 reply; 8+ messages in thread
From: Dongliang Mu @ 2021-09-06 4:13 UTC (permalink / raw)
To: Ryusuke Konishi; +Cc: paskripkin, Dongliang Mu, linux-nilfs, linux-kernel
The commit 8fd0c1b0647a ("nilfs2: fix memory leak in
nilfs_sysfs_delete_device_group") adds a kobject_put to free the leaking
object name. However, it is incomplete to only add kobject_put in the
nilfs_sysfs_delete_device_group. The function
nilfs_sysfs_create_device_group also needs the kobject_put to
free the object name in the error handling part.
Fix this by adding kobject_put in the error handling code of
nilfs_sysfs_create_device_group.
Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
---
fs/nilfs2/sysfs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/nilfs2/sysfs.c b/fs/nilfs2/sysfs.c
index 68e8d61e28dd..7ab60711ca76 100644
--- a/fs/nilfs2/sysfs.c
+++ b/fs/nilfs2/sysfs.c
@@ -1024,6 +1024,7 @@ int nilfs_sysfs_create_device_group(struct super_block *sb)
cleanup_dev_kobject:
kobject_del(&nilfs->ns_dev_kobj);
+ kobject_put(&nilfs->ns_dev_kobj);
free_dev_subgroups:
kfree(nilfs->ns_dev_subgroups);
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] nilfs2: fix memory leak in nilfs_sysfs_create_device_group
2021-09-06 4:13 [PATCH] nilfs2: fix memory leak in nilfs_sysfs_create_device_group Dongliang Mu
@ 2021-09-06 5:43 ` Dongliang Mu
2021-09-06 15:56 ` Pavel Skripkin
0 siblings, 1 reply; 8+ messages in thread
From: Dongliang Mu @ 2021-09-06 5:43 UTC (permalink / raw)
To: Ryusuke Konishi; +Cc: Pavel Skripkin, linux-nilfs, linux-kernel
On Mon, Sep 6, 2021 at 12:13 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> The commit 8fd0c1b0647a ("nilfs2: fix memory leak in
> nilfs_sysfs_delete_device_group") adds a kobject_put to free the leaking
> object name. However, it is incomplete to only add kobject_put in the
> nilfs_sysfs_delete_device_group. The function
> nilfs_sysfs_create_device_group also needs the kobject_put to
> free the object name in the error handling part.
>
> Fix this by adding kobject_put in the error handling code of
> nilfs_sysfs_create_device_group.
Even after I add this patch, my local syzkaller still reports this
memory leak one hour later. Therefore, there are some other paths or
magics which can trigger the memory leak. I need to dig deeper.
Pavel, do you have any idea about this crash report?
BUG: memory leak
unreferenced object 0xffff88804a1a8a60 (size 32):
comm "syz-executor", pid 14551, jiffies 4294960586 (age 14.780s)
hex dump (first 32 bytes):
6c 6f 6f 70 35 00 00 00 00 00 00 00 00 00 00 00 loop5...........
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<ffffffff814750c6>] kstrdup+0x36/0x70
[<ffffffff81475153>] kstrdup_const+0x53/0x80
[<ffffffff822773a2>] kvasprintf_const+0xc2/0x110
[<ffffffff82337c5b>] kobject_set_name_vargs+0x3b/0xe0
[<ffffffff823385ed>] kobject_init_and_add+0x6d/0xc0
[<ffffffff81d2bd08>] nilfs_sysfs_create_device_group+0x98/0x3a0
[<ffffffff81d14fc4>] init_nilfs+0x424/0x580
[<ffffffff81d02962>] nilfs_mount+0x532/0x8c0
[<ffffffff815c754b>] legacy_get_tree+0x2b/0x90
[<ffffffff81565158>] vfs_get_tree+0x28/0x100
[<ffffffff815a3a82>] path_mount+0xb92/0xfe0
[<ffffffff815a3f71>] do_mount+0xa1/0xc0
[<ffffffff815a4584>] __x64_sys_mount+0xf4/0x160
[<ffffffff8433fd35>] do_syscall_64+0x35/0xb0
[<ffffffff84400068>] entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
> fs/nilfs2/sysfs.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/nilfs2/sysfs.c b/fs/nilfs2/sysfs.c
> index 68e8d61e28dd..7ab60711ca76 100644
> --- a/fs/nilfs2/sysfs.c
> +++ b/fs/nilfs2/sysfs.c
> @@ -1024,6 +1024,7 @@ int nilfs_sysfs_create_device_group(struct super_block *sb)
>
> cleanup_dev_kobject:
> kobject_del(&nilfs->ns_dev_kobj);
> + kobject_put(&nilfs->ns_dev_kobj);
>
> free_dev_subgroups:
> kfree(nilfs->ns_dev_subgroups);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] nilfs2: fix memory leak in nilfs_sysfs_create_device_group
2021-09-06 5:43 ` Dongliang Mu
@ 2021-09-06 15:56 ` Pavel Skripkin
2021-09-06 16:47 ` Pavel Skripkin
2021-09-07 3:37 ` Dongliang Mu
0 siblings, 2 replies; 8+ messages in thread
From: Pavel Skripkin @ 2021-09-06 15:56 UTC (permalink / raw)
To: Dongliang Mu, Ryusuke Konishi; +Cc: linux-nilfs, linux-kernel
On 9/6/21 08:43, Dongliang Mu wrote:
> On Mon, Sep 6, 2021 at 12:13 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>>
>> The commit 8fd0c1b0647a ("nilfs2: fix memory leak in
>> nilfs_sysfs_delete_device_group") adds a kobject_put to free the leaking
>> object name. However, it is incomplete to only add kobject_put in the
>> nilfs_sysfs_delete_device_group. The function
>> nilfs_sysfs_create_device_group also needs the kobject_put to
>> free the object name in the error handling part.
>>
>> Fix this by adding kobject_put in the error handling code of
>> nilfs_sysfs_create_device_group.
>
> Even after I add this patch, my local syzkaller still reports this
> memory leak one hour later. Therefore, there are some other paths or
> magics which can trigger the memory leak. I need to dig deeper.
>
> Pavel, do you have any idea about this crash report?
>
> BUG: memory leak
> unreferenced object 0xffff88804a1a8a60 (size 32):
> comm "syz-executor", pid 14551, jiffies 4294960586 (age 14.780s)
> hex dump (first 32 bytes):
> 6c 6f 6f 70 35 00 00 00 00 00 00 00 00 00 00 00 loop5...........
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<ffffffff814750c6>] kstrdup+0x36/0x70
> [<ffffffff81475153>] kstrdup_const+0x53/0x80
> [<ffffffff822773a2>] kvasprintf_const+0xc2/0x110
> [<ffffffff82337c5b>] kobject_set_name_vargs+0x3b/0xe0
> [<ffffffff823385ed>] kobject_init_and_add+0x6d/0xc0
> [<ffffffff81d2bd08>] nilfs_sysfs_create_device_group+0x98/0x3a0
> [<ffffffff81d14fc4>] init_nilfs+0x424/0x580
> [<ffffffff81d02962>] nilfs_mount+0x532/0x8c0
> [<ffffffff815c754b>] legacy_get_tree+0x2b/0x90
> [<ffffffff81565158>] vfs_get_tree+0x28/0x100
> [<ffffffff815a3a82>] path_mount+0xb92/0xfe0
> [<ffffffff815a3f71>] do_mount+0xa1/0xc0
> [<ffffffff815a4584>] __x64_sys_mount+0xf4/0x160
> [<ffffffff8433fd35>] do_syscall_64+0x35/0xb0
> [<ffffffff84400068>] entry_SYSCALL_64_after_hwframe+0x44/0xae
>
>
Hi, Dongliang!
This report says nothing to me... It shows, that there is missing
kobject_put() somewhere. I think, we need a reproducer for this leak,
otherwise only code review can help :(
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] nilfs2: fix memory leak in nilfs_sysfs_create_device_group
2021-09-06 15:56 ` Pavel Skripkin
@ 2021-09-06 16:47 ` Pavel Skripkin
2021-09-06 16:51 ` Pavel Skripkin
2021-09-07 4:00 ` Dongliang Mu
2021-09-07 3:37 ` Dongliang Mu
1 sibling, 2 replies; 8+ messages in thread
From: Pavel Skripkin @ 2021-09-06 16:47 UTC (permalink / raw)
To: Dongliang Mu, Ryusuke Konishi; +Cc: linux-nilfs, linux-kernel
On 9/6/21 18:56, Pavel Skripkin wrote:
> On 9/6/21 08:43, Dongliang Mu wrote:
>> On Mon, Sep 6, 2021 at 12:13 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>>>
>>> The commit 8fd0c1b0647a ("nilfs2: fix memory leak in
>>> nilfs_sysfs_delete_device_group") adds a kobject_put to free the leaking
>>> object name. However, it is incomplete to only add kobject_put in the
>>> nilfs_sysfs_delete_device_group. The function
>>> nilfs_sysfs_create_device_group also needs the kobject_put to
>>> free the object name in the error handling part.
>>>
>>> Fix this by adding kobject_put in the error handling code of
>>> nilfs_sysfs_create_device_group.
>>
>> Even after I add this patch, my local syzkaller still reports this
>> memory leak one hour later. Therefore, there are some other paths or
>> magics which can trigger the memory leak. I need to dig deeper.
>>
>> Pavel, do you have any idea about this crash report?
>>
>> BUG: memory leak
>> unreferenced object 0xffff88804a1a8a60 (size 32):
>> comm "syz-executor", pid 14551, jiffies 4294960586 (age 14.780s)
>> hex dump (first 32 bytes):
>> 6c 6f 6f 70 35 00 00 00 00 00 00 00 00 00 00 00 loop5...........
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>> backtrace:
>> [<ffffffff814750c6>] kstrdup+0x36/0x70
>> [<ffffffff81475153>] kstrdup_const+0x53/0x80
>> [<ffffffff822773a2>] kvasprintf_const+0xc2/0x110
>> [<ffffffff82337c5b>] kobject_set_name_vargs+0x3b/0xe0
>> [<ffffffff823385ed>] kobject_init_and_add+0x6d/0xc0
>> [<ffffffff81d2bd08>] nilfs_sysfs_create_device_group+0x98/0x3a0
>> [<ffffffff81d14fc4>] init_nilfs+0x424/0x580
>> [<ffffffff81d02962>] nilfs_mount+0x532/0x8c0
>> [<ffffffff815c754b>] legacy_get_tree+0x2b/0x90
>> [<ffffffff81565158>] vfs_get_tree+0x28/0x100
>> [<ffffffff815a3a82>] path_mount+0xb92/0xfe0
>> [<ffffffff815a3f71>] do_mount+0xa1/0xc0
>> [<ffffffff815a4584>] __x64_sys_mount+0xf4/0x160
>> [<ffffffff8433fd35>] do_syscall_64+0x35/0xb0
>> [<ffffffff84400068>] entry_SYSCALL_64_after_hwframe+0x44/0xae
>>
>>
>
> Hi, Dongliang!
>
>
> This report says nothing to me... It shows, that there is missing
> kobject_put() somewhere. I think, we need a reproducer for this leak,
> otherwise only code review can help :(
>
>
>
Hm, I guess, I see... We should call kobject_put() in case of
kobject_init_and_add() failure:
lib/kobject.c:459
* If this function returns an error, kobject_put() must be called to
* properly clean up the memory associated with the object. This is the
so I suggest:
diff --git a/fs/nilfs2/sysfs.c b/fs/nilfs2/sysfs.c
index 68e8d61e28dd..e8717f4ba2a1 100644
--- a/fs/nilfs2/sysfs.c
+++ b/fs/nilfs2/sysfs.c
@@ -1026,6 +1026,7 @@ int nilfs_sysfs_create_device_group(struct
super_block *sb) kobject_del(&nilfs->ns_dev_kobj);
free_dev_subgroups:
+ kobject_put(&nilfs->ns_dev_kobj);
kfree(nilfs->ns_dev_subgroups);
failed_create_device_group:
This patch should fix 2 memory leaks :)
With regards,
Pavel Skripkin
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] nilfs2: fix memory leak in nilfs_sysfs_create_device_group
2021-09-06 16:47 ` Pavel Skripkin
@ 2021-09-06 16:51 ` Pavel Skripkin
2021-09-07 4:00 ` Dongliang Mu
1 sibling, 0 replies; 8+ messages in thread
From: Pavel Skripkin @ 2021-09-06 16:51 UTC (permalink / raw)
To: Dongliang Mu, Ryusuke Konishi; +Cc: linux-nilfs, linux-kernel
On 9/6/21 19:47, Pavel Skripkin wrote:
> On 9/6/21 18:56, Pavel Skripkin wrote:
>> On 9/6/21 08:43, Dongliang Mu wrote:
>>> On Mon, Sep 6, 2021 at 12:13 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>>>>
>>>> The commit 8fd0c1b0647a ("nilfs2: fix memory leak in
>>>> nilfs_sysfs_delete_device_group") adds a kobject_put to free the leaking
>>>> object name. However, it is incomplete to only add kobject_put in the
>>>> nilfs_sysfs_delete_device_group. The function
>>>> nilfs_sysfs_create_device_group also needs the kobject_put to
>>>> free the object name in the error handling part.
>>>>
>>>> Fix this by adding kobject_put in the error handling code of
>>>> nilfs_sysfs_create_device_group.
>>>
>>> Even after I add this patch, my local syzkaller still reports this
>>> memory leak one hour later. Therefore, there are some other paths or
>>> magics which can trigger the memory leak. I need to dig deeper.
>>>
>>> Pavel, do you have any idea about this crash report?
>>>
>>> BUG: memory leak
>>> unreferenced object 0xffff88804a1a8a60 (size 32):
>>> comm "syz-executor", pid 14551, jiffies 4294960586 (age 14.780s)
>>> hex dump (first 32 bytes):
>>> 6c 6f 6f 70 35 00 00 00 00 00 00 00 00 00 00 00 loop5...........
>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>>> backtrace:
>>> [<ffffffff814750c6>] kstrdup+0x36/0x70
>>> [<ffffffff81475153>] kstrdup_const+0x53/0x80
>>> [<ffffffff822773a2>] kvasprintf_const+0xc2/0x110
>>> [<ffffffff82337c5b>] kobject_set_name_vargs+0x3b/0xe0
>>> [<ffffffff823385ed>] kobject_init_and_add+0x6d/0xc0
>>> [<ffffffff81d2bd08>] nilfs_sysfs_create_device_group+0x98/0x3a0
>>> [<ffffffff81d14fc4>] init_nilfs+0x424/0x580
>>> [<ffffffff81d02962>] nilfs_mount+0x532/0x8c0
>>> [<ffffffff815c754b>] legacy_get_tree+0x2b/0x90
>>> [<ffffffff81565158>] vfs_get_tree+0x28/0x100
>>> [<ffffffff815a3a82>] path_mount+0xb92/0xfe0
>>> [<ffffffff815a3f71>] do_mount+0xa1/0xc0
>>> [<ffffffff815a4584>] __x64_sys_mount+0xf4/0x160
>>> [<ffffffff8433fd35>] do_syscall_64+0x35/0xb0
>>> [<ffffffff84400068>] entry_SYSCALL_64_after_hwframe+0x44/0xae
>>>
>>>
>>
>> Hi, Dongliang!
>>
>>
>> This report says nothing to me... It shows, that there is missing
>> kobject_put() somewhere. I think, we need a reproducer for this leak,
>> otherwise only code review can help :(
>>
>>
>>
>
> Hm, I guess, I see... We should call kobject_put() in case of
> kobject_init_and_add() failure:
>
> lib/kobject.c:459
>
> * If this function returns an error, kobject_put() must be called to
> * properly clean up the memory associated with the object. This is the
>
>
> so I suggest:
>
> diff --git a/fs/nilfs2/sysfs.c b/fs/nilfs2/sysfs.c
> index 68e8d61e28dd..e8717f4ba2a1 100644
> --- a/fs/nilfs2/sysfs.c
> +++ b/fs/nilfs2/sysfs.c
> @@ -1026,6 +1026,7 @@ int nilfs_sysfs_create_device_group(struct
> super_block *sb) kobject_del(&nilfs->ns_dev_kobj);
>
> free_dev_subgroups:
> + kobject_put(&nilfs->ns_dev_kobj);
> kfree(nilfs->ns_dev_subgroups);
>
> failed_create_device_group:
>
>
> This patch should fix 2 memory leaks :)
>
>
Also, I think, we should add kobject_put() call to
1. nilfs_sysfs_create_##name##_group
2. nilfs_sysfs_create_snapshot_group
To prevent other random kobject memory leaks in nilfs...
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] nilfs2: fix memory leak in nilfs_sysfs_create_device_group
2021-09-06 15:56 ` Pavel Skripkin
2021-09-06 16:47 ` Pavel Skripkin
@ 2021-09-07 3:37 ` Dongliang Mu
1 sibling, 0 replies; 8+ messages in thread
From: Dongliang Mu @ 2021-09-07 3:37 UTC (permalink / raw)
To: Pavel Skripkin; +Cc: Ryusuke Konishi, linux-nilfs, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2581 bytes --]
On Mon, Sep 6, 2021 at 11:56 PM Pavel Skripkin <paskripkin@gmail.com> wrote:
>
> On 9/6/21 08:43, Dongliang Mu wrote:
> > On Mon, Sep 6, 2021 at 12:13 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >>
> >> The commit 8fd0c1b0647a ("nilfs2: fix memory leak in
> >> nilfs_sysfs_delete_device_group") adds a kobject_put to free the leaking
> >> object name. However, it is incomplete to only add kobject_put in the
> >> nilfs_sysfs_delete_device_group. The function
> >> nilfs_sysfs_create_device_group also needs the kobject_put to
> >> free the object name in the error handling part.
> >>
> >> Fix this by adding kobject_put in the error handling code of
> >> nilfs_sysfs_create_device_group.
> >
> > Even after I add this patch, my local syzkaller still reports this
> > memory leak one hour later. Therefore, there are some other paths or
> > magics which can trigger the memory leak. I need to dig deeper.
> >
> > Pavel, do you have any idea about this crash report?
> >
> > BUG: memory leak
> > unreferenced object 0xffff88804a1a8a60 (size 32):
> > comm "syz-executor", pid 14551, jiffies 4294960586 (age 14.780s)
> > hex dump (first 32 bytes):
> > 6c 6f 6f 70 35 00 00 00 00 00 00 00 00 00 00 00 loop5...........
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> > backtrace:
> > [<ffffffff814750c6>] kstrdup+0x36/0x70
> > [<ffffffff81475153>] kstrdup_const+0x53/0x80
> > [<ffffffff822773a2>] kvasprintf_const+0xc2/0x110
> > [<ffffffff82337c5b>] kobject_set_name_vargs+0x3b/0xe0
> > [<ffffffff823385ed>] kobject_init_and_add+0x6d/0xc0
> > [<ffffffff81d2bd08>] nilfs_sysfs_create_device_group+0x98/0x3a0
> > [<ffffffff81d14fc4>] init_nilfs+0x424/0x580
> > [<ffffffff81d02962>] nilfs_mount+0x532/0x8c0
> > [<ffffffff815c754b>] legacy_get_tree+0x2b/0x90
> > [<ffffffff81565158>] vfs_get_tree+0x28/0x100
> > [<ffffffff815a3a82>] path_mount+0xb92/0xfe0
> > [<ffffffff815a3f71>] do_mount+0xa1/0xc0
> > [<ffffffff815a4584>] __x64_sys_mount+0xf4/0x160
> > [<ffffffff8433fd35>] do_syscall_64+0x35/0xb0
> > [<ffffffff84400068>] entry_SYSCALL_64_after_hwframe+0x44/0xae
> >
> >
>
> Hi, Dongliang!
>
>
> This report says nothing to me... It shows, that there is missing
> kobject_put() somewhere. I think, we need a reproducer for this leak,
> otherwise only code review can help :(
I have attached a syz reproducer to this email. Even if the kernel is
patched with your patch and my patch, it still triggers a memory leak.
>
>
>
> With regards,
> Pavel Skripkin
[-- Attachment #2: syz --]
[-- Type: application/octet-stream, Size: 13763 bytes --]
# {Threaded:true Collide:true Repeat:true RepeatTimes:0 Procs:8 Slowdown:1 Sandbox:none Fault:false FaultCall:-1 FaultNth:0 Leak:true NetInjection:true NetDevices:true NetReset:true Cgroups:true BinfmtMisc:true CloseFDs:true KCSAN:false DevlinkPCI:false USB:true VhciInjection:true Wifi:true IEEE802154:true Sysctl:true UseTmpDir:true HandleSegv:true Repro:false Trace:false}
r0 = openat2(0xffffffffffffff9c, 0x0, 0x0, 0x0)
syz_mount_image$nilfs2(&(0x7f0000000000), &(0x7f0000000100)='./file0\x00', 0x0, 0x19, &(0x7f0000000200)=[{&(0x7f0000010000)="0200000000003434180100007a4a793440d197c3010000001f00000000000000000010000000000002000000000000001000000005000000010000000000000002000000000000000000000000000000e001000000000000561b675f00000000571b675f00000000571b675f000000000100320000000100561b675f00000000004eed0000000000000000000b00000080002000c0001000e28253618b3b42849b9543a54024813a00"/192, 0xc0, 0x400}, {&(0x7f0000010100)="00000000000000000100"/32, 0x20, 0x500}, {&(0x7f0000010200)="1740cc4333e8f05b11faaf1e400007000000000000000000561b675f0000000010000000000000000b0000000500000030010000000000000100000000000000020000000000000001000000000000000100000001000000010000000000000000000000000000000600000000000000010000000000000003000000030000000200000000000000000000000000000003000000000000000100000000000000040000000000000002000000000000000400000000000000010000000000000001000000010000000500000000000000000000000000000005000000000000000100000000000000010000000100000006000000000000000000000000000000030000000000000001000000000000000300000003000000000000000000000001000000000000000200"/320, 0x140, 0x1000}, {&(0x7f0000010400)="0200000000000000100001022e0000000200000000000000100002022e2e00000b00000000000000e00706012e6e696c667300"/64, 0x40, 0x1800}, {&(0x7f0000010500)="f43f000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000ff0f00"/2080, 0x820, 0x2000}, {&(0x7f0000010e00)="00000000000000000000000000000000561b675f00000000561b675f000000000000000000000000000000000000000000800100"/64, 0x40, 0x3080}, {&(0x7f0000010f00)="01000000000000000008000000000000561b675f00000000561b675f0000000000000000000000000000000000000000ed4102000000000000000000000000000100"/96, 0x60, 0x3100}, {&(0x7f0000011000)="00000000000000000000000000000000561b675f00000000561b675f000000000000000000000000000000000000000000800100"/64, 0x40, 0x3380}, {&(0x7f0000011100)="00000000000000000000000000000000561b675f00000000561b675f000000000000000000000000000000000000000000800100"/64, 0x40, 0x3400}, {&(0x7f0000011200)="00000000000000000000000000000000561b675f00000000561b675f000000000000000000000000000000000000000000800100"/64, 0x40, 0x3480}, {&(0x7f0000011300)="00000000000000000000000000000000561b675f00000000561b675f000000000000000000000000000000000000000000800100"/64, 0x40, 0x3500}, {&(0x7f0000011400)="00000000000000000000000000000000561b675f00000000561b675f0000000000000000000000000000000000000000a4810100"/64, 0x40, 0x3580}, {&(0x7f0000011500)="0100"/32, 0x20, 0x3800}, {&(0x7f0000011600)="0000000000000000000000000000000000000000000000000100000000000000561b675f000000000b000000000000000200000000000000040000000000000003000000000000000000000000000000561b675f00000000561b675f000000000000000000000000000000000000000000800100000000000000000000000000020000000000000003000000000000000400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000200000000000000", 0xe0, 0x38c0}, {&(0x7f0000011700)="0200000000000000000000000000000000000000000000000300000000000000", 0x20, 0x3a40}, {&(0x7f0000011800)="0200000000000000000000000000000000000000000000000400000000000000", 0x20, 0x3b00}, {&(0x7f0000011900)="0200000000000000000000000000000000000000000000000500000000000000", 0x20, 0x3bc0}, {&(0x7f0000011a00)="0200000000000000000000000000000000000000000000000600000000000000", 0x20, 0x3c80}, {&(0x7f0000011b00)="0200000000000000000000000000000000000000000000000700000000000000", 0x20, 0x3d40}, {&(0x7f0000011c00)="0200000000000000000000000000000000000000000000000800000000000000", 0x20, 0x3e00}, {&(0x7f0000011d00)="0200000000000000000000000000000000000000000000000900000000000000", 0x20, 0x3ec0}, {&(0x7f0000011e00)="1d0000000000000002000000000000001e000000000000000000000000000000561b675f000000000b0000000300000000000000000000000000000003000000", 0x40, 0x4000}, {&(0x7f0000011f00)="f93f0000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000000040000000400000004000007f00"/2080, 0x820, 0x4800}, {&(0x7f0000012800)="03000000000000000100000000000000ffffffffffffffff000000000000000004000000000000000100000000000000ffffffffffffffff000000000000000005000000000000000100000000000000ffffffffffffffff000000000000000006000000000000000100000000000000ffffffffffffffff000000000000000007000000000000000100000000000000ffffffffffffffff000000000000000008000000000000000100000000000000ffffffffffffffff0000", 0xba, 0x5820}, {&(0x7f0000012900)="d348c23990010000561b675f0000000003000000000000000000000000000000561b675f00000000561b675f00000000000000000000000000000000000000000080010000000000000000000000000009000000000000000a000000000000000b000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000561b675f00000000561b675f0000000000000000000000000000000000000000008001000000000000000000000000000500000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000561b675f00000000561b675f00000000000000000000000000000000000000000080010000000000000000000000000006", 0x151, 0x6000}], 0x0, &(0x7f0000001580)={[{@order_relaxed}, {@nodiscard}, {@order_relaxed}, {@order_strict}]})
openat$cgroup_netprio_ifpriomap(r0, 0x0, 0x2, 0x0)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] nilfs2: fix memory leak in nilfs_sysfs_create_device_group
2021-09-06 16:47 ` Pavel Skripkin
2021-09-06 16:51 ` Pavel Skripkin
@ 2021-09-07 4:00 ` Dongliang Mu
2021-09-13 5:23 ` Dongliang Mu
1 sibling, 1 reply; 8+ messages in thread
From: Dongliang Mu @ 2021-09-07 4:00 UTC (permalink / raw)
To: Pavel Skripkin; +Cc: Ryusuke Konishi, linux-nilfs, linux-kernel
On Tue, Sep 7, 2021 at 12:47 AM Pavel Skripkin <paskripkin@gmail.com> wrote:
>
> On 9/6/21 18:56, Pavel Skripkin wrote:
> > On 9/6/21 08:43, Dongliang Mu wrote:
> >> On Mon, Sep 6, 2021 at 12:13 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >>>
> >>> The commit 8fd0c1b0647a ("nilfs2: fix memory leak in
> >>> nilfs_sysfs_delete_device_group") adds a kobject_put to free the leaking
> >>> object name. However, it is incomplete to only add kobject_put in the
> >>> nilfs_sysfs_delete_device_group. The function
> >>> nilfs_sysfs_create_device_group also needs the kobject_put to
> >>> free the object name in the error handling part.
> >>>
> >>> Fix this by adding kobject_put in the error handling code of
> >>> nilfs_sysfs_create_device_group.
> >>
> >> Even after I add this patch, my local syzkaller still reports this
> >> memory leak one hour later. Therefore, there are some other paths or
> >> magics which can trigger the memory leak. I need to dig deeper.
> >>
> >> Pavel, do you have any idea about this crash report?
> >>
> >> BUG: memory leak
> >> unreferenced object 0xffff88804a1a8a60 (size 32):
> >> comm "syz-executor", pid 14551, jiffies 4294960586 (age 14.780s)
> >> hex dump (first 32 bytes):
> >> 6c 6f 6f 70 35 00 00 00 00 00 00 00 00 00 00 00 loop5...........
> >> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> >> backtrace:
> >> [<ffffffff814750c6>] kstrdup+0x36/0x70
> >> [<ffffffff81475153>] kstrdup_const+0x53/0x80
> >> [<ffffffff822773a2>] kvasprintf_const+0xc2/0x110
> >> [<ffffffff82337c5b>] kobject_set_name_vargs+0x3b/0xe0
> >> [<ffffffff823385ed>] kobject_init_and_add+0x6d/0xc0
> >> [<ffffffff81d2bd08>] nilfs_sysfs_create_device_group+0x98/0x3a0
> >> [<ffffffff81d14fc4>] init_nilfs+0x424/0x580
> >> [<ffffffff81d02962>] nilfs_mount+0x532/0x8c0
> >> [<ffffffff815c754b>] legacy_get_tree+0x2b/0x90
> >> [<ffffffff81565158>] vfs_get_tree+0x28/0x100
> >> [<ffffffff815a3a82>] path_mount+0xb92/0xfe0
> >> [<ffffffff815a3f71>] do_mount+0xa1/0xc0
> >> [<ffffffff815a4584>] __x64_sys_mount+0xf4/0x160
> >> [<ffffffff8433fd35>] do_syscall_64+0x35/0xb0
> >> [<ffffffff84400068>] entry_SYSCALL_64_after_hwframe+0x44/0xae
> >>
> >>
> >
> > Hi, Dongliang!
> >
> >
> > This report says nothing to me... It shows, that there is missing
> > kobject_put() somewhere. I think, we need a reproducer for this leak,
> > otherwise only code review can help :(
> >
> >
> >
>
> Hm, I guess, I see... We should call kobject_put() in case of
> kobject_init_and_add() failure:
>
> lib/kobject.c:459
>
> * If this function returns an error, kobject_put() must be called to
> * properly clean up the memory associated with the object. This is the
>
>
> so I suggest:
>
> diff --git a/fs/nilfs2/sysfs.c b/fs/nilfs2/sysfs.c
> index 68e8d61e28dd..e8717f4ba2a1 100644
> --- a/fs/nilfs2/sysfs.c
> +++ b/fs/nilfs2/sysfs.c
> @@ -1026,6 +1026,7 @@ int nilfs_sysfs_create_device_group(struct
> super_block *sb) kobject_del(&nilfs->ns_dev_kobj);
>
> free_dev_subgroups:
> + kobject_put(&nilfs->ns_dev_kobj);
> kfree(nilfs->ns_dev_subgroups);
>
> failed_create_device_group:
>
>
> This patch should fix 2 memory leaks :)
Yes. We should move kobject_put from the label cleanup_dev_kobject to
free_dev_subgroups.
However, I tried the syz reproducer just sent to you. The memory leak
is still there.
>
>
>
>
> With regards,
> Pavel Skripkin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] nilfs2: fix memory leak in nilfs_sysfs_create_device_group
2021-09-07 4:00 ` Dongliang Mu
@ 2021-09-13 5:23 ` Dongliang Mu
0 siblings, 0 replies; 8+ messages in thread
From: Dongliang Mu @ 2021-09-13 5:23 UTC (permalink / raw)
To: Pavel Skripkin; +Cc: Ryusuke Konishi, linux-nilfs, linux-kernel
There are many commits related to memory leak fix in nilfs_sysfs.
4 days nilfs2: fix memory leak in nilfs_sysfs_delete_snapshot_group
4 days nilfs2: fix memory leak in nilfs_sysfs_create_snapshot_group
4 days nilfs2: fix memory leak in nilfs_sysfs_delete_##name##_group
4 days nilfs2: fix memory leak in nilfs_sysfs_create_##name##_group
4 days nilfs2: fix NULL pointer in nilfs_##name##_attr_release
4 days nilfs2: fix memory leak in nilfs_sysfs_create_device_group
This thread can be closed.
Best regards,
Dongliang Mu
On Tue, Sep 7, 2021 at 12:00 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> On Tue, Sep 7, 2021 at 12:47 AM Pavel Skripkin <paskripkin@gmail.com> wrote:
> >
> > On 9/6/21 18:56, Pavel Skripkin wrote:
> > > On 9/6/21 08:43, Dongliang Mu wrote:
> > >> On Mon, Sep 6, 2021 at 12:13 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> > >>>
> > >>> The commit 8fd0c1b0647a ("nilfs2: fix memory leak in
> > >>> nilfs_sysfs_delete_device_group") adds a kobject_put to free the leaking
> > >>> object name. However, it is incomplete to only add kobject_put in the
> > >>> nilfs_sysfs_delete_device_group. The function
> > >>> nilfs_sysfs_create_device_group also needs the kobject_put to
> > >>> free the object name in the error handling part.
> > >>>
> > >>> Fix this by adding kobject_put in the error handling code of
> > >>> nilfs_sysfs_create_device_group.
> > >>
> > >> Even after I add this patch, my local syzkaller still reports this
> > >> memory leak one hour later. Therefore, there are some other paths or
> > >> magics which can trigger the memory leak. I need to dig deeper.
> > >>
> > >> Pavel, do you have any idea about this crash report?
> > >>
> > >> BUG: memory leak
> > >> unreferenced object 0xffff88804a1a8a60 (size 32):
> > >> comm "syz-executor", pid 14551, jiffies 4294960586 (age 14.780s)
> > >> hex dump (first 32 bytes):
> > >> 6c 6f 6f 70 35 00 00 00 00 00 00 00 00 00 00 00 loop5...........
> > >> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> > >> backtrace:
> > >> [<ffffffff814750c6>] kstrdup+0x36/0x70
> > >> [<ffffffff81475153>] kstrdup_const+0x53/0x80
> > >> [<ffffffff822773a2>] kvasprintf_const+0xc2/0x110
> > >> [<ffffffff82337c5b>] kobject_set_name_vargs+0x3b/0xe0
> > >> [<ffffffff823385ed>] kobject_init_and_add+0x6d/0xc0
> > >> [<ffffffff81d2bd08>] nilfs_sysfs_create_device_group+0x98/0x3a0
> > >> [<ffffffff81d14fc4>] init_nilfs+0x424/0x580
> > >> [<ffffffff81d02962>] nilfs_mount+0x532/0x8c0
> > >> [<ffffffff815c754b>] legacy_get_tree+0x2b/0x90
> > >> [<ffffffff81565158>] vfs_get_tree+0x28/0x100
> > >> [<ffffffff815a3a82>] path_mount+0xb92/0xfe0
> > >> [<ffffffff815a3f71>] do_mount+0xa1/0xc0
> > >> [<ffffffff815a4584>] __x64_sys_mount+0xf4/0x160
> > >> [<ffffffff8433fd35>] do_syscall_64+0x35/0xb0
> > >> [<ffffffff84400068>] entry_SYSCALL_64_after_hwframe+0x44/0xae
> > >>
> > >>
> > >
> > > Hi, Dongliang!
> > >
> > >
> > > This report says nothing to me... It shows, that there is missing
> > > kobject_put() somewhere. I think, we need a reproducer for this leak,
> > > otherwise only code review can help :(
> > >
> > >
> > >
> >
> > Hm, I guess, I see... We should call kobject_put() in case of
> > kobject_init_and_add() failure:
> >
> > lib/kobject.c:459
> >
> > * If this function returns an error, kobject_put() must be called to
> > * properly clean up the memory associated with the object. This is the
> >
> >
> > so I suggest:
> >
> > diff --git a/fs/nilfs2/sysfs.c b/fs/nilfs2/sysfs.c
> > index 68e8d61e28dd..e8717f4ba2a1 100644
> > --- a/fs/nilfs2/sysfs.c
> > +++ b/fs/nilfs2/sysfs.c
> > @@ -1026,6 +1026,7 @@ int nilfs_sysfs_create_device_group(struct
> > super_block *sb) kobject_del(&nilfs->ns_dev_kobj);
> >
> > free_dev_subgroups:
> > + kobject_put(&nilfs->ns_dev_kobj);
> > kfree(nilfs->ns_dev_subgroups);
> >
> > failed_create_device_group:
> >
> >
> > This patch should fix 2 memory leaks :)
>
> Yes. We should move kobject_put from the label cleanup_dev_kobject to
> free_dev_subgroups.
>
> However, I tried the syz reproducer just sent to you. The memory leak
> is still there.
>
> >
> >
> >
> >
> > With regards,
> > Pavel Skripkin
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-09-13 5:24 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-06 4:13 [PATCH] nilfs2: fix memory leak in nilfs_sysfs_create_device_group Dongliang Mu
2021-09-06 5:43 ` Dongliang Mu
2021-09-06 15:56 ` Pavel Skripkin
2021-09-06 16:47 ` Pavel Skripkin
2021-09-06 16:51 ` Pavel Skripkin
2021-09-07 4:00 ` Dongliang Mu
2021-09-13 5:23 ` Dongliang Mu
2021-09-07 3:37 ` Dongliang Mu
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).