linux-kernel - Re: [PATCH] kobject: Safe return of kobject_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <YNRKGUgXW9gciMzO@kroah.com>
Date:   Thu, 24 Jun 2021 11:02:17 +0200
From:   "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
To:     Christian Löhle <CLoehle@...erstone.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] kobject: Safe return of kobject_get_path with NULL

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