* [PATCH v3 0/2] io-uring: Make statx api stable
@ 2022-02-15 18:03 Stefan Roesch
2022-02-15 18:03 ` [PATCH v3 1/2] fs: replace const char* parameter in vfs_statx and do_statx with struct filename Stefan Roesch
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Stefan Roesch @ 2022-02-15 18:03 UTC (permalink / raw)
To: io-uring, linux-fsdevel, kernel-team; +Cc: viro, shr
One of the key architectual tenets of io-uring is to keep the
parameters for io-uring stable. After the call has been submitted,
its value can be changed. Unfortunaltely this is not the case for
the current statx implementation.
Patches:
Patch 1: fs: replace const char* parameter in vfs_statx and do_statx with
struct filename
Create filename object outside of do_statx and vfs_statx, so io-uring
can create the filename object during the prepare phase
Patch 2: io-uring: Copy path name during prepare stage for statx
Create and store filename object during prepare phase
There is also a patch for the liburing libray to add a new test case. This
patch makes sure that the api is stable.
"liburing: add test for stable statx api"
The patch has been tested with the liburing test suite and fstests.
Changes:
V2: don't check name in vfs_fstatat
V3: don't check name in statx syscall
Stefan Roesch (2):
fs: replace const char* parameter in vfs_statx and do_statx with
struct filename
io-uring: Copy path name during prepare stage for statx
fs/internal.h | 4 +++-
fs/io_uring.c | 22 ++++++++++++++++++++--
fs/stat.c | 47 ++++++++++++++++++++++++++++++++++-------------
3 files changed, 57 insertions(+), 16 deletions(-)
base-commit: 705d84a366cfccda1e7aec1113a5399cd2ffee7d
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 1/2] fs: replace const char* parameter in vfs_statx and do_statx with struct filename 2022-02-15 18:03 [PATCH v3 0/2] io-uring: Make statx api stable Stefan Roesch @ 2022-02-15 18:03 ` Stefan Roesch 2022-02-15 18:03 ` [PATCH v3 2/2] io-uring: Copy path name during prepare stage for statx Stefan Roesch ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Stefan Roesch @ 2022-02-15 18:03 UTC (permalink / raw) To: io-uring, linux-fsdevel, kernel-team; +Cc: viro, shr This replaces the const char* __user filename parameter in the two functions do_statx and vfs_statx with a struct filename *. In addition to be able to correctly construct a filename object a new helper function getname_statx_lookup_flags is introduced. The function makes sure that do_statx and vfs_statx is invoked with the correct lookup flags. Signed-off-by: Stefan Roesch <shr@fb.com> --- fs/internal.h | 4 +++- fs/stat.c | 47 ++++++++++++++++++++++++++++++++++------------- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/fs/internal.h b/fs/internal.h index 8590c973c2f4..56c0477f4215 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -184,7 +184,9 @@ int sb_init_dio_done_wq(struct super_block *sb); /* * fs/stat.c: */ -int do_statx(int dfd, const char __user *filename, unsigned flags, + +int getname_statx_lookup_flags(int flags); +int do_statx(int dfd, struct filename *filename, unsigned int flags, unsigned int mask, struct statx __user *buffer); /* diff --git a/fs/stat.c b/fs/stat.c index 28d2020ba1f4..f0a9702cee67 100644 --- a/fs/stat.c +++ b/fs/stat.c @@ -184,6 +184,20 @@ int vfs_fstat(int fd, struct kstat *stat) return error; } +int getname_statx_lookup_flags(int flags) +{ + int lookup_flags = 0; + + if (!(flags & AT_SYMLINK_NOFOLLOW)) + lookup_flags |= LOOKUP_FOLLOW; + if (!(flags & AT_NO_AUTOMOUNT)) + lookup_flags |= LOOKUP_AUTOMOUNT; + if (flags & AT_EMPTY_PATH) + lookup_flags |= LOOKUP_EMPTY; + + return lookup_flags; +} + /** * vfs_statx - Get basic and extra attributes by filename * @dfd: A file descriptor representing the base dir for a relative filename @@ -199,7 +213,7 @@ int vfs_fstat(int fd, struct kstat *stat) * * 0 will be returned on success, and a -ve error code if unsuccessful. */ -static int vfs_statx(int dfd, const char __user *filename, int flags, +static int vfs_statx(int dfd, struct filename *filename, int flags, struct kstat *stat, u32 request_mask) { struct path path; @@ -210,15 +224,8 @@ static int vfs_statx(int dfd, const char __user *filename, int flags, AT_STATX_SYNC_TYPE)) return -EINVAL; - if (!(flags & AT_SYMLINK_NOFOLLOW)) - lookup_flags |= LOOKUP_FOLLOW; - if (!(flags & AT_NO_AUTOMOUNT)) - lookup_flags |= LOOKUP_AUTOMOUNT; - if (flags & AT_EMPTY_PATH) - lookup_flags |= LOOKUP_EMPTY; - retry: - error = user_path_at(dfd, filename, lookup_flags, &path); + error = filename_lookup(dfd, filename, flags, &path, NULL); if (error) goto out; @@ -240,8 +247,15 @@ static int vfs_statx(int dfd, const char __user *filename, int flags, int vfs_fstatat(int dfd, const char __user *filename, struct kstat *stat, int flags) { - return vfs_statx(dfd, filename, flags | AT_NO_AUTOMOUNT, - stat, STATX_BASIC_STATS); + int ret; + int statx_flags = flags | AT_NO_AUTOMOUNT; + struct filename *name; + + name = getname_flags(filename, getname_statx_lookup_flags(statx_flags), NULL); + ret = vfs_statx(dfd, name, statx_flags, stat, STATX_BASIC_STATS); + putname(name); + + return ret; } #ifdef __ARCH_WANT_OLD_STAT @@ -602,7 +616,7 @@ cp_statx(const struct kstat *stat, struct statx __user *buffer) return copy_to_user(buffer, &tmp, sizeof(tmp)) ? -EFAULT : 0; } -int do_statx(int dfd, const char __user *filename, unsigned flags, +int do_statx(int dfd, struct filename *filename, unsigned int flags, unsigned int mask, struct statx __user *buffer) { struct kstat stat; @@ -636,7 +650,14 @@ SYSCALL_DEFINE5(statx, unsigned int, mask, struct statx __user *, buffer) { - return do_statx(dfd, filename, flags, mask, buffer); + int ret; + struct filename *name; + + name = getname_flags(filename, getname_statx_lookup_flags(flags), NULL); + ret = do_statx(dfd, name, flags, mask, buffer); + putname(name); + + return ret; } #ifdef CONFIG_COMPAT -- 2.30.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] io-uring: Copy path name during prepare stage for statx 2022-02-15 18:03 [PATCH v3 0/2] io-uring: Make statx api stable Stefan Roesch 2022-02-15 18:03 ` [PATCH v3 1/2] fs: replace const char* parameter in vfs_statx and do_statx with struct filename Stefan Roesch @ 2022-02-15 18:03 ` Stefan Roesch 2022-02-18 16:15 ` [PATCH v3 0/2] io-uring: Make statx api stable Jens Axboe 2022-02-22 18:45 ` Jens Axboe 3 siblings, 0 replies; 10+ messages in thread From: Stefan Roesch @ 2022-02-15 18:03 UTC (permalink / raw) To: io-uring, linux-fsdevel, kernel-team; +Cc: viro, shr One of the key architectual tenets is to keep the parameters for io-uring stable. After the call has been submitted, its value can be changed. Unfortunaltely this is not the case for the current statx implementation. This changes replaces the const char * filename pointer in the io_statx structure with a struct filename *. In addition it also creates the filename object during the prepare phase. With this change, the opcode also needs to invoke cleanup, so the filename object gets freed after processing the request. Signed-off-by: Stefan Roesch <shr@fb.com> --- fs/io_uring.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 77b9c7e4793b..28b09b163df1 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -642,7 +642,7 @@ struct io_statx { int dfd; unsigned int mask; unsigned int flags; - const char __user *filename; + struct filename *filename; struct statx __user *buffer; }; @@ -4721,6 +4721,8 @@ static int io_fadvise(struct io_kiocb *req, unsigned int issue_flags) static int io_statx_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) { + const char __user *path; + if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL)) return -EINVAL; if (sqe->ioprio || sqe->buf_index || sqe->splice_fd_in) @@ -4730,10 +4732,22 @@ static int io_statx_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) req->statx.dfd = READ_ONCE(sqe->fd); req->statx.mask = READ_ONCE(sqe->len); - req->statx.filename = u64_to_user_ptr(READ_ONCE(sqe->addr)); + path = u64_to_user_ptr(READ_ONCE(sqe->addr)); req->statx.buffer = u64_to_user_ptr(READ_ONCE(sqe->addr2)); req->statx.flags = READ_ONCE(sqe->statx_flags); + req->statx.filename = getname_flags(path, + getname_statx_lookup_flags(req->statx.flags), + NULL); + + if (IS_ERR(req->statx.filename)) { + int ret = PTR_ERR(req->statx.filename); + + req->statx.filename = NULL; + return ret; + } + + req->flags |= REQ_F_NEED_CLEANUP; return 0; } @@ -6708,6 +6722,10 @@ static void io_clean_op(struct io_kiocb *req) putname(req->hardlink.oldpath); putname(req->hardlink.newpath); break; + case IORING_OP_STATX: + if (req->statx.filename) + putname(req->statx.filename); + break; } } if ((req->flags & REQ_F_POLLED) && req->apoll) { -- 2.30.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/2] io-uring: Make statx api stable 2022-02-15 18:03 [PATCH v3 0/2] io-uring: Make statx api stable Stefan Roesch 2022-02-15 18:03 ` [PATCH v3 1/2] fs: replace const char* parameter in vfs_statx and do_statx with struct filename Stefan Roesch 2022-02-15 18:03 ` [PATCH v3 2/2] io-uring: Copy path name during prepare stage for statx Stefan Roesch @ 2022-02-18 16:15 ` Jens Axboe 2022-02-22 18:45 ` Jens Axboe 2022-02-22 18:45 ` Jens Axboe 3 siblings, 1 reply; 10+ messages in thread From: Jens Axboe @ 2022-02-18 16:15 UTC (permalink / raw) To: Stefan Roesch, io-uring, linux-fsdevel, kernel-team; +Cc: viro On 2/15/22 11:03 AM, Stefan Roesch wrote: > One of the key architectual tenets of io-uring is to keep the > parameters for io-uring stable. After the call has been submitted, > its value can be changed. Unfortunaltely this is not the case for > the current statx implementation. > > Patches: > Patch 1: fs: replace const char* parameter in vfs_statx and do_statx with > struct filename > Create filename object outside of do_statx and vfs_statx, so io-uring > can create the filename object during the prepare phase > > Patch 2: io-uring: Copy path name during prepare stage for statx > Create and store filename object during prepare phase > > > There is also a patch for the liburing libray to add a new test case. This > patch makes sure that the api is stable. > "liburing: add test for stable statx api" > > The patch has been tested with the liburing test suite and fstests. Al, are you happy with this version? -- Jens Axboe ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/2] io-uring: Make statx api stable 2022-02-18 16:15 ` [PATCH v3 0/2] io-uring: Make statx api stable Jens Axboe @ 2022-02-22 18:45 ` Jens Axboe 0 siblings, 0 replies; 10+ messages in thread From: Jens Axboe @ 2022-02-22 18:45 UTC (permalink / raw) To: Stefan Roesch, io-uring, linux-fsdevel, kernel-team; +Cc: viro, Linus Torvalds On 2/18/22 9:15 AM, Jens Axboe wrote: > On 2/15/22 11:03 AM, Stefan Roesch wrote: >> One of the key architectual tenets of io-uring is to keep the >> parameters for io-uring stable. After the call has been submitted, >> its value can be changed. Unfortunaltely this is not the case for >> the current statx implementation. >> >> Patches: >> Patch 1: fs: replace const char* parameter in vfs_statx and do_statx with >> struct filename >> Create filename object outside of do_statx and vfs_statx, so io-uring >> can create the filename object during the prepare phase >> >> Patch 2: io-uring: Copy path name during prepare stage for statx >> Create and store filename object during prepare phase >> >> >> There is also a patch for the liburing libray to add a new test case. This >> patch makes sure that the api is stable. >> "liburing: add test for stable statx api" >> >> The patch has been tested with the liburing test suite and fstests. > > Al, are you happy with this version? I have staged this one for 5.18, it's in for-5.18/io_uring-statx. It will be sent separately from the general io_uring fixes/updates. -- Jens Axboe ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/2] io-uring: Make statx api stable 2022-02-15 18:03 [PATCH v3 0/2] io-uring: Make statx api stable Stefan Roesch ` (2 preceding siblings ...) 2022-02-18 16:15 ` [PATCH v3 0/2] io-uring: Make statx api stable Jens Axboe @ 2022-02-22 18:45 ` Jens Axboe [not found] ` <CGME20220224124715eucas1p2a7d1b7f2a5131ef1dd5b8280c1d3749b@eucas1p2.samsung.com> 3 siblings, 1 reply; 10+ messages in thread From: Jens Axboe @ 2022-02-22 18:45 UTC (permalink / raw) To: kernel-team, Stefan Roesch, io-uring, linux-fsdevel; +Cc: viro On Tue, 15 Feb 2022 10:03:26 -0800, Stefan Roesch wrote: > One of the key architectual tenets of io-uring is to keep the > parameters for io-uring stable. After the call has been submitted, > its value can be changed. Unfortunaltely this is not the case for > the current statx implementation. > > Patches: > Patch 1: fs: replace const char* parameter in vfs_statx and do_statx with > struct filename > Create filename object outside of do_statx and vfs_statx, so io-uring > can create the filename object during the prepare phase > > [...] Applied, thanks! [1/2] fs: replace const char* parameter in vfs_statx and do_statx with struct filename commit: 30512d54fae354a2359a740b75a1451b68aa3807 [2/2] io-uring: Copy path name during prepare stage for statx commit: 1e0561928e3ab5018615403a2a1293e1e44ee03e Best regards, -- Jens Axboe ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CGME20220224124715eucas1p2a7d1b7f2a5131ef1dd5b8280c1d3749b@eucas1p2.samsung.com>]
* Re: [PATCH v3 0/2] io-uring: Make statx api stable [not found] ` <CGME20220224124715eucas1p2a7d1b7f2a5131ef1dd5b8280c1d3749b@eucas1p2.samsung.com> @ 2022-02-24 12:47 ` Marek Szyprowski 2022-02-24 14:09 ` Jens Axboe 0 siblings, 1 reply; 10+ messages in thread From: Marek Szyprowski @ 2022-02-24 12:47 UTC (permalink / raw) To: Jens Axboe, kernel-team, Stefan Roesch, io-uring, linux-fsdevel; +Cc: viro Hi, On 22.02.2022 19:45, Jens Axboe wrote: > On Tue, 15 Feb 2022 10:03:26 -0800, Stefan Roesch wrote: >> One of the key architectual tenets of io-uring is to keep the >> parameters for io-uring stable. After the call has been submitted, >> its value can be changed. Unfortunaltely this is not the case for >> the current statx implementation. >> >> Patches: >> Patch 1: fs: replace const char* parameter in vfs_statx and do_statx with >> struct filename >> Create filename object outside of do_statx and vfs_statx, so io-uring >> can create the filename object during the prepare phase >> >> [...] > Applied, thanks! > > [1/2] fs: replace const char* parameter in vfs_statx and do_statx with struct filename > commit: 30512d54fae354a2359a740b75a1451b68aa3807 > [2/2] io-uring: Copy path name during prepare stage for statx > commit: 1e0561928e3ab5018615403a2a1293e1e44ee03e Those 2 commits landed in todays Linux next-20220223. They affect userspace in a way that breaks systemd opration: ... Freeing unused kernel image (initmem) memory: 1024K Run /sbin/init as init process systemd[1]: System time before build time, advancing clock. systemd[1]: Cannot be run in a chroot() environment. systemd[1]: Freezing execution. Reverting them on top of next-20220223 fixes the boot issue. Btw, those patches are not bisectable. The code at 30512d54fae354a2359a740b75a1451b68aa3807 doesn't compile. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/2] io-uring: Make statx api stable 2022-02-24 12:47 ` Marek Szyprowski @ 2022-02-24 14:09 ` Jens Axboe 2022-02-25 1:27 ` Qian Cai 0 siblings, 1 reply; 10+ messages in thread From: Jens Axboe @ 2022-02-24 14:09 UTC (permalink / raw) To: Marek Szyprowski, kernel-team, Stefan Roesch, io-uring, linux-fsdevel Cc: viro On 2/24/22 5:47 AM, Marek Szyprowski wrote: > Hi, > > On 22.02.2022 19:45, Jens Axboe wrote: >> On Tue, 15 Feb 2022 10:03:26 -0800, Stefan Roesch wrote: >>> One of the key architectual tenets of io-uring is to keep the >>> parameters for io-uring stable. After the call has been submitted, >>> its value can be changed. Unfortunaltely this is not the case for >>> the current statx implementation. >>> >>> Patches: >>> Patch 1: fs: replace const char* parameter in vfs_statx and do_statx with >>> struct filename >>> Create filename object outside of do_statx and vfs_statx, so io-uring >>> can create the filename object during the prepare phase >>> >>> [...] >> Applied, thanks! >> >> [1/2] fs: replace const char* parameter in vfs_statx and do_statx with struct filename >> commit: 30512d54fae354a2359a740b75a1451b68aa3807 >> [2/2] io-uring: Copy path name during prepare stage for statx >> commit: 1e0561928e3ab5018615403a2a1293e1e44ee03e > > Those 2 commits landed in todays Linux next-20220223. They affect > userspace in a way that breaks systemd opration: > > ... > > Freeing unused kernel image (initmem) memory: 1024K > Run /sbin/init as init process > systemd[1]: System time before build time, advancing clock. > systemd[1]: Cannot be run in a chroot() environment. > systemd[1]: Freezing execution. > > Reverting them on top of next-20220223 fixes the boot issue. Btw, those > patches are not bisectable. The code at > 30512d54fae354a2359a740b75a1451b68aa3807 doesn't compile. Thanks, I'll drop them from for-next until we figure out what that is. -- Jens Axboe ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/2] io-uring: Make statx api stable 2022-02-24 14:09 ` Jens Axboe @ 2022-02-25 1:27 ` Qian Cai 2022-02-26 1:22 ` Luis Chamberlain 0 siblings, 1 reply; 10+ messages in thread From: Qian Cai @ 2022-02-25 1:27 UTC (permalink / raw) To: Jens Axboe Cc: Marek Szyprowski, kernel-team, Stefan Roesch, io-uring, linux-fsdevel, viro On Thu, Feb 24, 2022 at 07:09:35AM -0700, Jens Axboe wrote: > > Freeing unused kernel image (initmem) memory: 1024K > > Run /sbin/init as init process > > systemd[1]: System time before build time, advancing clock. > > systemd[1]: Cannot be run in a chroot() environment. > > systemd[1]: Freezing execution. > > > > Reverting them on top of next-20220223 fixes the boot issue. Btw, those > > patches are not bisectable. The code at > > 30512d54fae354a2359a740b75a1451b68aa3807 doesn't compile. > > Thanks, I'll drop them from for-next until we figure out what that is. FYI, this breaks the boot on bare-metal as well. Loading, please wait... Starting version 245.4-4ubuntu3.15 Running in chroot, ignoring request. Running in chroot, ignoring request. Running in chroot, ignoring request. Running in chroot, ignoring request. Running in chroot, ignoring request. /scripts/init-top/console_setup: line 90: can't create /dev/tty1: No such device or address /scripts/init-top/console_setup: line 126: can't open /dev/tty1: No such device or address Begin: Loading essential drivers ... /init: line 155: modprobe: Permission denied /init: line 155: modprobe: Permission denied /init: line 155: modprobe: Permission denied /init: line 155: modprobe: Permission denied /init: line 155: modprobe: Permission denied /init: line 155: modprobe: Permission denied /init: line 155: modprobe: Permission denied /init: line 155: modprobe: Permission denied ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/2] io-uring: Make statx api stable 2022-02-25 1:27 ` Qian Cai @ 2022-02-26 1:22 ` Luis Chamberlain 0 siblings, 0 replies; 10+ messages in thread From: Luis Chamberlain @ 2022-02-26 1:22 UTC (permalink / raw) To: Qian Cai, Jens Axboe Cc: Marek Szyprowski, kernel-team, Stefan Roesch, io-uring, linux-fsdevel, viro On Thu, Feb 24, 2022 at 08:27:24PM -0500, Qian Cai wrote: > On Thu, Feb 24, 2022 at 07:09:35AM -0700, Jens Axboe wrote: > > > Freeing unused kernel image (initmem) memory: 1024K > > > Run /sbin/init as init process > > > systemd[1]: System time before build time, advancing clock. > > > systemd[1]: Cannot be run in a chroot() environment. > > > systemd[1]: Freezing execution. > > > > > > Reverting them on top of next-20220223 fixes the boot issue. Btw, those > > > patches are not bisectable. The code at > > > 30512d54fae354a2359a740b75a1451b68aa3807 doesn't compile. > > > > Thanks, I'll drop them from for-next until we figure out what that is. > > FYI, this breaks the boot on bare-metal as well. It also broke my boot on a simple debian testing KVM guest as well. Reverting those two commits fixes my boot. Jens, any chance we can include / ask for a bit more run time testing? What sort of tests get run ? Luis ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-02-26 1:22 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-15 18:03 [PATCH v3 0/2] io-uring: Make statx api stable Stefan Roesch
2022-02-15 18:03 ` [PATCH v3 1/2] fs: replace const char* parameter in vfs_statx and do_statx with struct filename Stefan Roesch
2022-02-15 18:03 ` [PATCH v3 2/2] io-uring: Copy path name during prepare stage for statx Stefan Roesch
2022-02-18 16:15 ` [PATCH v3 0/2] io-uring: Make statx api stable Jens Axboe
2022-02-22 18:45 ` Jens Axboe
2022-02-22 18:45 ` Jens Axboe
[not found] ` <CGME20220224124715eucas1p2a7d1b7f2a5131ef1dd5b8280c1d3749b@eucas1p2.samsung.com>
2022-02-24 12:47 ` Marek Szyprowski
2022-02-24 14:09 ` Jens Axboe
2022-02-25 1:27 ` Qian Cai
2022-02-26 1:22 ` Luis Chamberlain
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.