* [PATCH 1/2] io-wq: have io_wq_create() take a 'data' argument
2019-11-25 16:52 [PATCHSET v2 0/2] io_uring: ensure we inherit task creds Jens Axboe
@ 2019-11-25 16:52 ` Jens Axboe
2019-11-25 16:52 ` [PATCH 2/2] io_uring: async workers should inherit the user creds Jens Axboe
1 sibling, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2019-11-25 16:52 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
We currently pass in 4 arguments outside of the bounded size. In
preparation for adding one more argument, let's bundle them up in
a struct to make it more readable.
No functional changes in this patch.
Signed-off-by: Jens Axboe <[email protected]>
---
fs/io-wq.c | 14 ++++++--------
fs/io-wq.h | 12 +++++++++---
fs/io_uring.c | 9 +++++++--
3 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/fs/io-wq.c b/fs/io-wq.c
index 2666384aaf44..49ca58c714da 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -974,9 +974,7 @@ void io_wq_flush(struct io_wq *wq)
}
}
-struct io_wq *io_wq_create(unsigned bounded, struct mm_struct *mm,
- struct user_struct *user, get_work_fn *get_work,
- put_work_fn *put_work)
+struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
{
int ret = -ENOMEM, i, node;
struct io_wq *wq;
@@ -992,11 +990,11 @@ struct io_wq *io_wq_create(unsigned bounded, struct mm_struct *mm,
return ERR_PTR(-ENOMEM);
}
- wq->get_work = get_work;
- wq->put_work = put_work;
+ wq->get_work = data->get_work;
+ wq->put_work = data->put_work;
/* caller must already hold a reference to this */
- wq->user = user;
+ wq->user = data->user;
i = 0;
for_each_online_node(node) {
@@ -1009,7 +1007,7 @@ struct io_wq *io_wq_create(unsigned bounded, struct mm_struct *mm,
wqe->node = node;
wqe->acct[IO_WQ_ACCT_BOUND].max_workers = bounded;
atomic_set(&wqe->acct[IO_WQ_ACCT_BOUND].nr_running, 0);
- if (user) {
+ if (wq->user) {
wqe->acct[IO_WQ_ACCT_UNBOUND].max_workers =
task_rlimit(current, RLIMIT_NPROC);
}
@@ -1031,7 +1029,7 @@ struct io_wq *io_wq_create(unsigned bounded, struct mm_struct *mm,
goto err;
/* caller must have already done mmgrab() on this mm */
- wq->mm = mm;
+ wq->mm = data->mm;
wq->manager = kthread_create(io_wq_manager, wq, "io_wq_manager");
if (!IS_ERR(wq->manager)) {
diff --git a/fs/io-wq.h b/fs/io-wq.h
index 892989f3e41e..6db81f0f44e2 100644
--- a/fs/io-wq.h
+++ b/fs/io-wq.h
@@ -48,9 +48,15 @@ struct io_wq_work {
typedef void (get_work_fn)(struct io_wq_work *);
typedef void (put_work_fn)(struct io_wq_work *);
-struct io_wq *io_wq_create(unsigned bounded, struct mm_struct *mm,
- struct user_struct *user,
- get_work_fn *get_work, put_work_fn *put_work);
+struct io_wq_data {
+ struct mm_struct *mm;
+ struct user_struct *user;
+
+ get_work_fn *get_work;
+ put_work_fn *put_work;
+};
+
+struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data);
void io_wq_destroy(struct io_wq *wq);
void io_wq_enqueue(struct io_wq *wq, struct io_wq_work *work);
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 9f9c2d46c19c..a9a1fb9954cc 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3946,6 +3946,7 @@ static void io_get_work(struct io_wq_work *work)
static int io_sq_offload_start(struct io_ring_ctx *ctx,
struct io_uring_params *p)
{
+ struct io_wq_data data;
unsigned concurrency;
int ret;
@@ -3990,10 +3991,14 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
goto err;
}
+ data.mm = ctx->sqo_mm;
+ data.user = ctx->user;
+ data.get_work = io_get_work;
+ data.put_work = io_put_work;
+
/* Do QD, or 4 * CPUS, whatever is smallest */
concurrency = min(ctx->sq_entries, 4 * num_online_cpus());
- ctx->io_wq = io_wq_create(concurrency, ctx->sqo_mm, ctx->user,
- io_get_work, io_put_work);
+ ctx->io_wq = io_wq_create(concurrency, &data);
if (IS_ERR(ctx->io_wq)) {
ret = PTR_ERR(ctx->io_wq);
ctx->io_wq = NULL;
--
2.24.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 2/2] io_uring: async workers should inherit the user creds
2019-11-25 16:52 [PATCHSET v2 0/2] io_uring: ensure we inherit task creds Jens Axboe
2019-11-25 16:52 ` [PATCH 1/2] io-wq: have io_wq_create() take a 'data' argument Jens Axboe
@ 2019-11-25 16:52 ` Jens Axboe
1 sibling, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2019-11-25 16:52 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
If we don't inherit the original task creds, then we can confuse users
like fuse that pass creds in the request header. See link below on
identical aio issue.
Link: https://lore.kernel.org/linux-fsdevel/[email protected]/T/#u
Signed-off-by: Jens Axboe <[email protected]>
---
fs/io-wq.c | 10 ++++++++++
fs/io-wq.h | 1 +
fs/io_uring.c | 14 ++++++++++++++
3 files changed, 25 insertions(+)
diff --git a/fs/io-wq.c b/fs/io-wq.c
index 49ca58c714da..85c0090b2717 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -57,6 +57,7 @@ struct io_worker {
struct rcu_head rcu;
struct mm_struct *mm;
+ const struct cred *creds;
struct files_struct *restore_files;
};
@@ -111,6 +112,7 @@ struct io_wq {
struct task_struct *manager;
struct user_struct *user;
+ struct cred *creds;
struct mm_struct *mm;
refcount_t refs;
struct completion done;
@@ -136,6 +138,11 @@ static bool __io_worker_unuse(struct io_wqe *wqe, struct io_worker *worker)
{
bool dropped_lock = false;
+ if (worker->creds) {
+ revert_creds(worker->creds);
+ worker->creds = NULL;
+ }
+
if (current->files != worker->restore_files) {
__acquire(&wqe->lock);
spin_unlock_irq(&wqe->lock);
@@ -442,6 +449,8 @@ static void io_worker_handle_work(struct io_worker *worker)
set_fs(USER_DS);
worker->mm = wq->mm;
}
+ if (!worker->creds)
+ worker->creds = override_creds(wq->creds);
if (test_bit(IO_WQ_BIT_CANCEL, &wq->state))
work->flags |= IO_WQ_WORK_CANCEL;
if (worker->mm)
@@ -995,6 +1004,7 @@ struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
/* caller must already hold a reference to this */
wq->user = data->user;
+ wq->creds = data->creds;
i = 0;
for_each_online_node(node) {
diff --git a/fs/io-wq.h b/fs/io-wq.h
index 6db81f0f44e2..e09c6a54648c 100644
--- a/fs/io-wq.h
+++ b/fs/io-wq.h
@@ -51,6 +51,7 @@ typedef void (put_work_fn)(struct io_wq_work *);
struct io_wq_data {
struct mm_struct *mm;
struct user_struct *user;
+ struct cred *creds;
get_work_fn *get_work;
put_work_fn *put_work;
diff --git a/fs/io_uring.c b/fs/io_uring.c
index a9a1fb9954cc..26bfba729a1e 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -237,6 +237,8 @@ struct io_ring_ctx {
struct user_struct *user;
+ struct cred *creds;
+
/* 0 is for ctx quiesce/reinit/free, 1 is for sqo_thread started */
struct completion *completions;
@@ -3251,6 +3253,7 @@ static int io_sq_thread(void *data)
{
struct io_ring_ctx *ctx = data;
struct mm_struct *cur_mm = NULL;
+ const struct cred *old_cred;
mm_segment_t old_fs;
DEFINE_WAIT(wait);
unsigned inflight;
@@ -3261,6 +3264,7 @@ static int io_sq_thread(void *data)
old_fs = get_fs();
set_fs(USER_DS);
+ old_cred = override_creds(ctx->creds);
ret = timeout = inflight = 0;
while (!kthread_should_park()) {
@@ -3367,6 +3371,7 @@ static int io_sq_thread(void *data)
unuse_mm(cur_mm);
mmput(cur_mm);
}
+ revert_creds(old_cred);
kthread_parkme();
@@ -3993,6 +3998,7 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
data.mm = ctx->sqo_mm;
data.user = ctx->user;
+ data.creds = ctx->creds;
data.get_work = io_get_work;
data.put_work = io_put_work;
@@ -4347,6 +4353,8 @@ static void io_ring_ctx_free(struct io_ring_ctx *ctx)
io_unaccount_mem(ctx->user,
ring_pages(ctx->sq_entries, ctx->cq_entries));
free_uid(ctx->user);
+ if (ctx->creds)
+ put_cred(ctx->creds);
kfree(ctx->completions);
kmem_cache_free(req_cachep, ctx->fallback_req);
kfree(ctx);
@@ -4700,6 +4708,12 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p)
ctx->account_mem = account_mem;
ctx->user = user;
+ ctx->creds = prepare_creds();
+ if (!ctx->creds) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
ret = io_allocate_scq_urings(ctx, p);
if (ret)
goto err;
--
2.24.0
^ permalink raw reply related [flat|nested] 3+ messages in thread