From: Steve Grubb <sgrubb@redhat.com>
To: Paul Moore <pmoore@redhat.com>
Cc: Richard Guy Briggs <rgb@redhat.com>, linux-audit@redhat.com,
        linux-kernel@vger.kernel.org, eparis@redhat.com, v.rathor@gmail.com,
        ctcard@hotmail.com
Subject: Re: [PATCH V2 1/2] audit: stop an old auditd being starved out by a new auditd
Date: Mon, 21 Dec 2015 17:18:15 -0500
Message-ID: <3132444.Y0z3o3Cmva@x2>
Organization: Red Hat
User-Agent: KMail/4.14.10 (Linux/4.2.3-203.fc22.x86_64; KDE/4.14.14; x86_64; ; )
In-Reply-To: <1831194.c6xgxYyxjn@sifl>
References: <735d85abf2484b740fd3aa551cdffde67f9bd637.1450268948.git.rgb@redhat.com> <2544274.X8ZVJd57ai@x2> <1831194.c6xgxYyxjn@sifl>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3157
Lines: 76

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.

--
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/