Re: [PATCH 1/2] audit: stop an old auditd being starved out by a new auditd

[Paul Moore <pmoore@redhat.com>]

On Tuesday, September 29, 2015 12:36:11 AM Richard Guy Briggs wrote:
> On 15/09/28, Paul Moore wrote:
> > On Monday, September 28, 2015 07:17:31 AM Richard Guy Briggs wrote:

...

> > 
> > > So, I believ audit_make_reply() can be used just fine, setting portid,
> > > seq, done and multi to zero.
> > 
> > The 'multi' flag should definitely be set to zero, 'seq' is fine at zero,
> > but I think we can do better with 'portid'; we know the 'portid' value so
> > just use it in the call to audit_make_reply().
> 
> Most other audit_log_start() created messages set portid to zero except
> user messages, and those are set using the initiating process' portid
> and not the destination id.

Sorry, I was confusing the portid in sockaddr_nl with the portid in the 
nlmsghdr ... anyway, yes, the portid in the netlink header should always be 
zero since it references the sender and not the destination and the kernel has 
portid 0.

> > I don't like that we are reusing audit_make_reply() for non-reply netlink
> > messages, but I'll get over that.  This will likely get a revamp when we
> > get around to a proper fix of the queuing system.
> 
> This could even be renamed audit_make_message() and possibly be
> generalized to be useful to audit_log_start(), or rather
> audit_buffer_alloc().  Later...

Not exactly what I was thinking, but as I said, not worth worrying about right 
now.

> > > Ok, how about AUDIT_HIJACK_TEST, with a payload of the u32
> > > representation of the PID of the task attempting to replace it.
> > 
> > Why add the TEST?  It is a hijack attempt, or at least it is if the record
> > is emitted successfully :)  I would go simply with AUDIT_HIJACK or maybe
> > AUDIT_REPLACE (or similar) if "hijack" is a bit too inflammatory (it
> > probably is ...).
> 
> I had actually named it AUDIT_REPLACE_TEST, but your repeated use of the
> term "hijack" swayed me...  I'd still lean towards *_TEST since it is
> testing to replace a stale socket and not a live one.

While we are using the record for a test, that is not its only purpose and we 
might arrive at a future need to indicate a "hijack" that isn't a test.  Keep 
the "hijack" if you want, remove the "test".

-- 
paul moore
security @ redhat