Re: [RFC PATCH 2/9] audit,io_uring,io-wq: add some basic audit support to io_uring
On 6/2/21 4:46 PM, Richard Guy Briggs wrote:
On 2021-06-02 09:26, Pavel Begunkov wrote:
> On 5/28/21 5:02 PM, Paul Moore wrote:
>> On Wed, May 26, 2021 at 4:19 PM Paul Moore <paul(a)paul-moore.com> wrote:
>>> ... If we moved the _entry
>>> and _exit calls into the individual operation case blocks (quick
>>> openat example below) so that only certain operations were able to be
>>> audited would that be acceptable assuming the high frequency ops were
>>> untouched? My initial gut feeling was that this would involve >50% of
>>> the ops, but Steve Grubb seems to think it would be less; it may be
>>> time to look at that a bit more seriously, but if it gets a NACK
>>> regardless it isn't worth the time - thoughts?
>>>
>>> case IORING_OP_OPENAT:
>>> audit_uring_entry(req->opcode);
>>> ret = io_openat(req, issue_flags);
>>> audit_uring_exit(!ret, ret);
>>> break;
>>
>> I wanted to pose this question again in case it was lost in the
>> thread, I suspect this may be the last option before we have to "fix"
>> things at the Kconfig level. I definitely don't want to have to go
>> that route, and I suspect most everyone on this thread feels the same,
>> so I'm hopeful we can find a solution that is begrudgingly acceptable
>> to both groups.
>
> May work for me, but have to ask how many, and what is the
> criteria? I'd think anything opening a file or manipulating fs:
>
> IORING_OP_ACCEPT, IORING_OP_CONNECT, IORING_OP_OPENAT[2],
> IORING_OP_RENAMEAT, IORING_OP_UNLINKAT, IORING_OP_SHUTDOWN,
> IORING_OP_FILES_UPDATE
> + coming mkdirat and others.
>
> IORING_OP_CLOSE? IORING_OP_SEND IORING_OP_RECV?
>
> What about?
> IORING_OP_FSYNC, IORING_OP_SYNC_FILE_RANGE,
> IORING_OP_FALLOCATE, IORING_OP_STATX,
> IORING_OP_FADVISE, IORING_OP_MADVISE,
> IORING_OP_EPOLL_CTL
>
>
> Another question, io_uring may exercise asynchronous paths,
> i.e. io_issue_sqe() returns before requests completes.
> Shouldn't be the case for open/etc at the moment, but was that
> considered?
This would be why audit needs to monitor a thread until it wraps up, to
wait for the result code. My understanding is that both sync and async
parts of an op would be monitored.
There may be a misunderstanding
audit_start(req)
ret = io_issue_sqe(req);
audit_end(ret);
io_issue_sqe() may return 0 but leave the request inflight,
which will be completed asynchronously e.g. by IRQ, not going
through io_issue_sqe() or any io_read()/etc helpers again, and
after last audit_end() had already happened.
That's the case with read/write/timeout, but is not true for
open/etc.
> I don't see it happening, but would prefer to keep it open
> async reimplementation in a distant future. Does audit sleep?
Some parts do, some parts don't depending on what they are interacting
with in the kernel. It can be made to not sleep if needed.
Ok, good
--
Pavel Begunkov