From: Nicholas Mc Guire <der.herr@hofr.at>
To: Petr Mladek <pmladek@suse.com>
Cc: Nicholas Mc Guire <hofrat@osadl.org>,
Josh Poimboeuf <jpoimboe@redhat.com>,
Jessica Yu <jeyu@kernel.org>, Jiri Kosina <jikos@kernel.org>,
Miroslav Benes <mbenes@suse.cz>,
live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] livepatch: handle kzalloc failure
Date: Thu, 13 Dec 2018 15:00:58 +0100 [thread overview]
Message-ID: <20181213140057.GA29272@osadl.at> (raw)
In-Reply-To: <20181213123139.oukmwpkw2yfap6oa@pathway.suse.cz>
On Thu, Dec 13, 2018 at 01:31:39PM +0100, Petr Mladek wrote:
> On Thu 2018-12-13 12:09:49, Nicholas Mc Guire wrote:
> > kzalloc() return should always be checked - notably in example code
> > where this may be seen as reference. On failure of allocation
> > livepatch_fix1_dummy_alloc() should return NULL.
> >
> > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> > ---
> >
> > Problem was located with an experimental coccinelle script
> >
> > Patch was compile tested with: x86_64_defconfig + FTRACE=y
> > FUNCTION_TRACER=y, EXPERT=y, LATENCYTOP=y, SAMPLES=y, SAMPLE_LIVEPATCH=y
> > (with some unrelated sparse warnings on symbols not being static)
> >
> > Patch is against 4.20-rc6 (localversion-next is next-20181213)
> >
> > samples/livepatch/livepatch-shadow-fix1.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/samples/livepatch/livepatch-shadow-fix1.c b/samples/livepatch/livepatch-shadow-fix1.c
> > index 49b1355..a0e8f04 100644
> > --- a/samples/livepatch/livepatch-shadow-fix1.c
> > +++ b/samples/livepatch/livepatch-shadow-fix1.c
> > @@ -89,6 +89,9 @@ struct dummy *livepatch_fix1_dummy_alloc(void)
> > * pointer to handle resource release.
> > */
> > leak = kzalloc(sizeof(int), GFP_KERNEL);
> > + if (!leak)
> > + return NULL;
>
> It should be:
>
> if (!leak) {
> kfree(d);
> return NULL;
> }
>
> Note that The check is not strictly needed in this artificial
> example because we never read/write any data there. But I agree
> that we should add the check to promote the the right programming
> patterns.
>
thanks for catching this !
will send a V2.
thx!
hofrat
prev parent reply other threads:[~2018-12-13 14:01 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-13 11:09 [PATCH 1/2] livepatch: handle kzalloc failure Nicholas Mc Guire
2018-12-13 11:09 ` [PATCH 2/2] " Nicholas Mc Guire
2018-12-13 12:33 ` Petr Mladek
2018-12-13 12:31 ` [PATCH 1/2] " Petr Mladek
2018-12-13 14:00 ` Nicholas Mc Guire [this message]
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=20181213140057.GA29272@osadl.at \
--to=der.herr@hofr.at \
--cc=hofrat@osadl.org \
--cc=jeyu@kernel.org \
--cc=jikos@kernel.org \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=live-patching@vger.kernel.org \
--cc=mbenes@suse.cz \
--cc=pmladek@suse.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.