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

[Steve Grubb <sgrubb@redhat.com>]

On Monday, December 21, 2015 04:48:00 PM Paul Moore wrote:
> On Wednesday, December 16, 2015 11:23:19 AM Steve Grubb wrote:
> > On Wednesday, December 16, 2015 10:42:32 AM Richard Guy Briggs wrote:
> > > Nothing prevents a new auditd starting up and replacing a valid
> > > audit_pid when an old auditd is still running, effectively starving out
> > > the old auditd since audit_pid no longer points to the old valid auditd.
> > 
> > I guess the first question is why do we allow something to start up a new
> > auditd without killing off the old one?  Would that be a simpler fix?
> 
> I imagine there might be scenarios where you need to forcibly kill an
> instance of auditd such that things might not get fully cleaned up in the
> kernel, audit_{pid,sock,etc.}. 

But the first time an event is sent and auditd doesn't exist, it resets the 
audit_pid to 0.

static void kauditd_send_skb(struct sk_buff *skb)
{
        int err;
        /* take a reference in case we can't send it and we want to hold it */
        skb_get(skb);
        err = netlink_unicast(audit_sock, skb, audit_nlk_portid, 0);
        if (err < 0) {
                BUG_ON(err != -ECONNREFUSED); /* Shouldn't happen */
                if (audit_pid) {
                        pr_err("*NO* daemon at audit_pid=%d\n", audit_pid);
                        audit_log_lost("auditd disappeared");
                        audit_pid = 0;
                        audit_sock = NULL;
                }



> Keeping the ability to reset the kernel's auditd state, even when the kernel
> *thinks* auditd is still alive might be a nice thing to keep around for a
> while longer.

I'm just thinking its rare that anyone would try to steal away the audit 
socket. Its more work for everyone to create a new event and send it than to 
just not allow it. you can even force an event with "auditctl -m test" which 
should reset the pid if the kernel was out of sync.

The 

 
> > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > > index 843540c..cf84991 100644
> > > --- a/include/uapi/linux/audit.h
> > > +++ b/include/uapi/linux/audit.h
> > > @@ -70,6 +70,7 @@
> > > 
> > >  #define AUDIT_TTY_SET		1017	/* Set TTY auditing status */
> > >  #define AUDIT_SET_FEATURE	1018	/* Turn an audit feature on or off 
*/
> > >  #define AUDIT_GET_FEATURE	1019	/* Get which features are enabled 
*/
> > > 
> > > +#define AUDIT_REPLACE		1020	/* Replace auditd if this pack... 
*/
> > 
> > In every case, events in the 1000 block are to configure the kernel or to
> > ask the kernel a question. This is user space initiating. Kernel
> > initiating
> > events are in the 1300 block of numbers.
> 
> Change the audit event number as Steve suggests and I'll toss the patches
> into my audit next queue, although considering we are at 4.4-rc6 at
> present, I'll probably hold this until after the merge window closes,
> meaning it is 4.6 material.