On 1/23/24 3:40 PM, Jens Axboe wrote: > On 1/23/24 3:35 PM, Jens Axboe wrote: >> >> On Tue, 23 Jan 2024 16:55:02 -0500, Paul Moore wrote: >>> 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! 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 -- Jens Axboe
