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

On 5/22/21 3:36 AM, Paul Moore wrote:

On Fri, May 21, 2021 at 8:22 PM Pavel Begunkov [off-list ref] wrote:

quoted

On 5/21/21 10:49 PM, Paul Moore wrote:

[...]

quoted

quoted

+     if (req->opcode < IORING_OP_LAST)

always true at this point

I placed the opcode check before the audit call because the switch
statement below which handles the operation dispatching has a 'ret =
-EINVAL' for the default case, implying that there are some paths
where an invalid opcode could be passed into the function.  Obviously
if that is not the case and you can guarantee that req->opcode will
always be valid we can easily drop the check prior to the audit call.

It is always true at this point, would be completely broken
otherwise

quoted

quoted

+             audit_uring_entry(req->opcode);

So, it adds two if's with memory loads (i.e. current->audit_context)
per request in one of the hottest functions here... No way, nack

Maybe, if it's dynamically compiled into like kprobes if it's
_really_ used.

I'm open to suggestions on how to tweak the io_uring/audit
integration, if you don't like what I've proposed in this patchset,
lets try to come up with a solution that is more palatable.  If you
were going to add audit support for these io_uring operations, how
would you propose we do it?  Not being able to properly audit io_uring
operations is going to be a significant issue for a chunk of users, if
it isn't already, we need to work to find a solution to this problem.

Who knows. First of all, seems CONFIG_AUDIT is enabled by default
for many popular distributions, so I assume that is not compiled out.

What are use cases for audit? Always running I guess? Putting aside
compatibility problems, it sounds that with the amount of overhead
it adds there is no much profit in using io_uring in the first place.
Is that so?

__audit_uring_exit()
-> audit_filter_syscall()
  -> for (audit_list) if (...) audit_filter_rules()
    -> ...
-> audit_filter_inodes()
  -> ...

Unfortunately I don't think dynamically inserting audit calls is
something that would meet the needs of the audit community (I fear it
would run afoul of the various security certifications), and it
definitely isn't something that we support at present.

I see

-- 
Pavel Begunkov