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: Sun, 13 Sep 2015 12:08:19 -0400 Message-ID: <20150913160819.GT8140@madcap2.tricolour.ca> References: <5e786f07b6d8a19927c70345c14bd1a452164d38.1441644314.git.rgb@redhat.com> <20150907165818.GH8140@madcap2.tricolour.ca> <4246819.OGLW0CmS4i@sifl> <20150911102114.GP8140@madcap2.tricolour.ca> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Paul Moore Cc: Paul Moore , v.rathor@gmail.com, linux-kernel@vger.kernel.org, linux-audit@redhat.com List-Id: linux-audit@redhat.com On 15/09/11, Paul Moore wrote: > On Fri, Sep 11, 2015 at 6:21 AM, Richard Guy Briggs wrote: > > On 15/09/09, Paul Moore wrote: > >> On Monday, September 07, 2015 12:58:18 PM Richard Guy Briggs wrote: > >> > On 15/09/07, 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 > >> > > >> > Ok, self-nack on this one for a couple of problems... > >> > netlink_getsockbyportid() is static to af_netlink.c and "pid" should be > >> > task_tgid_vnr(current). Otherwise, any opinions on this approach? > >> > > >> > > --- > >> > > 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? > >> > >> Hmm. It seems like we should prevent the registration of a new auditd if we > >> already have an auditd instance connected, although as you say, that isn't the > >> easiest thing to do. > > > > I wanted to do that, but I feared it would carry some risk that an > > intentional act would be blocked. In that case, an intentional act > > could include explicitly killing the old auditd (or process registered > > as such). > > Well, if you are running another instance of auditd you should have > the right level of access to kill an older, existing auditd instance > so I'm not too worried about preventing an intentional act. Also, if > we check an existing connection with some sort of heartbeat/ping > message we wouldn't be preventing it for more than a few times I > believe. Agreed. > >> How painful would it be to return -EAGAIN to the new auditd while sending some > >> sort of keep-alive/ping/etc. message to the old daemon to check its status? > > > > Well, if it turns out that the only reason it ever fails is > > -ECONNREFUSED, then we just need to check with netlink_getsockbyportid() > > to see if it fails before accepting the new auditd. > > I'm not sure if the netdev crowd would be interested in exporting > _getsockbyportid(); for the sake of discussion let's assume no changes > to the netlink layer, if we get stuck we can revisit this idea. Agreed. > > If it is one of the others, can we put the new auditd task on a wait > > queue until we hear back one way or the other or just timeout on > > contacting the old auditd? > > Well, what if we don't have anything queued for the old auditd? Then it won't notice until something does get queued. That would be the purpose of sending a ping. > Although I suppose if nothing else we could send a record indicating > that another auditd attempted to replace it ... if we can send it > great, drop the new request and be glad we audited it, if we can't > send it, reset the auditd tracking. This is actually a good idea. > > I'll let Steve speak to dealing with -EAGAIN. auditlib already deals > > with -EAGAIN and -EINTR for some cases. I have a patch that added > > -ENOBUFS to those cases since I had seen some reports that -ENOBUFS had > > been returned in some cases (don't remember the circumstances). > > paul moore - 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