[RFC PATCH] audit: fix auditd/kernel connection state tracking

[Paul Moore]

On Tue, Mar 21, 2017 at 12:23 PM, Paul Moore <pmoore at redhat.com> wrote:
> From: Paul Moore <paul at paul-moore.com>
>
> What started as a rather straightforward race condition reported by
> Dmitry using the syzkaller fuzzer ended up revealing some major
> problems with how the audit subsystem managed its netlink sockets and
> its connection with the userspace audit daemon.  Fixing this properly
> had quite the cascading effect and what we are left with is this rather
> large and complicated patch.  My initial goal was to try and decompose
> this patch into multiple smaller patches, but the way these changes
> are intertwined makes it difficult to split these changes into
> meaningful pieces that don't break or somehow make things worse for
> the intermediate states.
>
> The patch makes a number of changes, but the most significant are
> highlighted below:
>
> * The auditd tracking variables, e.g. audit_sock, are now gone and
> replaced by a RCU/spin_lock protected variable auditd_conn which is
> a structure containing all of the auditd tracking information.
>
> * We no longer track the auditd sock directly, instead we track it
> via the network namespace in which it resides and we use the audit
> socket associated with that namespace.  In spirit, this is what the
> code was trying to do prior to this patch (at least I think that is
> what the original authors intended), but it was done rather poorly
> and added a layer of obfuscation that only masked the underlying
> problems.
>
> * Big backlog queue cleanup, again.  In v4.10 we made some pretty big
> changes to how the audit backlog queues work, here we haven't changed
> the queue design so much as cleaned up the implementation.  Brought
> about by the locking changes, we've simplified kauditd_thread() quite
> a bit by consolidating the queue handling into a new helper function,
> kauditd_send_queue(), which allows us to eliminate a lot of very
> similar code and makes the looping logic in kauditd_thread() clearer.
>
> * All netlink messages sent to auditd are now sent via
> auditd_send_unicast_skb().  Other than just making sense, this makes
> the lock handling easier.
>
> * Change the audit_log_start() sleep behavior so that we never sleep
> on auditd events (unchanged) or if the caller is holding the
> audit_cmd_mutex (changed).  Previously we didn't sleep if the caller
> was auditd or if the message type fell between a certain range; the
> type check was a poor effort of doing what the cmd_mutex check now
> does.  Richard Guy Briggs originally proposed not sleeping the
> cmd_mutex owner several years ago but his patch wasn't acceptable
> at the time.  At least the idea lives on here.
>
> * A problem with the lost record counter has been resolved.  Steve
> Grubb and I both happened to notice this problem and according to
> some quick testing by Steve, this problem goes back quite some time.
> It's largely a harmless problem, although it may have left some
> careful sysadmins quite puzzled.
>
> Cc: <stable at vger.kernel.org> # 4.10.x-
> Reported-by: Dmitry Vyukov <dvyukov at google.com>
> Signed-off-by: Paul Moore <paul at paul-moore.com>
> ---
>  kernel/audit.c   |  639 +++++++++++++++++++++++++++++++++---------------------
>  kernel/audit.h   |    9 -
>  kernel/auditsc.c |    6 -
>  3 files changed, 399 insertions(+), 255 deletions(-)

For those of you not following the other threads where this has been
mentioned, my plan is to keep this as an RFC for a couple of days and
if nothing ugly appears I'll send it up to Linus as part of the
audit/stable-4.11 branch.  If you are on Fedora (Rawhide) and you want
to play with a pre-built kernel, I'm currently building a kernel here:

* https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/530366

There is a slightly older build (it lacks some small optimizations
present in this latest draft of the patch) which is available now:

* https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/529771

-- 
paul moore
www.paul-moore.com