On 26/05/2020 09:43, Xiaoguang Wang wrote:
> In io_init_req(), if uers requires a new credentials, currently we'll
> save it in req->work.creds, but indeed io_wq_work is designed to describe
> needed running environment for requests that will go to io-wq, if one
> request is going to be submitted inline, we'd better not touch io_wq_work.
> Here add a new 'const struct cred *creds' in io_kiocb, if uers requires a
> new credentials, inline requests can use it.
>
> This patch is also a preparation for later patch.
What's the difference from keeping only one creds field in io_kiocb (i.e.
req->work.creds), but handling it specially (i.e. always initialising)? It will
be a lot easier than tossing it around.
Also, the patch doubles {get,put}_creds() for sqe->personality case, and that's
extra atomics without a good reason.
>
> Signed-off-by: Xiaoguang Wang <xiaoguang.wang@xxxxxxxxxxxxxxxxx>
> ---
> fs/io_uring.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 2af87f73848e..788d960abc69 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -635,6 +635,7 @@ struct io_kiocb {
> unsigned int flags;
> refcount_t refs;
> struct task_struct *task;
> + const struct cred *creds;
> unsigned long fsize;
> u64 user_data;
> u32 result;
> @@ -1035,8 +1036,10 @@ static inline void io_req_work_grab_env(struct io_kiocb *req,
> mmgrab(current->mm);
> req->work.mm = current->mm;
> }
> - if (!req->work.creds)
> + if (!req->creds)
> req->work.creds = get_current_cred();
> + else
> + req->work.creds = get_cred(req->creds);
> if (!req->work.fs && def->needs_fs) {
> spin_lock(¤t->fs->lock);
> if (!current->fs->in_exec) {
> @@ -1368,6 +1371,9 @@ static void __io_req_aux_free(struct io_kiocb *req)
> if (req->flags & REQ_F_NEED_CLEANUP)
> io_cleanup_req(req);
>
> + if (req->creds)
> + put_cred(req->creds);
> +
> kfree(req->io);
> if (req->file)
> io_put_file(req, req->file, (req->flags & REQ_F_FIXED_FILE));
> @@ -5673,13 +5679,13 @@ static void __io_queue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> again:
> linked_timeout = io_prep_linked_timeout(req);
>
> - if (req->work.creds && req->work.creds != current_cred()) {
> + if (req->creds && req->creds != current_cred()) {
> if (old_creds)
> revert_creds(old_creds);
> - if (old_creds == req->work.creds)
> + if (old_creds == req->creds)
> old_creds = NULL; /* restored original creds */
> else
> - old_creds = override_creds(req->work.creds);
> + old_creds = override_creds(req->creds);
> }
>
> ret = io_issue_sqe(req, sqe, true);
> @@ -5970,11 +5976,12 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
>
> id = READ_ONCE(sqe->personality);
> if (id) {
> - req->work.creds = idr_find(&ctx->personality_idr, id);
> - if (unlikely(!req->work.creds))
> + req->creds = idr_find(&ctx->personality_idr, id);
> + if (unlikely(!req->creds))
> return -EINVAL;
> - get_cred(req->work.creds);
> - }
> + get_cred(req->creds);
> + } else
> + req->creds = NULL;
>
> /* same numerical values with corresponding REQ_F_*, safe to copy */
> req->flags |= sqe_flags;
>
--
Pavel Begunkov
