Re: [RFC PATCH 2/9] audit,io_uring,io-wq: add some basic audit support to io_uring

[Pavel Begunkov <asml.silence@gmail.com>]

On 5/26/21 3:04 AM, Paul Moore wrote:
> On Tue, May 25, 2021 at 9:11 PM Jens Axboe <axboe@kernel.dk> wrote:
>> On 5/24/21 1:59 PM, Paul Moore wrote:
>>> That said, audit is not for everyone, and we have build time and
>>> runtime options to help make life easier.  Beyond simply disabling
>>> audit at compile time a number of Linux distributions effectively
>>> shortcut audit at runtime by adding a "never" rule to the audit
>>> filter, for example:
>>>
>>>  % auditctl -a task,never
>>
>> As has been brought up, the issue we're facing is that distros have
>> CONFIG_AUDIT=y and hence the above is the best real world case outside
>> of people doing custom kernels. My question would then be how much
>> overhead the above will add, considering it's an entry/exit call per op.
>> If auditctl is turned off, what is the expectation in turns of overhead?
> 
> I commented on that case in my last email to Pavel, but I'll try to go
> over it again in a little more detail.
> 
> As we discussed earlier in this thread, we can skip the req->opcode
> check before both the _entry and _exit calls, so we are left with just
> the bare audit calls in the io_uring code.  As the _entry and _exit
> functions are small, I've copied them and their supporting functions
> below and I'll try to explain what would happen in CONFIG_AUDIT=y,
> "task,never" case.
> 
> +  static inline struct audit_context *audit_context(void)
> +  {
> +    return current->audit_context;
> +  }
> 
> +  static inline bool audit_dummy_context(void)
> +  {
> +    void *p = audit_context();
> +    return !p || *(int *)p;
> +  }
> 
> +  static inline void audit_uring_entry(u8 op)
> +  {
> +    if (unlikely(audit_enabled && audit_context()))
> +      __audit_uring_entry(op);
> +  }

I'd rather agree that it's my cycle-picking. The case I care about
is CONFIG_AUDIT=y (because everybody enable it), and io_uring
tracing _not_ enabled at runtime. If enabled let them suffer
the overhead, it will probably dip down the performance

So, for the case I care about it's two of

if (unlikely(audit_enabled && current->audit_context))

in the hot path. load-test-jump + current, so it will
be around 7x2 instructions. We can throw away audit_enabled
as you say systemd already enables it, that will give
4x2 instructions including 2 conditional jumps.

That's not great at all. And that's why I brought up
the question about need of pre and post hooks and whether
can be combined. Would be just 4 instructions and that is
ok (ish).

> We would need to check with the current security requirements (there
> are distro people on the linux-audit list that keep track of that
> stuff), but looking at the opcodes right now my gut feeling is that
> most of the opcodes would be considered "security relevant" so
> selective auditing might not be that useful in practice.  It would
> definitely clutter the code and increase the chances that new opcodes
> would not be properly audited when they are merged.

I'm curious, why it's enabled by many distros by default? Are there
use cases they use? Tempting to add AUDIT_IOURING=default N, but
won't work I guess

-- 
Pavel Begunkov