On 1/16/20 3:42 PM, Stefan Metzmacher wrote: >>> I'm not sure if current->mm is needed, I just added it for completeness >>> and as hint that io_op_defs[req->opcode].needs_mm is there and a >>> needs_creds could also be added (if it helps with performance) >>> >>> Is it possible to trigger a change of current->mm from userspace? >>> >>> An IORING_REGISTER_CREDS would only be useful if it's possible to >>> register a set of credentials and then use per io_uring_sqe credentials. >>> That would also be fine for me, but I'm not sure it's needed for now. >> >> I think it'd be a cleaner way of doing the same thing as your patch >> does. It seems a little odd to do this by default (having the ring >> change personalities depending on who's using it), but from an opt-in >> point of view, I think it makes more sense. >> >> That would make the IORING_REGISTER_ call something like >> IORING_REGISTER_ADOPT_OWNER or something like that, meaning that the >> ring would just assume the identify of the task that's calling >> io_uring_enter(). >> >> Note that this also has to be passed through to the io-wq handler, as >> the mappings there are currently static as well. > > What's the next step here? Not sure, need to find some time to work on this! > I think the current state is a security problem! > > The inline execution either needs to change the creds temporary > or io_uring_enter() needs a general check that the current creds match > the creds of the ring and return -EPERM or something similar. Hmm, if you transfer the fd to someone else, you also give them access to your credentials etc. We could make that -EPERM, if the owner of the ring isn't the one invoking the submit. But that doesn't really help the SQPOLL case, which simply consumes SQE entries. There can be no checking there. -- Jens Axboe
