From mboxrd@z Thu Jan 1 00:00:00 1970 From: Richard Guy Briggs Subject: Re: [PATCH V1] audit: add warning that an old auditd may be starved out by a new auditd Date: Wed, 9 Sep 2015 02:31:54 -0400 Message-ID: <20150909063154.GL8140@madcap2.tricolour.ca> References: <5e786f07b6d8a19927c70345c14bd1a452164d38.1441644314.git.rgb@redhat.com> <1441724246.27892.89.camel@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1441724246.27892.89.camel@redhat.com> Sender: linux-kernel-owner@vger.kernel.org To: Eric Paris Cc: linux-audit@redhat.com, linux-kernel@vger.kernel.org, sgrubb@redhat.com, pmoore@redhat.com, v.rathor@gmail.com, ctcard@hotmail.com List-Id: linux-audit@redhat.com On 15/09/08, Eric Paris wrote: > This is already going to be in the audit log, right? We're going to > send a CONFIG_CHANGE record with old_pid == the existing auditd. I bet > it gets delivered to the old auditd. Actually, delivered by the new auditd is what I'm seeing... (Tested by running "auditd -f" to have it show up in the debug output and not in the log file.) I did see it once as two seperate messages, one setting to zero and the next to the new PID, but that may have been a testing procedure error... (Note: Why does auditd run in a terminal with -n or -f not respond to ^C?) > But why is this a printk(KERN_WARN) ? Because we're still trying to figure out what is going on... But point taken, we already should have that information. If it is listed as a warning it has a better chance of getting reported. What do you suggest instead? Eric, thanks for taking the time to review this... > On Mon, 2015-09-07 at 12:48 -0400, 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. > > > > There isn't an easy way to detect if an old auditd is still running on > > the existing audit_pid other than attempting to send a message to see if > > it fails. If no message to auditd has been attempted since auditd died > > unnaturally or got killed, audit_pid will still indicate it is alive. > > > > Signed-off-by: Richard Guy Briggs > > --- > > Note: Would it be too bold to actually block the registration of a new > > auditd if the netlink_getsockbyportid() call succeeded? Would other > > checks be appropriate? > > > > kernel/audit.c | 5 +++++ > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > diff --git a/kernel/audit.c b/kernel/audit.c > > index 18cdfe2..1fa1e0d 100644 > > --- a/kernel/audit.c > > +++ b/kernel/audit.c > > @@ -872,6 +872,11 @@ static int audit_receive_msg(struct sk_buff > > *skb, struct nlmsghdr *nlh) > > if (s.mask & AUDIT_STATUS_PID) { > > int new_pid = s.pid; > > > > + if (audit_pid && new_pid && > > + !IS_ERR(netlink_getsockbyportid(audit_sock, audit_nlk_portid))) > > + pr_warn("auditd replaced by new auditd before normal shutdown: " > > + "(old)audit_pid=%d (by)pid=%d new_pid=%d", > > + audit_pid, pid, new_pid); > > if ((!new_pid) && (task_tgid_vnr(current) != audit_pid)) > > return -EACCES; > > if (audit_enabled != AUDIT_OFF) - RGB -- Richard Guy Briggs 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