On Mon 20-07-15 16:46:42, Jan Kara wrote: > On Sun 19-07-15 18:21:49, Kinglong Mee wrote: > > On 7/15/2015 21:21, Jan Kara wrote: > > > From: Jan Kara <jack@xxxxxxx> > > > > > > fsnotify_clear_marks_by_group_flags() can race with > > > fsnotify_destroy_marks() so when fsnotify_destroy_mark_locked() drops > > > mark_mutex, a mark from the list iterated by > > > fsnotify_clear_marks_by_group_flags() can be freed and we dereference > > > free memory in the loop there. > > > > > > Fix the problem by keeping mark_mutex held in > > > fsnotify_destroy_mark_locked(). The reason why we drop that mutex is > > > that we need to call a ->freeing_mark() callback which may acquire > > > mark_mutex again. To avoid this and similar lock inversion issues, we > > > move the call to ->freeing_mark() callback to the kthread destroying the > > > mark. > > > > > > Reported-by: Ashish Sangwan <a.sangwan@xxxxxxxxxxx> > > > Suggested-by: Lino Sanfilippo <LinoSanfilippo@xxxxxx> > > > Signed-off-by: Jan Kara <jack@xxxxxxx> > > <snip> > > > With this patch, I got so many memleak notice, > > > > unreferenced object 0xffff880035bef640 (size 64): > > comm "fsnotify_mark", pid 26, jiffies 4294673717 (age 628.737s) > > hex dump (first 32 bytes): > > 28 36 3f 76 00 88 ff ff 28 36 3f 76 00 88 ff ff (6?v....(6?v.... > > 00 00 00 00 00 00 00 00 00 80 00 00 00 00 ad de ................ > > backtrace: > > [<ffffffff816cd34e>] kmemleak_alloc+0x4e/0xb0 > > [<ffffffff811ac6b5>] __kmalloc+0x1e5/0x290 > > [<ffffffff81204f25>] inotify_handle_event+0x75/0x160 > > [<ffffffff81205abc>] inotify_ignored_and_remove_idr+0x5c/0x80 > > [<ffffffff8120505e>] inotify_freeing_mark+0xe/0x10 > > [<ffffffff81203ca6>] fsnotify_mark_destroy+0xb6/0x150 > > [<ffffffff810a4487>] kthread+0xd7/0xf0 > > [<ffffffff816d92df>] ret_from_fork+0x3f/0x70 > > [<ffffffffffffffff>] 0xffffffffffffffff > > > > It is caused by ->freeing_mark() insert an event to group, > > but snotify_put_mark() kfree the group without free the event. > > Thanks for report! You are right that my patch introduces a race between > fsnotify kthread and fsnotify_destroy_group() which can result in leaking > inotify event on group destruction. I haven't yet decided whether the right > fix is not to queue events for dying notification group (as that is > pointless anyway) or whether we should just fix the original problem > differently... Whenever I look at fsnotify code mark handling I get lost in > the maze of locks, lists, and subtle differences between how different > notification systems handle notification marks :( I'll think about it over > night. Bah, my sent emails were just getting queued for last two days. Sorry for the delay.. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
