On Thu, Jun 15, 2017 at 3:24 PM, Richard Guy Briggs <[email protected]> wrote: > On 2017-06-15 11:28, Paul Moore wrote: >> From: Paul Moore <[email protected]> >> >> Originally reported by Adam and Dusty, it appears we have a small >> race window in kauditd_thread(), as documented in the Fedora BZ: >> >> * https://bugzilla.redhat.com/show_bug.cgi?id=1459326#c35 >> >> "This issue is partly due to the read-copy nature of RCU, and >> partly due to how we sync the auditd_connection state across >> kauditd_thread and the audit control channel. The kauditd_thread >> thread is always running so it can service the record queues and >> emit the multicast messages, if it happens to be just past the >> "main_queue" label, but before the "if (sk == NULL || ...)" >> if-statement which calls auditd_reset() when the new auditd >> connection is registered it could end up resetting the auditd >> connection, regardless of if it is valid or not. This is a rather >> small window and the variable nature of multi-core scheduling >> explains why this is proving rather difficult to reproduce." >> >> The fix is to have functions only call auditd_reset() when they >> believe that the kernel/auditd connection is still valid, e.g. >> non-NULL, and to have these callers pass their local copy of the >> auditd_connection pointer to auditd_reset() where it can be compared >> with the current connection state before resetting. If the caller >> has a stale state tracking pointer then the reset is ignored. >> >> We also make a small change to kauditd_thread() so that if the >> kernel/auditd connection is dead we skip the retry queue and send the >> records straight to the hold queue. This is necessary as we used to >> rely on auditd_reset() to occasionally purge the retry queue but we >> are going to be calling the reset function much less now and we want >> to make sure the retry queue doesn't grow unbounded. >> >> Reported-by: Adam Williamson <[email protected]> >> Reported-by: Dusty Mabe <[email protected]> >> Signed-off-by: Paul Moore <[email protected]> > > This looks reasonable. > Reviewed-by: Richard Guy Briggs <[email protected]>
Thanks for the review. Similar to the other patch, I'll merge this tomorrow if nothing pops up. (Yes, it's slightly more complicated than the other patch, but it's a relatively low risk bug-fix so it warrants merging at this late stage I think.) -- paul moore www.paul-moore.com -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
