Re: [PATCH V2 1/2] audit: stop an old auditd being starved out by a new auditd
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.