Re: [GIT PULL] audit patches

[Andrew Morton]

> On Thu, 22 Feb 2007 08:22:47 -0500 Stephen Smalley <sds@xxxxxxxxxxxxx> 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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/