From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Petr Mladek <pmladek@suse.com>
Cc: David Vernet <void@manifault.com>,
Miroslav Benes <mbenes@suse.cz>,
linux-doc@vger.kernel.org, live-patching@vger.kernel.org,
linux-kernel@vger.kernel.org, jpoimboe@redhat.com,
jikos@kernel.org, joe.lawrence@redhat.com, corbet@lwn.net,
yhs@fb.com, songliubraving@fb.com
Subject: Re: [PATCH] livepatch: Fix leak on klp_init_patch_early failure path
Date: Wed, 15 Dec 2021 16:35:24 +0100 [thread overview]
Message-ID: <YboLPAmOc8/6khu2@kroah.com> (raw)
In-Reply-To: <YbmlL0ZyfSuek9OB@alley>
On Wed, Dec 15, 2021 at 09:19:59AM +0100, Petr Mladek wrote:
> On Tue 2021-12-14 16:50:15, Greg Kroah-Hartman wrote:
> >
> > kobject_init() does allocate things internally, where does it say it
> > does not? What is trying to be "fixed" here?
>
> Could you please show where things are allocated in kobject_init()?
> I do not see it in the code!
>
> It looks to me like a cargo cult claim to me.
Hm, I thought I saw it yesterday when I reviewed the code. Let me look
again...
> Documentation/core-api/kobject.rst says:
>
> Once you registered your kobject via kobject_add(), you must never use
> kfree() to free it directly. The only safe way is to use kobject_put().
>
> kobject_add() makes perfect sense because it copies the name, takes
> reference to the parent, etc.
>
> kobject_init() just initializes the structure members and nothing else.
Now it does. In the past, I think we did create some memory. I know
when we hook debugobjects up to kobjects (there's an external patch for
that floating around somewhere), that is one reason to keep the
kobject_put() rule, and there might have been other reasons in the past
20+ years as well.
So yes, while you are correct today, the "normal" reference counted
object model patern is "after the object is initialized, it MUST only be
freed by handling its reference count." So let's stick to that rule for
now.
If you want, I can put some code in the kobject_init() logic to force
this to be the case if it bothers you :)
thanks,
greg k-h
next prev parent reply other threads:[~2021-12-15 16:01 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-13 19:17 [PATCH] livepatch: Fix leak on klp_init_patch_early failure path David Vernet
2021-12-13 20:10 ` Josh Poimboeuf
2021-12-13 22:58 ` David Vernet
2021-12-13 23:32 ` Song Liu
2021-12-14 3:13 ` Josh Poimboeuf
2021-12-14 8:45 ` Petr Mladek
2021-12-14 9:17 ` Miroslav Benes
2021-12-14 15:26 ` David Vernet
2021-12-14 15:50 ` Greg Kroah-Hartman
2021-12-15 8:19 ` Petr Mladek
2021-12-15 15:35 ` Greg Kroah-Hartman [this message]
2021-12-16 14:14 ` Petr Mladek
2021-12-16 14:35 ` Greg Kroah-Hartman
2021-12-17 13:10 ` Petr Mladek
2021-12-16 15:14 ` David Vernet
2021-12-17 13:53 ` Petr Mladek
2021-12-15 8:33 ` Miroslav Benes
2021-12-14 15:52 ` Greg Kroah-Hartman
2021-12-14 18:26 ` David Vernet
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=YboLPAmOc8/6khu2@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=corbet@lwn.net \
--cc=jikos@kernel.org \
--cc=joe.lawrence@redhat.com \
--cc=jpoimboe@redhat.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=live-patching@vger.kernel.org \
--cc=mbenes@suse.cz \
--cc=pmladek@suse.com \
--cc=songliubraving@fb.com \
--cc=void@manifault.com \
--cc=yhs@fb.com \
/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.