On 2/22/20 11:00 PM, Jens Axboe wrote:
> On 2/21/20 12:10 PM, Jens Axboe wrote:
>>> Got it. Then, it may happen in the future after returning from
>>> __io_arm_poll_handler() and io_uring_enter(). And by that time io_submit_sqes()
>>> should have already restored creds (i.e. personality stuff) on the way back.
>>> This might be a problem.
>>
>> Not sure I follow, can you elaborate? Just to be sure, the requests that
>> go through the poll handler will go through __io_queue_sqe() again. Oh I
>> guess your point is that that is one level below where we normally
>> assign the creds.
>
> Fixed this one.
>
>>> BTW, Is it by design, that all requests of a link use personality creds
>>> specified in the head's sqe?
>>
>> No, I think that's more by accident. We should make sure they use the
>> specified creds, regardless of the issue time. Care to clean that up?
>> Would probably help get it right for the poll case, too.
>
> Took a look at this, and I think you're wrong. Every iteration of
> io_submit_sqe() will lookup the right creds, and assign them to the
> current task in case we're going to issue it. In the case of a link
> where we already have the head, then we grab the current work
> environment. This means assigning req->work.creds from
> get_current_cred(), if not set, and these are the credentials we looked
> up already.
What does look wrong is that we don't restore the right credentials for
queuing the head, so basically the opposite problem. Something like the
below should fix that.
commit b94ddeebd4d068d9205b319179974e09da2591fd
Author: Jens Axboe <axboe@xxxxxxxxx>
Date: Sat Feb 22 23:22:19 2020 -0700
io_uring: handle multiple personalities in link chains
If we have a chain of requests and they don't all use the same
credentials, then the head of the chain will be issued with the
credentails of the tail of the chain.
Ensure __io_queue_sqe() overrides the credentials, if they are different.
Fixes: 75c6a03904e0 ("io_uring: support using a registered personality for commands")
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
diff --git a/fs/io_uring.c b/fs/io_uring.c
index de650df9ac53..59024e4757d6 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -4705,11 +4705,18 @@ static void __io_queue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe)
{
struct io_kiocb *linked_timeout;
struct io_kiocb *nxt = NULL;
+ const struct cred *old_creds = NULL;
int ret;
again:
linked_timeout = io_prep_linked_timeout(req);
+ if (req->work.creds && req->work.creds != get_current_cred()) {
+ if (old_creds)
+ revert_creds(old_creds);
+ old_creds = override_creds(req->work.creds);
+ }
+
ret = io_issue_sqe(req, sqe, &nxt, true);
/*
@@ -4759,6 +4766,8 @@ static void __io_queue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe)
goto punt;
goto again;
}
+ if (old_creds)
+ revert_creds(old_creds);
}
static void io_queue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe)
--
Jens Axboe
