From: Pavel Begunkov <asml.silence@gmail.com> To: Paul Moore <paul@paul-moore.com> Cc: Jens Axboe <axboe@kernel.dk>, linux-security-module@vger.kernel.org, selinux@vger.kernel.org, linux-audit@redhat.com, io-uring@vger.kernel.org, linux-fsdevel@vger.kernel.org, Kumar Kartikeya Dwivedi <memxor@gmail.com>, Alexander Viro <viro@zeniv.linux.org.uk> Subject: Re: [RFC PATCH 2/9] audit,io_uring,io-wq: add some basic audit support to io_uring Date: Thu, 3 Jun 2021 11:51:44 +0100 [thread overview] Message-ID: <41bc1351-b07b-d9de-f7e3-8c58be14ba9f@gmail.com> (raw) In-Reply-To: <CAHC9VhS7Vhby4YR94U2YOwMtva-rc=_ifRcZYi1YVPwfi+Xuzg@mail.gmail.com> On 6/2/21 8:46 PM, Paul Moore wrote: > On Wed, Jun 2, 2021 at 4:27 AM Pavel Begunkov <asml.silence@gmail.com> wrote: >> On 5/28/21 5:02 PM, Paul Moore wrote: >>> On Wed, May 26, 2021 at 4:19 PM Paul Moore <paul@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 > > Looking quickly at v5.13-rc4 the following seems like candidates for > auditing, there may be a small number of subtractions/additions to > this list as people take a closer look, but it should serve as a > starting point: > > IORING_OP_SENDMSG > IORING_OP_RECVMSG > IORING_OP_ACCEPT > IORING_OP_CONNECT > IORING_OP_FALLOCATE > IORING_OP_OPENAT > IORING_OP_CLOSE > IORING_OP_MADVISE > IORING_OP_OPENAT2 > IORING_OP_SHUTDOWN > IORING_OP_RENAMEAT > IORING_OP_UNLINKAT > > ... can you live with that list? it will bloat binary somewhat, but considering it's all in one place -- io_issue_sqe(), it's workable. Not nice to have send/recv msg in the list, but I admit they may do some crazy things. What can be traced for them? Because at the moment of issue_sqe() not everything is read from the userspace. see: io_sendmsg() { ...; io_sendmsg_copy_hdr(); }, will copy header only in io_sendmsg() in most cases, and then stash it for re-issuing if needed. >> 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? > > Yes, I noticed that when testing the code (and it makes sense when you > look at how io_uring handles things). Depending on the state of the > system when the io_uring request is submitted I've seen both sync and > async io_uring operations with the associated different calling > contexts. In the case where io_issue_sqe() needs to defer the > operation to a different context you will see an audit record > indicating that the operation failed and then another audit record > when it completes; it's actually pretty interesting to be able to see > how the system and io_uring are working. Copying a reply to another message to keep clear out of misunderstanding. "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." And there is interest in async send/recv[msg] as well (via IRQ as described, callbacks, etc.). > We could always mask out these delayed attempts, but at this early > stage they are helpful, and they may be useful for admins. > >> I don't see it happening, but would prefer to keep it open >> async reimplementation in a distant future. Does audit sleep? > > The only place in the audit_uring_entry()/audit_uring_exit() code path > that could sleep at present is the call to audit_log_uring() which is > made when the rules dictate that an audit record be generated. The > offending code is an allocation in audit_log_uring() which is > currently GFP_KERNEL but really should be GFP_ATOMIC, or similar. It > was a copy-n-paste from the similar syscall function where GFP_KERNEL > is appropriate due to the calling context at the end of the syscall. > I'll change that as soon as I'm done with this email. Ok, depends where it steers, but there may be a requirement to not sleep for some hooks because of not having a sleepable context. > > Of course if you are calling io_uring_enter(2), or something similar, > then audit may sleep as part of the normal syscall processing (as > mentioned above), but that is due to the fact that io_uring_enter(2) > is a syscall and not because of anything in io_issue_sqe(). > -- Pavel Begunkov
WARNING: multiple messages have this Message-ID (diff)
From: Pavel Begunkov <asml.silence@gmail.com> To: Paul Moore <paul@paul-moore.com> Cc: Jens Axboe <axboe@kernel.dk>, selinux@vger.kernel.org, linux-security-module@vger.kernel.org, linux-audit@redhat.com, Kumar Kartikeya Dwivedi <memxor@gmail.com>, linux-fsdevel@vger.kernel.org, io-uring@vger.kernel.org, Alexander Viro <viro@zeniv.linux.org.uk> Subject: Re: [RFC PATCH 2/9] audit,io_uring,io-wq: add some basic audit support to io_uring Date: Thu, 3 Jun 2021 11:51:44 +0100 [thread overview] Message-ID: <41bc1351-b07b-d9de-f7e3-8c58be14ba9f@gmail.com> (raw) In-Reply-To: <CAHC9VhS7Vhby4YR94U2YOwMtva-rc=_ifRcZYi1YVPwfi+Xuzg@mail.gmail.com> On 6/2/21 8:46 PM, Paul Moore wrote: > On Wed, Jun 2, 2021 at 4:27 AM Pavel Begunkov <asml.silence@gmail.com> wrote: >> On 5/28/21 5:02 PM, Paul Moore wrote: >>> On Wed, May 26, 2021 at 4:19 PM Paul Moore <paul@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 > > Looking quickly at v5.13-rc4 the following seems like candidates for > auditing, there may be a small number of subtractions/additions to > this list as people take a closer look, but it should serve as a > starting point: > > IORING_OP_SENDMSG > IORING_OP_RECVMSG > IORING_OP_ACCEPT > IORING_OP_CONNECT > IORING_OP_FALLOCATE > IORING_OP_OPENAT > IORING_OP_CLOSE > IORING_OP_MADVISE > IORING_OP_OPENAT2 > IORING_OP_SHUTDOWN > IORING_OP_RENAMEAT > IORING_OP_UNLINKAT > > ... can you live with that list? it will bloat binary somewhat, but considering it's all in one place -- io_issue_sqe(), it's workable. Not nice to have send/recv msg in the list, but I admit they may do some crazy things. What can be traced for them? Because at the moment of issue_sqe() not everything is read from the userspace. see: io_sendmsg() { ...; io_sendmsg_copy_hdr(); }, will copy header only in io_sendmsg() in most cases, and then stash it for re-issuing if needed. >> 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? > > Yes, I noticed that when testing the code (and it makes sense when you > look at how io_uring handles things). Depending on the state of the > system when the io_uring request is submitted I've seen both sync and > async io_uring operations with the associated different calling > contexts. In the case where io_issue_sqe() needs to defer the > operation to a different context you will see an audit record > indicating that the operation failed and then another audit record > when it completes; it's actually pretty interesting to be able to see > how the system and io_uring are working. Copying a reply to another message to keep clear out of misunderstanding. "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." And there is interest in async send/recv[msg] as well (via IRQ as described, callbacks, etc.). > We could always mask out these delayed attempts, but at this early > stage they are helpful, and they may be useful for admins. > >> I don't see it happening, but would prefer to keep it open >> async reimplementation in a distant future. Does audit sleep? > > The only place in the audit_uring_entry()/audit_uring_exit() code path > that could sleep at present is the call to audit_log_uring() which is > made when the rules dictate that an audit record be generated. The > offending code is an allocation in audit_log_uring() which is > currently GFP_KERNEL but really should be GFP_ATOMIC, or similar. It > was a copy-n-paste from the similar syscall function where GFP_KERNEL > is appropriate due to the calling context at the end of the syscall. > I'll change that as soon as I'm done with this email. Ok, depends where it steers, but there may be a requirement to not sleep for some hooks because of not having a sleepable context. > > Of course if you are calling io_uring_enter(2), or something similar, > then audit may sleep as part of the normal syscall processing (as > mentioned above), but that is due to the fact that io_uring_enter(2) > is a syscall and not because of anything in io_issue_sqe(). > -- Pavel Begunkov -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
next prev parent reply other threads:[~2021-06-03 10:52 UTC|newest] Thread overview: 144+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-21 21:49 [RFC PATCH 0/9] Add LSM access controls and auditing to io_uring Paul Moore 2021-05-21 21:49 ` Paul Moore 2021-05-21 21:49 ` [RFC PATCH 1/9] audit: prepare audit_context for use in calling contexts beyond syscalls Paul Moore 2021-05-21 21:49 ` Paul Moore 2021-05-21 21:49 ` [RFC PATCH 2/9] audit,io_uring,io-wq: add some basic audit support to io_uring Paul Moore 2021-05-21 21:49 ` [RFC PATCH 2/9] audit, io_uring, io-wq: " Paul Moore 2021-05-22 0:22 ` [RFC PATCH 2/9] audit,io_uring,io-wq: " Pavel Begunkov 2021-05-22 0:22 ` Pavel Begunkov 2021-05-22 2:36 ` Paul Moore 2021-05-22 2:36 ` Paul Moore 2021-05-23 20:26 ` Pavel Begunkov 2021-05-23 20:26 ` Pavel Begunkov 2021-05-24 19:59 ` Paul Moore 2021-05-24 19:59 ` Paul Moore 2021-05-25 8:27 ` Pavel Begunkov 2021-05-25 8:27 ` Pavel Begunkov 2021-05-25 14:53 ` Paul Moore 2021-05-25 14:53 ` Paul Moore 2021-05-26 1:11 ` Jens Axboe 2021-05-26 1:11 ` Jens Axboe 2021-05-26 2:04 ` Paul Moore 2021-05-26 2:04 ` Paul Moore 2021-05-26 10:19 ` Pavel Begunkov 2021-05-26 10:19 ` Pavel Begunkov 2021-05-26 14:38 ` Paul Moore 2021-05-26 14:38 ` Paul Moore 2021-05-26 15:11 ` Steve Grubb 2021-05-26 15:11 ` [RFC PATCH 2/9] audit, io_uring, io-wq: " Steve Grubb 2021-05-26 15:17 ` [RFC PATCH 2/9] audit,io_uring,io-wq: " Stefan Metzmacher 2021-05-26 15:17 ` Stefan Metzmacher 2021-05-26 15:49 ` Richard Guy Briggs 2021-05-26 15:49 ` Richard Guy Briggs 2021-05-26 17:22 ` Jens Axboe 2021-05-26 17:22 ` Jens Axboe 2021-05-27 17:27 ` Richard Guy Briggs 2021-05-27 17:27 ` Richard Guy Briggs 2021-05-26 15:49 ` Victor Stewart 2021-05-26 15:49 ` Victor Stewart 2021-05-26 16:38 ` Casey Schaufler 2021-05-26 16:38 ` Casey Schaufler 2021-05-26 17:15 ` Jens Axboe 2021-05-26 17:15 ` Jens Axboe 2021-05-26 17:31 ` Jens Axboe 2021-05-26 17:31 ` Jens Axboe 2021-05-26 17:54 ` Jens Axboe 2021-05-26 17:54 ` Jens Axboe 2021-05-26 18:01 ` Jens Axboe 2021-05-26 18:01 ` Jens Axboe 2021-05-26 18:44 ` Paul Moore 2021-05-26 18:44 ` Paul Moore 2021-05-26 18:57 ` Pavel Begunkov 2021-05-26 18:57 ` Pavel Begunkov 2021-05-26 19:10 ` Paul Moore 2021-05-26 19:10 ` Paul Moore 2021-05-26 19:44 ` Jens Axboe 2021-05-26 19:44 ` Jens Axboe 2021-05-26 20:19 ` Paul Moore 2021-05-26 20:19 ` Paul Moore 2021-05-28 16:02 ` Paul Moore 2021-05-28 16:02 ` Paul Moore 2021-06-02 8:26 ` Pavel Begunkov 2021-06-02 8:26 ` Pavel Begunkov 2021-06-02 15:46 ` Richard Guy Briggs 2021-06-02 15:46 ` Richard Guy Briggs 2021-06-03 10:39 ` Pavel Begunkov 2021-06-03 10:39 ` Pavel Begunkov 2021-06-02 19:46 ` Paul Moore 2021-06-02 19:46 ` Paul Moore 2021-06-03 10:51 ` Pavel Begunkov [this message] 2021-06-03 10:51 ` Pavel Begunkov 2021-06-03 15:54 ` Casey Schaufler 2021-06-03 15:54 ` Casey Schaufler 2021-06-03 15:54 ` Jens Axboe 2021-06-03 15:54 ` Jens Axboe 2021-06-04 5:04 ` Paul Moore 2021-06-04 5:04 ` Paul Moore 2021-05-26 18:38 ` Paul Moore 2021-05-26 18:38 ` Paul Moore 2021-06-02 17:29 ` [RFC PATCH 2/9] audit, io_uring, io-wq: " Richard Guy Briggs 2021-06-02 17:29 ` Richard Guy Briggs 2021-06-02 20:46 ` Paul Moore 2021-06-02 20:46 ` Paul Moore 2021-08-25 1:21 ` Richard Guy Briggs 2021-08-25 1:21 ` Richard Guy Briggs 2021-08-25 19:41 ` Paul Moore 2021-08-25 19:41 ` Paul Moore 2021-05-21 21:50 ` [RFC PATCH 3/9] audit: dev/test patch to force io_uring auditing Paul Moore 2021-05-21 21:50 ` Paul Moore 2021-05-21 21:50 ` [RFC PATCH 4/9] audit: add filtering for io_uring records Paul Moore 2021-05-21 21:50 ` Paul Moore 2021-05-28 22:35 ` Richard Guy Briggs 2021-05-28 22:35 ` Richard Guy Briggs 2021-05-30 15:26 ` Paul Moore 2021-05-30 15:26 ` Paul Moore 2021-05-31 13:44 ` Richard Guy Briggs 2021-05-31 13:44 ` Richard Guy Briggs 2021-06-02 1:40 ` Paul Moore 2021-06-02 1:40 ` Paul Moore 2021-06-02 15:37 ` Richard Guy Briggs 2021-06-02 15:37 ` Richard Guy Briggs 2021-06-02 17:20 ` Paul Moore 2021-06-02 17:20 ` Paul Moore 2021-05-31 13:44 ` [PATCH 1/2] audit: add filtering for io_uring records, addendum Richard Guy Briggs 2021-05-31 13:44 ` Richard Guy Briggs 2021-05-31 16:08 ` kernel test robot 2021-05-31 16:08 ` kernel test robot 2021-05-31 16:08 ` kernel test robot 2021-05-31 17:38 ` kernel test robot 2021-05-31 17:38 ` kernel test robot 2021-05-31 17:38 ` kernel test robot 2021-06-07 23:15 ` Paul Moore 2021-06-07 23:15 ` Paul Moore 2021-06-08 12:55 ` Richard Guy Briggs 2021-06-08 12:55 ` Richard Guy Briggs 2021-06-09 2:45 ` Paul Moore 2021-06-09 2:45 ` Paul Moore 2021-05-31 13:44 ` [PATCH 2/2] audit: block PERM fields being used with io_uring filtering Richard Guy Briggs 2021-05-31 13:44 ` Richard Guy Briggs 2021-05-21 21:50 ` [RFC PATCH 5/9] fs: add anon_inode_getfile_secure() similar to anon_inode_getfd_secure() Paul Moore 2021-05-21 21:50 ` Paul Moore 2021-05-21 21:50 ` [RFC PATCH 6/9] io_uring: convert io_uring to the secure anon inode interface Paul Moore 2021-05-21 21:50 ` Paul Moore 2021-05-21 21:50 ` [RFC PATCH 7/9] lsm,io_uring: add LSM hooks to io_uring Paul Moore 2021-05-21 21:50 ` Paul Moore 2021-05-26 14:48 ` Stefan Metzmacher 2021-05-26 14:48 ` Stefan Metzmacher 2021-05-26 20:45 ` Paul Moore 2021-05-26 20:45 ` Paul Moore 2021-05-21 21:50 ` [RFC PATCH 8/9] selinux: add support for the io_uring access controls Paul Moore 2021-05-21 21:50 ` Paul Moore 2021-05-21 21:50 ` [RFC PATCH 9/9] Smack: Brutalist io_uring support with debug Paul Moore 2021-05-21 21:50 ` Paul Moore 2021-05-22 0:53 ` [RFC PATCH 0/9] Add LSM access controls and auditing to io_uring Tetsuo Handa 2021-05-22 0:53 ` Tetsuo Handa 2021-05-22 2:06 ` Paul Moore 2021-05-22 2:06 ` Paul Moore 2021-05-26 15:00 ` Jeff Moyer 2021-05-26 15:00 ` Jeff Moyer 2021-05-26 18:49 ` Paul Moore 2021-05-26 18:49 ` Paul Moore 2021-05-26 19:07 ` Jeff Moyer 2021-05-26 19:07 ` Jeff Moyer 2021-05-26 19:10 ` Paul Moore 2021-05-26 19:10 ` Paul Moore
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=41bc1351-b07b-d9de-f7e3-8c58be14ba9f@gmail.com \ --to=asml.silence@gmail.com \ --cc=axboe@kernel.dk \ --cc=io-uring@vger.kernel.org \ --cc=linux-audit@redhat.com \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-security-module@vger.kernel.org \ --cc=memxor@gmail.com \ --cc=paul@paul-moore.com \ --cc=selinux@vger.kernel.org \ --cc=viro@zeniv.linux.org.uk \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.