IO-Uring Archive on lore.kernel.org
 help / color / Atom feed
* [PATCHSET 0/4] Allow relative lookups
@ 2020-02-07 15:50 Jens Axboe
  2020-02-07 15:50 ` [PATCH 1/4] io_uring: grab owning task fs_struct Jens Axboe
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Jens Axboe @ 2020-02-07 15:50 UTC (permalink / raw)
  To: io-uring

Due to an oversight on my part, AT_FDCWD lookups only work when the
lookup can be done inline, not async. This patchset rectifies that,
aiming for 5.6 for this one as it would be a shame to have openat etc
without that.

Just 3 small simple patches - grab the task ->fs, add io-wq suppor for
passing it in and setting it, and finally add a ->needs_fs to the opcode
table list of requirements for openat/openat2/statx.

Last patch just ensures we allow AT_FDCWD.

 fs/io-wq.c    | 19 +++++++++++++++----
 fs/io-wq.h    |  4 +++-
 fs/io_uring.c | 29 ++++++++++++++++++++++++++++-
 3 files changed, 46 insertions(+), 6 deletions(-)

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] io_uring: grab owning task fs_struct
  2020-02-07 15:50 [PATCHSET 0/4] Allow relative lookups Jens Axboe
@ 2020-02-07 15:50 ` Jens Axboe
  2020-02-07 15:50 ` [PATCH 2/4] io-wq: add support for inheriting ->fs Jens Axboe
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2020-02-07 15:50 UTC (permalink / raw)
  To: io-uring; +Cc: Jens Axboe

We need this for work that operates on relative paths.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index ba97bb433922..da6a5998fa30 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -75,6 +75,7 @@
 #include <linux/fsnotify.h>
 #include <linux/fadvise.h>
 #include <linux/eventpoll.h>
+#include <linux/fs_struct.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/io_uring.h>
@@ -264,6 +265,8 @@ struct io_ring_ctx {
 
 	const struct cred	*creds;
 
+	struct fs_struct	*fs;
+
 	/* 0 is for ctx quiesce/reinit/free, 1 is for sqo_thread started */
 	struct completion	*completions;
 
@@ -6261,6 +6264,16 @@ static void io_ring_ctx_free(struct io_ring_ctx *ctx)
 	if (ctx->account_mem)
 		io_unaccount_mem(ctx->user,
 				ring_pages(ctx->sq_entries, ctx->cq_entries));
+	if (ctx->fs) {
+		struct fs_struct *fs = ctx->fs;
+
+		spin_lock(&ctx->fs->lock);
+		if (--fs->users)
+			fs = NULL;
+		spin_unlock(&ctx->fs->lock);
+		if (fs)
+			free_fs_struct(fs);
+	}
 	free_uid(ctx->user);
 	put_cred(ctx->creds);
 	kfree(ctx->completions);
@@ -6768,6 +6781,13 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p)
 	ctx->user = user;
 	ctx->creds = get_current_cred();
 
+	task_lock(current);
+	spin_lock(&current->fs->lock);
+	ctx->fs = current->fs;
+	ctx->fs->users++;
+	spin_unlock(&current->fs->lock);
+	task_unlock(current);
+
 	ret = io_allocate_scq_urings(ctx, p);
 	if (ret)
 		goto err;
-- 
2.25.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/4] io-wq: add support for inheriting ->fs
  2020-02-07 15:50 [PATCHSET 0/4] Allow relative lookups Jens Axboe
  2020-02-07 15:50 ` [PATCH 1/4] io_uring: grab owning task fs_struct Jens Axboe
@ 2020-02-07 15:50 ` Jens Axboe
  2020-02-07 15:50 ` [PATCH 3/4] io_uring: add ->needs_fs to the opcode requirements table Jens Axboe
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2020-02-07 15:50 UTC (permalink / raw)
  To: io-uring; +Cc: Jens Axboe

Some work items need this for relative path lookup, make it available
like the other inherited credentials/mm/etc.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io-wq.c | 19 +++++++++++++++----
 fs/io-wq.h |  4 +++-
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/fs/io-wq.c b/fs/io-wq.c
index cb60a42b9fdf..58b1891bcfe5 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -16,6 +16,7 @@
 #include <linux/slab.h>
 #include <linux/kthread.h>
 #include <linux/rculist_nulls.h>
+#include <linux/fs_struct.h>
 
 #include "io-wq.h"
 
@@ -59,6 +60,7 @@ struct io_worker {
 	const struct cred *cur_creds;
 	const struct cred *saved_creds;
 	struct files_struct *restore_files;
+	struct fs_struct *restore_fs;
 };
 
 #if BITS_PER_LONG == 64
@@ -141,13 +143,17 @@ static bool __io_worker_unuse(struct io_wqe *wqe, struct io_worker *worker)
 		worker->cur_creds = worker->saved_creds = NULL;
 	}
 
-	if (current->files != worker->restore_files) {
+	if ((current->files != worker->restore_files) ||
+	    (current->fs != worker->restore_fs)) {
 		__acquire(&wqe->lock);
 		spin_unlock_irq(&wqe->lock);
 		dropped_lock = true;
 
 		task_lock(current);
-		current->files = worker->restore_files;
+		if (current->files != worker->restore_files)
+			current->files = worker->restore_files;
+		if (current->fs != worker->restore_fs)
+			current->fs = worker->restore_fs;
 		task_unlock(current);
 	}
 
@@ -311,6 +317,7 @@ static void io_worker_start(struct io_wqe *wqe, struct io_worker *worker)
 
 	worker->flags |= (IO_WORKER_F_UP | IO_WORKER_F_RUNNING);
 	worker->restore_files = current->files;
+	worker->restore_fs = current->fs;
 	io_wqe_inc_running(wqe, worker);
 }
 
@@ -476,9 +483,13 @@ static void io_worker_handle_work(struct io_worker *worker)
 		if (work->flags & IO_WQ_WORK_CB)
 			work->func(&work);
 
-		if (work->files && current->files != work->files) {
+		if ((work->files && current->files != work->files) ||
+		    (work->fs && current->fs != work->fs)) {
 			task_lock(current);
-			current->files = work->files;
+			if (work->files && current->files != work->files)
+				current->files = work->files;
+			if (work->fs && current->fs != work->fs)
+				current->fs = work->fs;
 			task_unlock(current);
 		}
 		if (work->mm != worker->mm)
diff --git a/fs/io-wq.h b/fs/io-wq.h
index 50b3378febf2..f152ba677d8f 100644
--- a/fs/io-wq.h
+++ b/fs/io-wq.h
@@ -74,6 +74,7 @@ struct io_wq_work {
 	struct files_struct *files;
 	struct mm_struct *mm;
 	const struct cred *creds;
+	struct fs_struct *fs;
 	unsigned flags;
 };
 
@@ -81,10 +82,11 @@ struct io_wq_work {
 	do {						\
 		(work)->list.next = NULL;		\
 		(work)->func = _func;			\
-		(work)->flags = 0;			\
 		(work)->files = NULL;			\
 		(work)->mm = NULL;			\
 		(work)->creds = NULL;			\
+		(work)->fs = NULL;			\
+		(work)->flags = 0;			\
 	} while (0)					\
 
 typedef void (get_work_fn)(struct io_wq_work *);
-- 
2.25.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/4] io_uring: add ->needs_fs to the opcode requirements table
  2020-02-07 15:50 [PATCHSET 0/4] Allow relative lookups Jens Axboe
  2020-02-07 15:50 ` [PATCH 1/4] io_uring: grab owning task fs_struct Jens Axboe
  2020-02-07 15:50 ` [PATCH 2/4] io-wq: add support for inheriting ->fs Jens Axboe
@ 2020-02-07 15:50 ` Jens Axboe
  2020-02-07 15:50 ` [PATCH 4/4] io_uring: allow AT_FDCWD for non-file openat/openat2/statx Jens Axboe
  2020-02-07 21:56 ` [PATCHSET 0/4] Allow relative lookups Stefan Metzmacher
  4 siblings, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2020-02-07 15:50 UTC (permalink / raw)
  To: io-uring; +Cc: Jens Axboe

Set it for openat/openat2/statx, as they all potentially need relative
path lookups if AT_FDCWD is used.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index da6a5998fa30..957de0f99bcd 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -616,6 +616,8 @@ struct io_op_def {
 	unsigned		not_supported : 1;
 	/* needs file table */
 	unsigned		file_table : 1;
+	/* needs ->fs assigned */
+	unsigned		needs_fs : 1;
 };
 
 static const struct io_op_def io_op_defs[] = {
@@ -694,6 +696,7 @@ static const struct io_op_def io_op_defs[] = {
 		.needs_file		= 1,
 		.fd_non_neg		= 1,
 		.file_table		= 1,
+		.needs_fs		= 1,
 	},
 	[IORING_OP_CLOSE] = {
 		.needs_file		= 1,
@@ -707,6 +710,7 @@ static const struct io_op_def io_op_defs[] = {
 		.needs_mm		= 1,
 		.needs_file		= 1,
 		.fd_non_neg		= 1,
+		.needs_fs		= 1,
 	},
 	[IORING_OP_READ] = {
 		.needs_mm		= 1,
@@ -738,6 +742,7 @@ static const struct io_op_def io_op_defs[] = {
 		.needs_file		= 1,
 		.fd_non_neg		= 1,
 		.file_table		= 1,
+		.needs_fs		= 1,
 	},
 	[IORING_OP_EPOLL_CTL] = {
 		.unbound_nonreg_file	= 1,
@@ -911,6 +916,8 @@ static inline void io_req_work_grab_env(struct io_kiocb *req,
 	}
 	if (!req->work.creds)
 		req->work.creds = get_current_cred();
+	if (!req->work.fs && def->needs_fs)
+		req->work.fs = req->ctx->fs;
 }
 
 static inline void io_req_work_drop_env(struct io_kiocb *req)
-- 
2.25.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 4/4] io_uring: allow AT_FDCWD for non-file openat/openat2/statx
  2020-02-07 15:50 [PATCHSET 0/4] Allow relative lookups Jens Axboe
                   ` (2 preceding siblings ...)
  2020-02-07 15:50 ` [PATCH 3/4] io_uring: add ->needs_fs to the opcode requirements table Jens Axboe
@ 2020-02-07 15:50 ` Jens Axboe
  2020-02-07 21:56 ` [PATCHSET 0/4] Allow relative lookups Stefan Metzmacher
  4 siblings, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2020-02-07 15:50 UTC (permalink / raw)
  To: io-uring; +Cc: Jens Axboe

Don't just check for dirfd == -1, we should allow AT_FDCWD as well for
relative lookups.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 957de0f99bcd..b35703582086 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -4467,7 +4467,7 @@ static int io_req_needs_file(struct io_kiocb *req, int fd)
 {
 	if (!io_op_defs[req->opcode].needs_file)
 		return 0;
-	if (fd == -1 && io_op_defs[req->opcode].fd_non_neg)
+	if ((fd == -1 || fd == AT_FDCWD) && io_op_defs[req->opcode].fd_non_neg)
 		return 0;
 	return 1;
 }
-- 
2.25.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCHSET 0/4] Allow relative lookups
  2020-02-07 15:50 [PATCHSET 0/4] Allow relative lookups Jens Axboe
                   ` (3 preceding siblings ...)
  2020-02-07 15:50 ` [PATCH 4/4] io_uring: allow AT_FDCWD for non-file openat/openat2/statx Jens Axboe
@ 2020-02-07 21:56 ` Stefan Metzmacher
  2020-02-07 22:47   ` Jens Axboe
  4 siblings, 1 reply; 10+ messages in thread
From: Stefan Metzmacher @ 2020-02-07 21:56 UTC (permalink / raw)
  To: Jens Axboe, io-uring

[-- Attachment #1.1: Type: text/plain, Size: 659 bytes --]

Hi Jens,

Am 07.02.20 um 16:50 schrieb Jens Axboe:
> Due to an oversight on my part, AT_FDCWD lookups only work when the
> lookup can be done inline, not async. This patchset rectifies that,
> aiming for 5.6 for this one as it would be a shame to have openat etc
> without that.
> 
> Just 3 small simple patches - grab the task ->fs, add io-wq suppor for
> passing it in and setting it, and finally add a ->needs_fs to the opcode
> table list of requirements for openat/openat2/statx.
> 
> Last patch just ensures we allow AT_FDCWD.

Thanks! But IOSQE_FIXED_FILE is still not supported and not rejected at
the same time, correct?

metze



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCHSET 0/4] Allow relative lookups
  2020-02-07 21:56 ` [PATCHSET 0/4] Allow relative lookups Stefan Metzmacher
@ 2020-02-07 22:47   ` Jens Axboe
  2020-02-07 22:56     ` Stefan Metzmacher
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Axboe @ 2020-02-07 22:47 UTC (permalink / raw)
  To: Stefan Metzmacher, io-uring

On 2/7/20 2:56 PM, Stefan Metzmacher wrote:
> Hi Jens,
> 
> Am 07.02.20 um 16:50 schrieb Jens Axboe:
>> Due to an oversight on my part, AT_FDCWD lookups only work when the
>> lookup can be done inline, not async. This patchset rectifies that,
>> aiming for 5.6 for this one as it would be a shame to have openat etc
>> without that.
>>
>> Just 3 small simple patches - grab the task ->fs, add io-wq suppor for
>> passing it in and setting it, and finally add a ->needs_fs to the opcode
>> table list of requirements for openat/openat2/statx.
>>
>> Last patch just ensures we allow AT_FDCWD.
> 
> Thanks! But IOSQE_FIXED_FILE is still not supported and not rejected at
> the same time, correct?

That's in a separate patch:

https://git.kernel.dk/cgit/linux-block/commit/?h=io_uring-5.6&id=5e159663813f0b7837342426cfb68185b6609359

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCHSET 0/4] Allow relative lookups
  2020-02-07 22:47   ` Jens Axboe
@ 2020-02-07 22:56     ` Stefan Metzmacher
  2020-02-07 23:07       ` Stefan Metzmacher
  2020-02-08 20:05       ` Jens Axboe
  0 siblings, 2 replies; 10+ messages in thread
From: Stefan Metzmacher @ 2020-02-07 22:56 UTC (permalink / raw)
  To: Jens Axboe, io-uring

[-- Attachment #1.1: Type: text/plain, Size: 1320 bytes --]

Hi Jens,

>> Am 07.02.20 um 16:50 schrieb Jens Axboe:
>>> Due to an oversight on my part, AT_FDCWD lookups only work when the
>>> lookup can be done inline, not async. This patchset rectifies that,
>>> aiming for 5.6 for this one as it would be a shame to have openat etc
>>> without that.
>>>
>>> Just 3 small simple patches - grab the task ->fs, add io-wq suppor for
>>> passing it in and setting it, and finally add a ->needs_fs to the opcode
>>> table list of requirements for openat/openat2/statx.
>>>
>>> Last patch just ensures we allow AT_FDCWD.
>>
>> Thanks! But IOSQE_FIXED_FILE is still not supported and not rejected at
>> the same time, correct?
> 
> That's in a separate patch:
> 
> https://git.kernel.dk/cgit/linux-block/commit/?h=io_uring-5.6&id=5e159663813f0b7837342426cfb68185b6609359

Do we handle the error path correct?
As far as I can see io_req_set_file() is called before
io_{statx,openat,openat2}_prep() and req->file is already filled.
Maybe a generic way would be better using io_op_defs[op].allow_fixed_file.

In the long run we may want to add support for openat2 with
IOSQE_FIXED_FILE, then Samba could register the share root directory as
fixed file and it could be used for all openat2 calls.
But for now it's fine to just reject it...

Thanks!
metze



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCHSET 0/4] Allow relative lookups
  2020-02-07 22:56     ` Stefan Metzmacher
@ 2020-02-07 23:07       ` Stefan Metzmacher
  2020-02-08 20:05       ` Jens Axboe
  1 sibling, 0 replies; 10+ messages in thread
From: Stefan Metzmacher @ 2020-02-07 23:07 UTC (permalink / raw)
  To: Jens Axboe, io-uring

[-- Attachment #1.1: Type: text/plain, Size: 1367 bytes --]

Am 07.02.20 um 23:56 schrieb Stefan Metzmacher:
> Hi Jens,
> 
>>> Am 07.02.20 um 16:50 schrieb Jens Axboe:
>>>> Due to an oversight on my part, AT_FDCWD lookups only work when the
>>>> lookup can be done inline, not async. This patchset rectifies that,
>>>> aiming for 5.6 for this one as it would be a shame to have openat etc
>>>> without that.
>>>>
>>>> Just 3 small simple patches - grab the task ->fs, add io-wq suppor for
>>>> passing it in and setting it, and finally add a ->needs_fs to the opcode
>>>> table list of requirements for openat/openat2/statx.
>>>>
>>>> Last patch just ensures we allow AT_FDCWD.
>>>
>>> Thanks! But IOSQE_FIXED_FILE is still not supported and not rejected at
>>> the same time, correct?
>>
>> That's in a separate patch:
>>
>> https://git.kernel.dk/cgit/linux-block/commit/?h=io_uring-5.6&id=5e159663813f0b7837342426cfb68185b6609359
> 
> Do we handle the error path correct?
> As far as I can see io_req_set_file() is called before
> io_{statx,openat,openat2}_prep() and req->file is already filled.
> Maybe a generic way would be better using io_op_defs[op].allow_fixed_file.

BTW: I'm really wondering what req->needs_fixed_file is for...

Does it mean that IORING_SETUP_SQPOLL only works with IOSQE_FIXED_FILE?
Which would mean IORING_SETUP_SQPOLL can't support openat2 and others.

metze



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCHSET 0/4] Allow relative lookups
  2020-02-07 22:56     ` Stefan Metzmacher
  2020-02-07 23:07       ` Stefan Metzmacher
@ 2020-02-08 20:05       ` Jens Axboe
  1 sibling, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2020-02-08 20:05 UTC (permalink / raw)
  To: Stefan Metzmacher, io-uring

On 2/7/20 3:56 PM, Stefan Metzmacher wrote:
> Hi Jens,
> 
>>> Am 07.02.20 um 16:50 schrieb Jens Axboe:
>>>> Due to an oversight on my part, AT_FDCWD lookups only work when the
>>>> lookup can be done inline, not async. This patchset rectifies that,
>>>> aiming for 5.6 for this one as it would be a shame to have openat etc
>>>> without that.
>>>>
>>>> Just 3 small simple patches - grab the task ->fs, add io-wq suppor for
>>>> passing it in and setting it, and finally add a ->needs_fs to the opcode
>>>> table list of requirements for openat/openat2/statx.
>>>>
>>>> Last patch just ensures we allow AT_FDCWD.
>>>
>>> Thanks! But IOSQE_FIXED_FILE is still not supported and not rejected at
>>> the same time, correct?
>>
>> That's in a separate patch:
>>
>> https://git.kernel.dk/cgit/linux-block/commit/?h=io_uring-5.6&id=5e159663813f0b7837342426cfb68185b6609359
> 
> Do we handle the error path correct?
> As far as I can see io_req_set_file() is called before
> io_{statx,openat,openat2}_prep() and req->file is already filled.
> Maybe a generic way would be better using io_op_defs[op].allow_fixed_file.

Worst case, as far as I can tell, is that we'll think it's a valid
descriptor (because the both the fixed index and fd are valid) and we'll
still error in the prep. Only concern would be that maybe we should make
this an -EBADF return, which would be 100% consistent between them (and
with other cases). I'll make that change.

> In the long run we may want to add support for openat2 with
> IOSQE_FIXED_FILE, then Samba could register the share root directory as
> fixed file and it could be used for all openat2 calls.
> But for now it's fine to just reject it...

It's not impossible to support, it's just that it requires changes
outside of io_uring to do so. So for now it'll just not be possible, I'm
afraid.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-07 15:50 [PATCHSET 0/4] Allow relative lookups Jens Axboe
2020-02-07 15:50 ` [PATCH 1/4] io_uring: grab owning task fs_struct Jens Axboe
2020-02-07 15:50 ` [PATCH 2/4] io-wq: add support for inheriting ->fs Jens Axboe
2020-02-07 15:50 ` [PATCH 3/4] io_uring: add ->needs_fs to the opcode requirements table Jens Axboe
2020-02-07 15:50 ` [PATCH 4/4] io_uring: allow AT_FDCWD for non-file openat/openat2/statx Jens Axboe
2020-02-07 21:56 ` [PATCHSET 0/4] Allow relative lookups Stefan Metzmacher
2020-02-07 22:47   ` Jens Axboe
2020-02-07 22:56     ` Stefan Metzmacher
2020-02-07 23:07       ` Stefan Metzmacher
2020-02-08 20:05       ` Jens Axboe

IO-Uring Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/io-uring/0 io-uring/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 io-uring io-uring/ https://lore.kernel.org/io-uring \
		io-uring@vger.kernel.org
	public-inbox-index io-uring

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.io-uring


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git