Messages in this thread Patch in this message | | | Subject | [patch 1/1] selinux: always initialize arguments to security_sid_to_context (Was: Re: [GIT PULL] audit patches) | From | Stephen Smalley <> | Date | Fri, 23 Feb 2007 09:00:18 -0500 |
| |
On Thu, 2007-02-22 at 13:19 -0800, Andrew Morton wrote: > > 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.
Ok, patch below.
Always initialize *scontext and *scontext_len in security_sid_to_context.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |