io-uring.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Richard Guy Briggs <rgb@redhat.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: Stefan Metzmacher <metze@samba.org>,
	Paul Moore <paul@paul-moore.com>,
	Pavel Begunkov <asml.silence@gmail.com>,
	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, 27 May 2021 13:27:07 -0400	[thread overview]
Message-ID: <20210527172707.GI2268484@madcap2.tricolour.ca> (raw)
In-Reply-To: <aaa98bcc-0515-f0e4-f15f-058ef0eb61e7@kernel.dk>

On 2021-05-26 11:22, Jens Axboe wrote:
> On 5/26/21 9:49 AM, Richard Guy Briggs wrote:
> >> So why is there anything special needed for io_uring (now that the
> >> native worker threads are used)?
> > 
> > Because syscall has been bypassed by a memory-mapped work queue.
> 
> I don't follow this one at all, that's just the delivery mechanism if
> you choose to use SQPOLL. If you do, then a thread sibling of the
> original task does the actual system call. There's no magic involved
> there, and the tasks are related.
> 
> So care to expand on that?

These may be poor examples, but hear me out...

In the case of an open call, I'm guessing that they are mapped 1:1
syscall to io_uring action so that the action can be asynchronous.  So
in this case, there is a record of the action, but we don't see the
result success/failure.  I assume that file paths can only be specified
in the original syscall and not in an SQPOLL action?

In the case of a syscall read/write (which aren't as interesting
generally to audit, but I'm concerned there are other similar situations
that are), the syscall would be called for every buffer that is passed
back and forth user/kernel and vice versa, providing a way to log all
that activity.  In the case of SQPOLL, I understand that a syscall sets
up the action and then io_uring takes over and does the bulk transfer
and we'd not have the visibility of how many times that action was
repeated nor that the result success/failure was due to its asynchrony.

Perhaps I am showing my ignorance, so please correct me if I have it
wrong.

> >> Is there really any io_uring opcode that bypasses the security checks the corresponding native syscall
> >> would do? If so, I think that should just be fixed...
> > 
> > This is by design to speed it up.  This is what Paul's iouring entry and
> > exit hooks do.
> 
> As far as I can tell, we're doing double logging at that point, if the
> syscall helper does the audit as well. We'll get something logging the
> io_uring opcode (eg IORING_OP_OPENAT2), then log again when we call the
> fs helper. That's _assuming_ that we always hit the logging part when we
> call into the vfs, but that's something that can be updated to be true
> and kept an eye on for future additions.
> 
> Why is the double logging useful? It only tells you that the invocation
> was via io_uring as the delivery mechanism rather than the usual system
> call, but the effect is the same - the file is opened, for example.
> 
> I feel like I'm missing something here, or the other side is. Or both!

Paul addressed this in his reply, but let me add a more concrete
example...  There was one corner case I was looking at that showed up
this issue.  Please indicate if I have mischaracterized or
misunderstood.

A syscall would generate records something like this:

	AUDIT_SYSCALL
	AUDIT_...
	AUDIT_EOE

A io_uring SQPOLL event would generate something like this:

	AUDIT_URINGOP
	AUDIT_...
	AUDIT_EOE

The "hybrid" event that is a syscall that starts an io_uring action
would generate something like this:

	AUDIT_URINGOP
	[possible AUDIT_CONFIG_CHANGE (from killed_trees)]
	AUDIT_SYSCALL
	AUDIT_...
	AUDIT_EOE

The AUDIT_... is all the operation-specific records that log parameters
that aren't able to be expressed in the SYSLOG or URINGOP records such
as pathnames, other arguments, and context (pwd, etc...).  So this isn't
"double logging".  It is either introducing an io_uring event, or it is
providing more detail about the io_uring arguments to a syscall event.

> Jens Axboe

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635


  reply	other threads:[~2021-05-27 17:27 UTC|newest]

Thread overview: 71+ 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 ` [RFC PATCH 1/9] audit: prepare audit_context for use in calling contexts beyond syscalls 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-22  0:22   ` Pavel Begunkov
2021-05-22  2:36     ` Paul Moore
2021-05-23 20:26       ` Pavel Begunkov
2021-05-24 19:59         ` Paul Moore
2021-05-25  8:27           ` Pavel Begunkov
2021-05-25 14:53             ` Paul Moore
2021-05-26  1:11           ` Jens Axboe
2021-05-26  2:04             ` Paul Moore
2021-05-26 10:19               ` Pavel Begunkov
2021-05-26 14:38                 ` Paul Moore
2021-05-26 15:11                   ` Steve Grubb
2021-05-26 15:17                   ` Stefan Metzmacher
2021-05-26 15:49                     ` Richard Guy Briggs
2021-05-26 17:22                       ` Jens Axboe
2021-05-27 17:27                         ` Richard Guy Briggs [this message]
2021-05-26 15:49                     ` Victor Stewart
2021-05-26 16:38                       ` Casey Schaufler
2021-05-26 17:15               ` Jens Axboe
2021-05-26 17:31                 ` Jens Axboe
2021-05-26 17:54                   ` Jens Axboe
2021-05-26 18:01                     ` Jens Axboe
2021-05-26 18:44                       ` Paul Moore
2021-05-26 18:57                         ` Pavel Begunkov
2021-05-26 19:10                           ` Paul Moore
2021-05-26 19:44                         ` Jens Axboe
2021-05-26 20:19                           ` Paul Moore
2021-05-28 16:02                             ` Paul Moore
2021-06-02  8:26                               ` Pavel Begunkov
2021-06-02 15:46                                 ` Richard Guy Briggs
2021-06-03 10:39                                   ` Pavel Begunkov
2021-06-02 19:46                                 ` Paul Moore
2021-06-03 10:51                                   ` Pavel Begunkov
2021-06-03 15:54                                     ` Casey Schaufler
2021-06-03 15:54                               ` Jens Axboe
2021-06-04  5:04                                 ` 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 20:46     ` Paul Moore
2021-08-25  1:21       ` Richard Guy Briggs
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 ` [RFC PATCH 4/9] audit: add filtering for io_uring records Paul Moore
2021-05-28 22:35   ` Richard Guy Briggs
2021-05-30 15:26     ` Paul Moore
2021-05-31 13:44       ` Richard Guy Briggs
2021-06-02  1:40         ` Paul Moore
2021-06-02 15:37           ` Richard Guy Briggs
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 16:08         ` kernel test robot
2021-05-31 17:38         ` kernel test robot
2021-06-07 23:15         ` Paul Moore
2021-06-08 12:55           ` Richard Guy Briggs
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-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 ` [RFC PATCH 6/9] io_uring: convert io_uring to the secure anon inode interface Paul Moore
2021-05-21 21:50 ` [RFC PATCH 7/9] lsm,io_uring: add LSM hooks to io_uring Paul Moore
2021-05-26 14:48   ` Stefan Metzmacher
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 ` [RFC PATCH 9/9] Smack: Brutalist io_uring support with debug 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  2:06   ` Paul Moore
2021-05-26 15:00 ` Jeff Moyer
2021-05-26 18:49   ` Paul Moore
2021-05-26 19:07     ` Jeff Moyer
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=20210527172707.GI2268484@madcap2.tricolour.ca \
    --to=rgb@redhat.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=metze@samba.org \
    --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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).