On 5/22/21 3:36 AM, Paul Moore wrote:
> On Fri, May 21, 2021 at 8:22 PM Pavel Begunkov <asml.silence@xxxxxxxxx> wrote:
>> On 5/21/21 10:49 PM, Paul Moore wrote:
[...]
>>>
>>> + 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
>>> + 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
