Thread (7 messages) 7 messages, 2 authors, 2024-01-23

Re: [PATCH] io_uring: enable audit and restrict cred override for IORING_OP_FIXED_FD_INSTALL

From: Paul Moore <paul@paul-moore.com>
Date: 2024-01-23 23:58:30
Also in: io-uring, selinux

On Tue, Jan 23, 2024 at 5:43 PM Jens Axboe [off-list ref] wrote:
On 1/23/24 3:40 PM, Jens Axboe wrote:
quoted
On 1/23/24 3:35 PM, Jens Axboe wrote:
quoted
On Tue, 23 Jan 2024 16:55:02 -0500, Paul Moore wrote:
quoted
We need to correct some aspects of the IORING_OP_FIXED_FD_INSTALL
command to take into account the security implications of making an
io_uring-private file descriptor generally accessible to a userspace
task.

The first change in this patch is to enable auditing of the FD_INSTALL
operation as installing a file descriptor into a task's file descriptor
table is a security relevant operation and something that admins/users
may want to audit.

[...]
Applied, thanks!

[1/1] io_uring: enable audit and restrict cred override for IORING_OP_FIXED_FD_INSTALL
      commit: 16bae3e1377846734ec6b87eee459c0f3551692c
So after doing that and writing the test case and testing it, it dawned
on me that we should potentially allow the current task creds. And to
make matters worse, this is indeed what happens if eg the application
would submit this with IOSQE_ASYNC or if it was part of a linked series
and we marked it async.

While I originally reasoned for why this is fine as it'd be silly to
register your current creds and then proceed to pass in that personality,
I do think that we should probably handle that case and clearly separate
the case of "we assigned creds from the submitting task because we're
handing it to a thread" vs "the submitting task asked for other creds
that were previously registered".

I'll take a look and see what works the best here.
Actually, a quick look and it's fine, the usual async offload will do
the right thing. So let's just keep it as-is, I don't think there's any
point to complicating this for some theoretically-valid-but-obscure use
case!
Perhaps the one case where REQ_F_CREDS is our friend for FD_INSTALL ;)
FWIW, the test case is here, and I'll augment it now to add IOSQE_ASYNC
as well just to cover all the bases.

https://git.kernel.dk/cgit/liburing/commit/?id=bc576ca398661b266d3e4a4f5db3a9cf7f33fe62
Great, thanks!

-- 
paul-moore.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help