On Tue, Jan 23, 2024 at 5:43 PM Jens Axboe <axboe@xxxxxxxxx> wrote: > 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! 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
