auditd: Fix kernel deadlock after ENOBUFS

[adriansr]

What does this PR do?

This fixes a deadlock when the netlink channel is congested (initialization fails with "no buffer space available" / errno=ENOBUFS).

Apart from preventing the deadlock by consuming events from the netlink channel during close, it adds code to handle the unlikely ENOBUFS during initialization, to prevent failure of the auditd module.

Why is it important?

Prevents a deadlock that has been reported on large deployments where Auditbeat is restarted frequently and the hosts have a large amount of auditd events.

The deadlock is reported to happen around 3 times for ~5000 daily restarts.

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• [ ] I have made corresponding changes to the documentation
• [ ] I have made corresponding change to the default configuration files
• [ ] I have added tests that prove my fix is effective or that my feature works
• I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

See #26031

Related issues

• Closes Auditbeat: kauditd deadlocks when netlink channel is congested during startup #26031

[elasticmachine]

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

[adriansr]

A more correct fix is to refactor the auditd module or go-libaudit to consume audit messages before initialization and until the netlink connection is closed for good. But for a bugfix, I'd rather avoid a complex refactor.

I was a bit puzzled that errno=ENOBUFS is consistently received only by the SetPID operation. It seems that it can only be triggered from ACK responses and it can't happen until Auditbeat is configured as the Audit daemon. That would explain why we don't see it for other initialization calls like GetStatus.

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Build Cause: Pull request #26032 updated

• Start Time: 2021-06-02T13:53:00.537+0000

• Duration: 73 min 5 sec

• Commit: 238ede0

Test stats 🧪

Test	Results
Failed	0
Passed	1004
Skipped	216
Total	1220

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	1004
Skipped	216
Total	1220

[andrewstucki]

This is the only reader at this point, correct? Do we have to worry about any sort of data races in the case of a successfully started client that also has a read loop, or we don't care because we're shutting down anyway?

[adriansr]

There is the main reader from auditd that may be reading at the same time. In my tests, there's no issue of having both readers in parallel

[wenlxie]

Thanks for the fix.
Another fix maybe simple:
When there is a setPid error during init, then not do setPid() operation in client.close()?

[adriansr]

@wenlxie I've seen some cases where the SetPID succeeds but still the next SetPID in Close blocks. It's much less likely though.