public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	majianpeng <majianpeng@gmail.com>
Subject: Re: Possible memory leaks in proc_sysctl.c
Date: Wed, 18 Apr 2012 17:14:35 +0100	[thread overview]
Message-ID: <20120418161435.GJ1505@arm.com> (raw)
In-Reply-To: <m1ehrloxic.fsf@fess.ebiederm.org>

On Wed, Apr 18, 2012 at 04:44:27PM +0100, Eric W. Biederman wrote:
> Catalin Marinas <catalin.marinas@arm.com> writes:
> > On Wed, Apr 18, 2012 at 03:52:58PM +0100, Eric W. Biederman wrote:
> >> So simply saying kmemleak shut up this is deliberate in these few cases
> >> where we don't intend to unregister the structure and have a deliberate
> >> leak seems the clean and maintainable way to go.
> >
> > OK, I got it now, sounds fair. But please add a comment to the
> > kmemleak_not_leak() annotations so that others know it's a deliberate
> > leak (rather than a false positive).
> >
> > Also the patch should include the linux/kmemleak.h file for the
> > kmemleak_not_leak() prototype as header changes in the future may cause
> > problems (I think the one you posted did not include it).
> 
> I will take a look when I merge the patch.
> 
> Would something like kmemleak_ignore() be better?  What I want is
> kmemleak_this_is_a_deliberate_leak_so_shut_up(), but the API doesn't
> seem to exactly include that function.  I'm not certain what the proper
> name is as I haven't worked much with kmemleak.

kmemleak_ignore() can be used as long as the object does not reference
other objects since it won't even be scanned. In this case, it only
hides the returned header object but there seem to be two more objects
allocated during the register_sysctl_table() call and referenced from
the returned header.

kmemleak_not_leak() would do the right thing here, though it was meant
for annotating false positives rather than real leaks. I think a comment
on the calling site would be enough.

-- 
Catalin

      reply	other threads:[~2012-04-18 16:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-18 10:12 Possible memory leaks in proc_sysctl.c Catalin Marinas
2012-04-18 13:22 ` Eric W. Biederman
2012-04-18 14:18   ` Catalin Marinas
2012-04-18 14:52     ` Eric W. Biederman
2012-04-18 15:15       ` Catalin Marinas
2012-04-18 15:44         ` Eric W. Biederman
2012-04-18 16:14           ` Catalin Marinas [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=20120418161435.GJ1505@arm.com \
    --to=catalin.marinas@arm.com \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=majianpeng@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox