All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Tobin C. Harding" <me@tobin.cc>
To: "Rafael J. Wysocki" <rafael@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Petr Mladek <pmladek@suse.com>, Miroslav Benes <mbenes@suse.cz>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: kobject_init_and_add() confusion
Date: Wed, 1 May 2019 09:38:03 +1000	[thread overview]
Message-ID: <20190430233803.GB10777@eros.localdomain> (raw)

Hi,

Looks like I've created a bit of confusion trying to fix memleaks in
calls to kobject_init_and_add().  Its spread over various patches and
mailing lists so I'm starting a new thread and CC'ing anyone that
commented on one of those patches.

If there is a better way to go about this discussion please do tell me.

The problem
-----------

Calls to kobject_init_and_add() are leaking memory throughout the kernel
because of how the error paths are handled.

The solution
------------

Write the error path code correctly.

Example
-------

We have samples/kobject/kobject-example.c but it uses
kobject_create_and_add().  I thought of adding another example file here
but could not think of how to do it off the top of my head without being
super contrived.  Can add this to the TODO list if it will help.

Here is an attempted canonical usage of kobject_init_and_add() typical
of the code that currently is getting it wrong.  This is the second time
I've written this and the first time it was wrong even after review (you
know who you are, you are definitely buying the next round of drinks :)


Assumes we have an object in memory already that has the kobject
embedded in it. Variable 'kobj' below would typically be &ptr->kobj


	void fn(void)
	{
	        int ret;

	        ret = kobject_init_and_add(kobj, ktype, NULL, "foo");
	        if (ret) {
			/*
			 * This means kobject_init() has succeeded
			 * but kobject_add() failed.
			 */
			goto err_put;
		}

	        ret = some_init_fn();
	        if (ret) {
			/*
			 * We need to wind back kobject_add() AND kobject_put().
			 * kobject_add() incremented the refcount in
			 * kobj->parent, that needs to be decremented THEN we need
			 * the call to kobject_put() to decrement the refcount of kobj.
			 */
			goto err_del;
		}

	        ret = some_other_init_fn();
	        if (ret)
	                goto other_err;

	        kobject_uevent(kobj, KOBJ_ADD);
	        return 0;

	other_err:
	        other_clean_up_fn();
	err_del:
	        kobject_del(kobj);
	err_put:
		kobject_put(kobj);

	        return ret;
	}


Have I got this correct?

TODO
----

- Fix all the callsites to kobject_init_and_add()
- Further clarify the function docstring for kobject_init_and_add() [perhaps]
- Add a section to Documentation/kobject.txt [optional]
- Add a sample usage file under samples/kobject [optional]


Thanks,
Tobin.

             reply	other threads:[~2019-04-30 23:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-30 23:38 Tobin C. Harding [this message]
2019-05-01  7:54 ` kobject_init_and_add() confusion Rafael J. Wysocki
2019-05-01 21:44   ` Tobin C. Harding
2019-05-10  2:35   ` Tobin C. Harding
2019-05-10  7:52     ` Rafael J. Wysocki
2019-05-10  9:40     ` Petr Mladek
2019-05-11  6:32       ` Tobin C. Harding
2019-05-01 11:10 ` Greg Kroah-Hartman
2019-05-01 21:58   ` Tobin C. Harding
2019-05-02  7:19     ` Greg Kroah-Hartman
2019-05-02  8:34 ` Petr Mladek
2019-05-02  9:06   ` Greg Kroah-Hartman
2019-05-03  1:16   ` Tobin C. Harding
2019-05-03  7:56     ` Petr Mladek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190430233803.GB10777@eros.localdomain \
    --to=me@tobin.cc \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=pmladek@suse.com \
    --cc=rafael@kernel.org \
    --cc=tyreld@linux.vnet.ibm.com \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.