On Fri 20-04-18 14:29:39, Tetsuo Handa wrote: > Eric Biggers wrote: > > But, there is still a related bug: when mounting sysfs, if register_shrinker() > > fails in sget_userns(), then kernfs_kill_sb() gets called, which frees the > > 'struct kernfs_super_info'. But, the 'struct kernfs_super_info' is also freed > > in kernfs_mount_ns() by: > > > > sb = sget_userns(fs_type, kernfs_test_super, kernfs_set_super, flags, > > &init_user_ns, info); > > if (IS_ERR(sb) || sb->s_fs_info != info) > > kfree(info); > > if (IS_ERR(sb)) > > return ERR_CAST(sb); > > > > I guess the problem is that sget_userns() shouldn't take ownership of the 'info' > > if it returns an error -- but, it actually does if register_shrinker() fails, > > resulting in a double free. > > > > Here is a reproducer and the KASAN splat. This is on Linus' tree (87ef12027b9b) > > with vfs/for-linus merged in. > > I'm waiting for response from Michal Hocko regarding > http://lkml.kernel.org/r/201804111909.EGC64586.QSFLFJFOVHOOtM@xxxxxxxxxxxxxxxxxxx . I didn't plan to respond util all the Al's concerns with the existing scheme are resolved. This is not an urgent thing to fix so better fix it properly. Your API change is kinda ugly so it would be preferable to do it properly as suggested by Al. Maybe that will be more work but my understanding is that the resulting code would be better. If that is not the case then I do not really have any fundamental objection to your patch except it is ugly. -- Michal Hocko SUSE Labs
