linux-kernel - Re: [PATCH v1] kobject: make sure parent is not released before children

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <71775e76-6175-d64b-0f4e-1beeb6b589b3@infradead.org>
Date:   Tue, 14 Apr 2020 15:38:46 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     Brendan Higgins <brendanhiggins@...gle.com>,
        gregkh@...uxfoundation.org, rafael@...nel.org
Cc:     linux-kernel@...r.kernel.org, naresh.kamboju@...aro.org,
        sakari.ailus@...ux.intel.com, andy.shevchenko@...il.com,
        hdegoede@...hat.com, rafael.j.wysocki@...el.com,
        linux-kselftest@...r.kernel.org, rostedt@...dmis.org,
        sergey.senozhatsky@...il.com, andriy.shevchenko@...ux.intel.com,
        shuah@...nel.org, anders.roxell@...aro.org,
        lkft-triage@...ts.linaro.org, linux@...musvillemoes.dk,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Subject: Re: [PATCH v1] kobject: make sure parent is not released before
 children

On 4/14/20 1:42 PM, Brendan Higgins wrote:
> From: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> 
> Previously, kobjects were released before the associated kobj_types;
> this can cause a kobject deallocation to fail when the kobject has
> children; an example of this is in software_node_unregister_nodes(); it
> calls release on the parent before children meaning that children can be
> released after the parent, which may be needed for removal.
> 
> So, take a reference to the parent before we delete a node to ensure
> that the parent is not released before the children.
> 
> Reported-by: Naresh Kamboju <naresh.kamboju@...aro.org>
> Fixes: 7589238a8cf3 ("Revert "software node: Simplify software_node_release() function"")
> Link: https://lore.kernel.org/linux-kselftest/CAFd5g44s5NQvT8TG_x4rwbqoa7zWzkV0TX+ETZoQdOB7OwXCPQ@mail.gmail.com/T/#m71f37f3985f2abd7209c8ca8e0fa4edc45e171d6
> Co-developed-by: Brendan Higgins <brendanhiggins@...gle.com>
> Signed-off-by: Brendan Higgins <brendanhiggins@...gle.com>

Tested-by: Randy Dunlap <rdunlap@...radead.org>

Fixes the lib/test_printf.ko use-after-free on linux-next 20200410
that I reported last week.


> ---
> 
> This patch is based on the diff written by Heikki linked above.
> 
> Heikki, can you either reply with a Signed-off-by? Otherwise, I can
> resend with me as the author and I will list you as the Co-developed-by.
> 
> Sorry for all the CCs: I just want to make sure everyone who was a party
> to the original bug sees this.
> 
> ---
>  lib/kobject.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/kobject.c b/lib/kobject.c
> index 83198cb37d8d..5921e2470b46 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -663,6 +663,7 @@ EXPORT_SYMBOL(kobject_get_unless_zero);
>   */
>  static void kobject_cleanup(struct kobject *kobj)
>  {
> +	struct kobject *parent = kobj->parent;
>  	struct kobj_type *t = get_ktype(kobj);
>  	const char *name = kobj->name;
>  
> @@ -680,6 +681,9 @@ static void kobject_cleanup(struct kobject *kobj)
>  		kobject_uevent(kobj, KOBJ_REMOVE);
>  	}
>  
> +	/* make sure the parent is not released before the (last) child */
> +	kobject_get(parent);
> +
>  	/* remove from sysfs if the caller did not do it */
>  	if (kobj->state_in_sysfs) {
>  		pr_debug("kobject: '%s' (%p): auto cleanup kobject_del\n",
> @@ -693,6 +697,8 @@ static void kobject_cleanup(struct kobject *kobj)
>  		t->release(kobj);
>  	}
>  
> +	kobject_put(parent);
> +
>  	/* free name if we allocated it */
>  	if (name) {
>  		pr_debug("kobject: '%s': free name\n", name);
> 
> base-commit: 8632e9b5645bbc2331d21d892b0d6961c1a08429
> 

Thanks.
-- 
~Randy