LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Stephen Smalley <sds@tycho.nsa.gov>
Cc: viro@ftp.linux.org.uk, torvalds@linux-foundation.org,
	linux-kernel@vger.kernel.org, sgrubb@redhat.com,
	jmorris@namei.org
Subject: Re: [GIT PULL] audit patches
Date: Thu, 22 Feb 2007 13:19:31 -0800	[thread overview]
Message-ID: <20070222131931.1ed6ffa4.akpm@linux-foundation.org> (raw)
In-Reply-To: <1172150567.14363.337.camel@moss-spartans.epoch.ncsc.mil>

> On Thu, 22 Feb 2007 08:22:47 -0500 Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Wed, 2007-02-21 at 16:03 -0800, Andrew Morton wrote:
> > 
> > Looking at the changes to audit_receive_msg():
> > 
> > 
> > 				if (sid) {
> > 					if (selinux_sid_to_string(
> > 							sid, &ctx, &len)) {
> > 						audit_log_format(ab,
> > 							" ssid=%u", sid);
> > 						/* Maybe call audit_panic? */
> > 					} else
> > 						audit_log_format(ab,
> > 							" subj=%s", ctx);
> > 					kfree(ctx);
> > 				}
> > 
> > This is assuming that selinux_sid_to_string() always initialises `ctx'.
> > 
> > But AFAICT there are two error paths in security_sid_to_context() which
> > forget to do that, so we end up doing kfree(uninitialised-local).
> > 
> > I'd consider that a shortcoming in security_sid_to_context(), so not a
> > problem in this patch, as long as people agree with my blaming above.
> 
> I wouldn't assume that the function initializes an argument if it
> returns an error, and at least some of the callers (in auditsc.c) appear
> to correctly initialize ctx to NULL themselves before calling
> selinux_sid_to_string().  But if you'd prefer the function to always
> handle it, we can do that.
> 

Well we now have (at least) one caller which assumes that *ctx is
initialied in error cases.

And I think it's sane to make it do that: safer, and will simplify coding
in the callers.

  reply	other threads:[~2007-02-22 21:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-18  4:01 Al Viro
2007-02-22  0:03 ` Andrew Morton
2007-02-22 13:22   ` Stephen Smalley
2007-02-22 21:19     ` Andrew Morton [this message]
2007-02-23 14:00       ` [patch 1/1] selinux: always initialize arguments to security_sid_to_context (Was: Re: [GIT PULL] audit patches) Stephen Smalley
2008-04-29 18:08 [git pull] audit patches Al Viro
2009-04-05 19:17 Al Viro

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=20070222131931.1ed6ffa4.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=jmorris@namei.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sds@tycho.nsa.gov \
    --cc=sgrubb@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ftp.linux.org.uk \
    --subject='Re: [GIT PULL] audit patches' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).