On Thu, 2007-02-22 at 13:19 -0800, Andrew Morton wrote: > > On Thu, 22 Feb 2007 08:22:47 -0500 Stephen Smalley <[EMAIL PROTECTED]> > > 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.
Ok, patch below. Always initialize *scontext and *scontext_len in security_sid_to_context. Signed-off-by: Stephen Smalley <[EMAIL PROTECTED]> --- security/selinux/ss/services.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index ca9154d..1e52356 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -609,6 +609,9 @@ int security_sid_to_context(u32 sid, char **scontext, u32 *scontext_len) struct context *context; int rc = 0; + *scontext = NULL; + *scontext_len = 0; + if (!ss_initialized) { if (sid <= SECINITSID_NUM) { char *scontextp; -- Stephen Smalley National Security Agency - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/