From: Paul Moore <paul@paul-moore.com> To: Pavel Begunkov <asml.silence@gmail.com> Cc: 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>, Jens Axboe <axboe@kernel.dk>, 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: Fri, 21 May 2021 22:36:40 -0400 [thread overview] Message-ID: <CAHC9VhRjzWxweB8d8fypUx11CX6tRBnxSWbXH+5qM1virE509A@mail.gmail.com> (raw) In-Reply-To: <f07bd213-6656-7516-9099-c6ecf4174519@gmail.com> On Fri, May 21, 2021 at 8:22 PM Pavel Begunkov <asml.silence@gmail.com> wrote: > On 5/21/21 10:49 PM, Paul Moore wrote: > > WARNING - This is a work in progress and should not be merged > > anywhere important. It is almost surely not complete, and while it > > probably compiles it likely hasn't been booted and will do terrible > > things. You have been warned. > > > > This patch adds basic auditing to io_uring operations, regardless of > > their context. This is accomplished by allocating audit_context > > structures for the io-wq worker and io_uring SQPOLL kernel threads > > as well as explicitly auditing the io_uring operations in > > io_issue_sqe(). The io_uring operations are audited using a new > > AUDIT_URINGOP record, an example is shown below: > > > > % <TODO - insert AUDIT_URINGOP record example> > > > > Thanks to Richard Guy Briggs for review and feedback. > > > > Signed-off-by: Paul Moore <paul@paul-moore.com> > > --- > [...] > > diff --git a/fs/io_uring.c b/fs/io_uring.c > > index e481ac8a757a..e9941d1ad8fd 100644 > > --- a/fs/io_uring.c > > +++ b/fs/io_uring.c > > @@ -78,6 +78,7 @@ > > #include <linux/task_work.h> > > #include <linux/pagemap.h> > > #include <linux/io_uring.h> > > +#include <linux/audit.h> > > > > #define CREATE_TRACE_POINTS > > #include <trace/events/io_uring.h> > > @@ -6105,6 +6106,9 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags) > > if (req->work.creds && req->work.creds != current_cred()) > > creds = override_creds(req->work.creds); > > > > + 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. > > + 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. 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. -- paul moore www.paul-moore.com
WARNING: multiple messages have this Message-ID (diff)
From: Paul Moore <paul@paul-moore.com> To: Pavel Begunkov <asml.silence@gmail.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: Fri, 21 May 2021 22:36:40 -0400 [thread overview] Message-ID: <CAHC9VhRjzWxweB8d8fypUx11CX6tRBnxSWbXH+5qM1virE509A@mail.gmail.com> (raw) In-Reply-To: <f07bd213-6656-7516-9099-c6ecf4174519@gmail.com> On Fri, May 21, 2021 at 8:22 PM Pavel Begunkov <asml.silence@gmail.com> wrote: > On 5/21/21 10:49 PM, Paul Moore wrote: > > WARNING - This is a work in progress and should not be merged > > anywhere important. It is almost surely not complete, and while it > > probably compiles it likely hasn't been booted and will do terrible > > things. You have been warned. > > > > This patch adds basic auditing to io_uring operations, regardless of > > their context. This is accomplished by allocating audit_context > > structures for the io-wq worker and io_uring SQPOLL kernel threads > > as well as explicitly auditing the io_uring operations in > > io_issue_sqe(). The io_uring operations are audited using a new > > AUDIT_URINGOP record, an example is shown below: > > > > % <TODO - insert AUDIT_URINGOP record example> > > > > Thanks to Richard Guy Briggs for review and feedback. > > > > Signed-off-by: Paul Moore <paul@paul-moore.com> > > --- > [...] > > diff --git a/fs/io_uring.c b/fs/io_uring.c > > index e481ac8a757a..e9941d1ad8fd 100644 > > --- a/fs/io_uring.c > > +++ b/fs/io_uring.c > > @@ -78,6 +78,7 @@ > > #include <linux/task_work.h> > > #include <linux/pagemap.h> > > #include <linux/io_uring.h> > > +#include <linux/audit.h> > > > > #define CREATE_TRACE_POINTS > > #include <trace/events/io_uring.h> > > @@ -6105,6 +6106,9 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags) > > if (req->work.creds && req->work.creds != current_cred()) > > creds = override_creds(req->work.creds); > > > > + 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. > > + 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. 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. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
next prev parent reply other threads:[~2021-05-22 2:36 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 [this message] 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 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=CAHC9VhRjzWxweB8d8fypUx11CX6tRBnxSWbXH+5qM1virE509A@mail.gmail.com \ --to=paul@paul-moore.com \ --cc=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=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.