From: Jesse Barnes <jbarnes@virtuousgeek.org>
To: Jiri Slaby <jirislaby@gmail.com>
Cc: Jiri Slaby <jslaby@suse.cz>,
mjg@redhat.com, platform-driver-x86@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] X86: intel_ips, check for kzalloc properly
Date: Mon, 21 Jun 2010 08:34:18 -0700 [thread overview]
Message-ID: <20100621083418.6315d1d5@virtuousgeek.org> (raw)
In-Reply-To: <4C1F865C.1070707@gmail.com>
On Mon, 21 Jun 2010 17:33:48 +0200
Jiri Slaby <jirislaby@gmail.com> wrote:
> On 06/21/2010 05:25 PM, Jesse Barnes wrote:
> > On Mon, 21 Jun 2010 17:02:11 +0200
> > Jiri Slaby <jslaby@suse.cz> wrote:
> >
> >> Stanse found that there are two NULL checks missing in ips_monitor. So
> >> check their value too and bail out appropriately if the allocation
> >> failed.
> >>
> >> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> >> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> >> Cc: Matthew Garrett <mjg@redhat.com>
> >> ---
> >> drivers/platform/x86/intel_ips.c | 3 ++-
> >> 1 files changed, 2 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/intel_ips.c b/drivers/platform/x86/intel_ips.c
> >> index cdaf40e..3c7ea9a 100644
> >> --- a/drivers/platform/x86/intel_ips.c
> >> +++ b/drivers/platform/x86/intel_ips.c
> >> @@ -931,7 +931,8 @@ static int ips_monitor(void *data)
> >> mch_samples = kzalloc(sizeof(u16) * IPS_SAMPLE_COUNT, GFP_KERNEL);
> >> cpu_samples = kzalloc(sizeof(u32) * IPS_SAMPLE_COUNT, GFP_KERNEL);
> >> mchp_samples = kzalloc(sizeof(u32) * IPS_SAMPLE_COUNT, GFP_KERNEL);
> >> - if (!mcp_samples || !ctv1_samples || !ctv2_samples || !mch_samples) {
> >> + if (!mcp_samples || !ctv1_samples || !ctv2_samples || !mch_samples ||
> >> + !cpu_samples || !mchp_samples) {
> >> dev_err(&ips->dev->dev,
> >> "failed to allocate sample array, ips disabled\n");
> >> kfree(mcp_samples);
> >
> > Ah cool, am I also missing the appropriate kfree() calls for the last
> > two? The context doesn't have it. Otherwise,
>
> I checked when I was patching that and the last is not freed there.
> However I didn't add it since at least one must be non-NULL and in this
> case it can be only the last. (Until somebody add another allocation
> indeed.) So should I add even the last free there?
Up to Matthew. Not adding it will make people do a double take, but I
agree it'll be safe.
--
Jesse Barnes, Intel Open Source Technology Center
next prev parent reply other threads:[~2010-06-21 15:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-06-21 15:02 [PATCH] X86: intel_ips, check for kzalloc properly Jiri Slaby
2010-06-21 15:25 ` Jesse Barnes
2010-06-21 15:33 ` Jiri Slaby
2010-06-21 15:34 ` Jesse Barnes [this message]
2010-06-21 15:40 ` Jiri Slaby
2010-06-21 16:58 ` Jesse Barnes
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=20100621083418.6315d1d5@virtuousgeek.org \
--to=jbarnes@virtuousgeek.org \
--cc=jirislaby@gmail.com \
--cc=jslaby@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=mjg@redhat.com \
--cc=platform-driver-x86@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox