Re: [PATCH] TaskTracker : Simplified thread information tracker.

[Richard Guy Briggs <rgb@redhat.com>]

On 15/01/04, Tetsuo Handa wrote:
> Hello.
> 
> Richard Guy Briggs wrote:
> > > Richard Guy Briggs wrote:
> > > > On 14/09/28, Tetsuo Handa wrote:
> > > > > (Q2) Does auxiliary record work with only type=SYSCALL case?
> > > > 
> > > > Auxiliary records don't work with AUDIT_LOGIN because that record has a
> > > > NULL context.  Similarly for core dumps (AUDIT_ANOM_ABEND), AUDIT_SECCOMP,
> > > > configuration changes (AUDIT_CONFIG_CHANGE, AUDIT_FEATURE_CHANGE), most
> > > > (all?) AUDIT_USER_* messages.
> > > > 
> > > I see, thank you.
> > > 
> > > Although I feel that, from the point of view of troubleshooting, emitting
> > > history of thread's comm name into NULL-context records would help sysadmin
> > > to map login session and operations a user did from that login session,
> > > I'm OK with starting history of thread's comm name as auxiliary records
> > > (i.e. not emitted into NULL-context records).
> > > 
> > > Adding LKML for reviewers. What else can I do for merging this patch?
> > 
> > I'm willing to take it with some reflection and no significant
> > objections, in particular from userspace audit.  I'll have a closer look
> > at it.
> 
> Any comments on this patch?

Steve already mentioned any user-influenced fields need to be escaped,
so I'd recommend audit_log_untrustedstring() as being much simpler from
your perspective and much better tested and understood from audit
maintainer's perspective.  At least use the existing 'o' printf format
specifier instead of inventing your own.  I do acknowledge that the
resulting output from your function is easier to read in its raw format
passed from the kernel, however, it makes your code harder to maintain.

As for the date-stamping bits, they seem to be the majority of the code
in audit_update_history().  I'd just emit a number and punt that to
userspace for decoding.  Alternatively, I'd use an existing service in
the kernel to do that date formatting, or at least call a new function
to format that date string should a suitable one not already exist, so
you can remove that complexity from audit_update_history().

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545