public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: John Stultz <john.stultz@linaro.org>
To: Colin Cross <ccross@android.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	Android Kernel Team <kernel-team@android.com>,
	kbuild test robot <fengguang.wu@intel.com>
Subject: Re: [RFC][PATCH 2/3] staging: ion: Fix possible null pointer dereference
Date: Mon, 16 Dec 2013 16:37:11 -0800	[thread overview]
Message-ID: <52AF9CB7.90003@linaro.org> (raw)
In-Reply-To: <CAMbhsRRMMMreYzLMZHxjrRrKj0rZdS5SjLNbpGUuW93mg+Q65w@mail.gmail.com>

On 12/16/2013 04:26 PM, Colin Cross wrote:
> On Mon, Dec 16, 2013 at 1:32 PM, John Stultz <john.stultz@linaro.org> wrote:
>> The kbuild test robot reported:
>>
>> drivers/staging/android/ion/ion_system_heap.c:122 alloc_largest_available() error: potential null dereference 'info'.  (kmalloc returns null)
>>
>> Where the pointer returned from kmalloc goes unchecked for failure.
>>
>> This patch adds a simple check for a null return, and handles the error.
>>
>> XXX: Not sure if continue or 'return NULL' is the right thing to do.
> Returning NULL is fine, there is no reason the kmalloc is going to
> succeed if it retries, and it will just have to allocate more of them
> if it starts fulfilling the allocation with smaller order chunks.
>
> Allocating the struct before entering the loop might make error handling easier.

Ok, I had actually done that in my first pass, but then got worried the
allocation/free might have extra overhead in the case where the pool is
exhausted, so I stuck with the existing pattern. But if that's not a
common state, then I agree, it does make the handling simpler.

I'll rework this and re-submit. Thanks for the feedback!

thanks
-john

  reply	other threads:[~2013-12-17  0:37 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-16 21:32 [RFC][PATCH 0/3] ION fixups for staging-next John Stultz
2013-12-16 21:32 ` [RFC][PATCH 1/3] staging: ion: Add HAVE_MEMBLOCK config dependency John Stultz
2013-12-16 21:32 ` [RFC][PATCH 2/3] staging: ion: Fix possible null pointer dereference John Stultz
2013-12-16 21:40   ` Greg KH
2013-12-17  0:26   ` Colin Cross
2013-12-17  0:37     ` John Stultz [this message]
2013-12-16 21:32 ` [RFC][PATCH 3/3] staging: ion: Avoid using rt_mutexes directly John Stultz
2013-12-17  0:17   ` Colin Cross
2013-12-17  1:22     ` John Stultz
2013-12-17  1:34       ` Colin Cross

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=52AF9CB7.90003@linaro.org \
    --to=john.stultz@linaro.org \
    --cc=ccross@android.com \
    --cc=fengguang.wu@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@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