Re: [PATCH] kobject: Safe return of kobject_get_path with NULL

From: gregkh@xxxxxxxxxxxxxxxxxxx
Date: Thu Jun 24 2021 - 05:02:22 EST


On Thu, Jun 24, 2021 at 06:42:41AM +0000, Christian Löhle wrote:
> Hey Greg,
>
> >> Prevent NULL dereference within get_kobj_path_length
> >>
> >> Calling kobject_get_path could provoke a NULL dereference
> >> if NULL was passed. while fill_kobj_path will return
> >> with a sane 0 for NULL, kobjet_get_path_length did not.
> >
> >Who passes NULL into that function?  Shouldn't that be fixed first?
>
> It seems to me like here specifically it was a sd_open on some no longer
> existing device. I agree, but could not find a fix for that, and even if, it might
> not have been in the current tree.
> But when looking at the kobject code it felt like it was meant to be safe for
> NULL, (like any parent in the tree can be NULL), but the do while does hide that
> a bit.
> So is it not meant to be safe?

Not always, no. It's better to fix the root problem and not paper over
it by doing something like this.

> I will try to find the sd_open issue some more, but cannot reproduce the issue
> consistently enough right now.
>
>
> >Pleaase always run your patches through checkpatch.pl so you do not get
> >maintainers asking you to use checkpatch.pl...
>
> I did, so please tell me what part bothers you, so I can get that fixed, either in v2
> or maybe even in checkpatch.pl?
> (Only thing I spotted now is the kobjet typo)

You should put a blank line after variable definitions and before the
code logic. Normally checkpatch catches that, odd it did not here...

thanks,

greg k-h