Messages in this thread | | | Subject | Re: [PATCH 00/11] fix memory leak while kset_register() fails | From | Yang Yingliang <> | Date | Fri, 21 Oct 2022 16:24:23 +0800 |
| |
On 2022/10/21 13:37, Greg KH wrote: > On Fri, Oct 21, 2022 at 01:29:31AM -0400, Luben Tuikov wrote: >> On 2022-10-20 22:20, Yang Yingliang wrote: >>> The previous discussion link: >>> https://lore.kernel.org/lkml/0db486eb-6927-927e-3629-958f8f211194@huawei.com/T/ >> The very first discussion on this was here: >> >> https://www.spinics.net/lists/dri-devel/msg368077.html >> >> Please use this link, and not the that one up there you which quoted above, >> and whose commit description is taken verbatim from the this link. >> >>> kset_register() is currently used in some places without calling >>> kset_put() in error path, because the callers think it should be >>> kset internal thing to do, but the driver core can not know what >>> caller doing with that memory at times. The memory could be freed >>> both in kset_put() and error path of caller, if it is called in >>> kset_register(). >> As I explained in the link above, the reason there's >> a memory leak is that one cannot call kset_register() without >> the kset->kobj.name being set--kobj_add_internal() returns -EINVAL, >> in this case, i.e. kset_register() fails with -EINVAL. >> >> Thus, the most common usage is something like this: >> >> kobj_set_name(&kset->kobj, format, ...); >> kset->kobj.kset = parent_kset; >> kset->kobj.ktype = ktype; >> res = kset_register(kset); >> >> So, what is being leaked, is the memory allocated in kobj_set_name(), >> by the common idiom shown above. This needs to be mentioned in >> the documentation, at least, in case, in the future this is absolved >> in kset_register() redesign, etc. > Based on this, can kset_register() just clean up from itself when an > error happens? Ideally that would be the case, as the odds of a kset > being embedded in a larger structure is probably slim, but we would have > to search the tree to make sure. I have search the whole tree, the kset used in bus_register() - patch #3, kset_create_and_add() - patch #4 __class_register() - patch #5, fw_cfg_build_symlink() - patch #6 and amdgpu_discovery.c - patch #10 is embedded in a larger structure. In these cases, we can not call kset_put() in error path in kset_register() itself.
Thanks, Yang > > thanks, > > greg k-h > .
| |