| Message ID | 20231025140205.3586473-1-mszeredi@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | querying mount attributes | expand |
On Wed, 25 Oct 2023 16:01:58 +0200, Miklos Szeredi wrote: > Implement mount querying syscalls agreed on at LSF/MM 2023. > > Features: > > - statx-like want/got mask > - allows returning ascii strings (fs type, root, mount point) > - returned buffer is relocatable (no pointers) > > [...] I think we should start showing clear signs of commitment to this. In absence of strong objections I don't see a reason to let this rot on list until we forget about it. Maybe this will entice people to provide more reviews as well. It's all pretty close to what we discussed at LSFMM23 and we stated that we aim to merge something by the end of the year. Let's see if that can actually happen. I don't have huge quarrels with this. Yes, there's stuff I'd like to see done differently but nothing I consider blockers. So let's get this into -next once rc1 is out so it can get a full cycle of exposure. I've renamed struct statmnt to struct statmount to align with statx() and struct statx. I also renamed struct stmt_state to struct kstatmount as that's how we usually do this. And I renamed struct __mount_arg to struct mnt_id_req and dropped the comment. Libraries can expose this in whatever form they want but we'll also have direct consumers. I'd rather have this struct be underscore free and officially sanctioned. --- Applied to the vfs.mount branch of the vfs/vfs.git tree. Patches in the vfs.mount branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.master [1/6] add unique mount ID https://git.kernel.org/vfs/vfs/c/ec873c3baa0c [2/6] mounts: keep list of mounts in an rbtree https://git.kernel.org/vfs/vfs/c/f15247ad234c [3/6] namespace: extract show_path() helper https://git.kernel.org/vfs/vfs/c/6e5f64ac5382 [4/6] add statmount(2) syscall https://git.kernel.org/vfs/vfs/c/edf3b2ac1bd5 [5/6] add listmount(2) syscall https://git.kernel.org/vfs/vfs/c/4412ca803757 [6/6] wire up syscalls for statmount/listmount https://git.kernel.org/vfs/vfs/c/d0a56e829d2c
On 25/10/23 22:01, Miklos Szeredi wrote: > Implement mount querying syscalls agreed on at LSF/MM 2023. > > Features: > > - statx-like want/got mask > - allows returning ascii strings (fs type, root, mount point) > - returned buffer is relocatable (no pointers) > > Still missing: > - man pages > - kselftest > > Please find the test utility at the end of this mail. > > Usage: statmnt [-l|-r] [-u] (mnt_id|path) > > Git tree: > > git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git#statmount-v4 > > > Changes v3..v4: > > - incorporate patch moving list of mounts to an rbtree > - wire up syscalls for all archs > - add LISTMOUNT_RECURSIVE (depth first iteration of mount tree) > - add LSMT_ROOT (list root instead of a specific mount ID) > - list_for_each_entry_del() moved to a separate patchset > > Changes v1..v3: > > - rename statmnt(2) -> statmount(2) > - rename listmnt(2) -> listmount(2) > - make ABI 32bit compatible by passing 64bit args in a struct (tested on > i386 and x32) > - only accept new 64bit mount IDs > - fix compile on !CONFIG_PROC_FS > - call security_sb_statfs() in both syscalls > - make lookup_mnt_in_ns() static > - add LISTMOUNT_UNREACHABLE flag to listmnt() to explicitly ask for > listing unreachable mounts > - remove .sb_opts > - remove subtype from .fs_type > - return the number of bytes used (including strings) in .size > - rename .mountpoint -> .mnt_point > - point strings by an offset against char[] VLA at the end of the struct. > E.g. printf("fs_type: %s\n", st->str + st->fs_type); > - don't save string lengths > - extend spare space in struct statmnt (complete size is now 512 bytes) > > > Miklos Szeredi (6): > add unique mount ID > mounts: keep list of mounts in an rbtree > namespace: extract show_path() helper > add statmount(2) syscall > add listmount(2) syscall > wire up syscalls for statmount/listmount > > arch/alpha/kernel/syscalls/syscall.tbl | 3 + > arch/arm/tools/syscall.tbl | 3 + > arch/arm64/include/asm/unistd32.h | 4 + > arch/ia64/kernel/syscalls/syscall.tbl | 3 + > arch/m68k/kernel/syscalls/syscall.tbl | 3 + > arch/microblaze/kernel/syscalls/syscall.tbl | 3 + > arch/mips/kernel/syscalls/syscall_n32.tbl | 3 + > arch/mips/kernel/syscalls/syscall_n64.tbl | 3 + > arch/mips/kernel/syscalls/syscall_o32.tbl | 3 + > arch/parisc/kernel/syscalls/syscall.tbl | 3 + > arch/powerpc/kernel/syscalls/syscall.tbl | 3 + > arch/s390/kernel/syscalls/syscall.tbl | 3 + > arch/sh/kernel/syscalls/syscall.tbl | 3 + > arch/sparc/kernel/syscalls/syscall.tbl | 3 + > arch/x86/entry/syscalls/syscall_32.tbl | 3 + > arch/x86/entry/syscalls/syscall_64.tbl | 2 + > arch/xtensa/kernel/syscalls/syscall.tbl | 3 + > fs/internal.h | 2 + > fs/mount.h | 27 +- > fs/namespace.c | 573 ++++++++++++++++---- > fs/pnode.c | 2 +- > fs/proc_namespace.c | 13 +- > fs/stat.c | 9 +- > include/linux/mount.h | 5 +- > include/linux/syscalls.h | 8 + > include/uapi/asm-generic/unistd.h | 8 +- > include/uapi/linux/mount.h | 65 +++ > include/uapi/linux/stat.h | 1 + > 28 files changed, 635 insertions(+), 129 deletions(-) Looks ok to me,covers the primary cases I needed when I worked on using fsinfo() in systemd. Karel, is there anything missing you would need for adding libmount support? Reviewed-by: Ian Kent <raven@themaw.net> >
On Wed, Nov 1, 2023 at 12:13 PM Christian Brauner <brauner@kernel.org> wrote: > I've renamed struct statmnt to struct statmount to align with statx() > and struct statx. I also renamed struct stmt_state to struct kstatmount > as that's how we usually do this. And I renamed struct __mount_arg to > struct mnt_id_req and dropped the comment. Libraries can expose this in > whatever form they want but we'll also have direct consumers. I'd rather > have this struct be underscore free and officially sanctioned. Thanks. arch/arm64/include/asm/unistd.h needs this fixup: -#define __NR_compat_syscalls 457 +#define __NR_compat_syscalls 459 Can you fix inline, or should I send a proper patch? Thanks, Miklos
On Wed, Nov 01, 2023 at 02:18:30PM +0100, Miklos Szeredi wrote: > On Wed, Nov 1, 2023 at 12:13 PM Christian Brauner <brauner@kernel.org> wrote: > > > I've renamed struct statmnt to struct statmount to align with statx() > > and struct statx. I also renamed struct stmt_state to struct kstatmount > > as that's how we usually do this. And I renamed struct __mount_arg to > > struct mnt_id_req and dropped the comment. Libraries can expose this in > > whatever form they want but we'll also have direct consumers. I'd rather > > have this struct be underscore free and officially sanctioned. > > Thanks. > > arch/arm64/include/asm/unistd.h needs this fixup: > > -#define __NR_compat_syscalls 457 > +#define __NR_compat_syscalls 459 Everytime with that file. It's like a tradition that I forget to update it at least once. > > Can you fix inline, or should I send a proper patch? No need to send. I'll just fix it it here.
On Wed, Nov 01, 2023 at 07:52:45PM +0800, Ian Kent wrote: > On 25/10/23 22:01, Miklos Szeredi wrote: > Looks ok to me,covers the primary cases I needed when I worked > on using fsinfo() in systemd. Our work on systemd was about two areas: get mount info (stat/listmount() now) from the kernel, and get the mount ID from notification. There was watch_queue.h with WATCH_TYPE_MOUNT_NOTIFY and struct mount_notification->auxiliary_mount (aka mount ID) and event subtype to get the change status (new mount, umount, etc.) For example David's: https://patchwork.kernel.org/project/linux-security-module/patch/155991711016.15579.4449417925184028666.stgit@warthog.procyon.org.uk/ Do we have any replacement for this? > Karel, is there anything missing you would need for adding > libmount support? Miklos's statmount() and listmount() API is excellent from my point of view. It looks pretty straightforward to use, and with the unique mount ID, it's safe too. It will be ideal for things like umount(8) (and recursive umount, etc.). For complex scenarios (systemd), we need to get from the kernel the unique ID's after any change in the mount table to save resources and call statmount() only for the affected mount node. Parse mountinfo sucks, call for(listmount(-1)) { statmount() } sucks too :-) Karel
On Mon, Nov 6, 2023 at 2:11 PM Karel Zak <kzak@redhat.com> wrote: > > On Wed, Nov 01, 2023 at 07:52:45PM +0800, Ian Kent wrote: > > On 25/10/23 22:01, Miklos Szeredi wrote: > > Looks ok to me,covers the primary cases I needed when I worked > > on using fsinfo() in systemd. > > Our work on systemd was about two areas: get mount info (stat/listmount() > now) from the kernel, and get the mount ID from notification. > > There was watch_queue.h with WATCH_TYPE_MOUNT_NOTIFY and struct > mount_notification->auxiliary_mount (aka mount ID) and event subtype > to get the change status (new mount, umount, etc.) > > For example David's: > https://patchwork.kernel.org/project/linux-security-module/patch/155991711016.15579.4449417925184028666.stgit@warthog.procyon.org.uk/ > > Do we have any replacement for this? > The plan is to extend fanotify for mount namespace change notifications. Here is a simple POC for FAN_UNMOUNT notification: https://lore.kernel.org/linux-fsdevel/20230414182903.1852019-1-amir73il@gmail.com/ I was waiting for Miklos' patches to land, so that we can report mnt_id_unique (of mount and its parent mount) in the events. The plan is to start with setting a mark on a vfsmount to get FAN_MOUNT/FAN_UNMOUNT notifications for changes to direct children of that mount. This part, I was planning to do myself. I cannot say for sure when I will be able to get to it, but it should be a rather simple patch. If anybody else would like to volunteer for the task, I will be happy to assist. Not sure if we are going to need special notifications for mount move and mount beneath? Not sure if we are going to need notifications on mount attribute changes? We may later also implement a mark on a mount namespace to get events on all mount namespace changes. If you have any feedback about this rough plan, or more items to the wish list, please feel free to share them. Thanks, Amir.
On 6/11/23 20:10, Karel Zak wrote: > On Wed, Nov 01, 2023 at 07:52:45PM +0800, Ian Kent wrote: >> On 25/10/23 22:01, Miklos Szeredi wrote: >> Looks ok to me,covers the primary cases I needed when I worked >> on using fsinfo() in systemd. > Our work on systemd was about two areas: get mount info (stat/listmount() > now) from the kernel, and get the mount ID from notification. > > There was watch_queue.h with WATCH_TYPE_MOUNT_NOTIFY and struct > mount_notification->auxiliary_mount (aka mount ID) and event subtype > to get the change status (new mount, umount, etc.) > > For example David's: > https://patchwork.kernel.org/project/linux-security-module/patch/155991711016.15579.4449417925184028666.stgit@warthog.procyon.org.uk/ > > Do we have any replacement for this? Not yet. I tried to mention it early on but I don't think my description conveyed what's actually needed. > >> Karel, is there anything missing you would need for adding >> libmount support? > Miklos's statmount() and listmount() API is excellent from my point of > view. It looks pretty straightforward to use, and with the unique > mount ID, it's safe too. It will be ideal for things like umount(8) > (and recursive umount, etc.). Thanks Karel, that's what I was hoping. > > For complex scenarios (systemd), we need to get from the kernel the > unique ID's after any change in the mount table to save resources and > call statmount() only for the affected mount node. Parse mountinfo > sucks, call for(listmount(-1)) { statmount() } sucks too :-) I have been looking at the notifications side of things. I too need that functionality for the systemd work I was doing on this. There was a need for event rate management too to get the most out of the mount query improvements which I really only realized about the time the work stopped. So for me there's some new work needed as well. I'm not sure yet which way to go as the watch queue implementation that was merged is just the framework and is a bit different from what we were using so I'm not sure if I can port specific extensions of David's notifications work to it. I'm only just now getting to a point where I can spend enough time on it to work this out. Ian
On 6/11/23 21:33, Amir Goldstein wrote: > On Mon, Nov 6, 2023 at 2:11 PM Karel Zak <kzak@redhat.com> wrote: >> On Wed, Nov 01, 2023 at 07:52:45PM +0800, Ian Kent wrote: >>> On 25/10/23 22:01, Miklos Szeredi wrote: >>> Looks ok to me,covers the primary cases I needed when I worked >>> on using fsinfo() in systemd. >> Our work on systemd was about two areas: get mount info (stat/listmount() >> now) from the kernel, and get the mount ID from notification. >> >> There was watch_queue.h with WATCH_TYPE_MOUNT_NOTIFY and struct >> mount_notification->auxiliary_mount (aka mount ID) and event subtype >> to get the change status (new mount, umount, etc.) >> >> For example David's: >> https://patchwork.kernel.org/project/linux-security-module/patch/155991711016.15579.4449417925184028666.stgit@warthog.procyon.org.uk/ >> >> Do we have any replacement for this? >> > The plan is to extend fanotify for mount namespace change notifications. > > Here is a simple POC for FAN_UNMOUNT notification: > > https://lore.kernel.org/linux-fsdevel/20230414182903.1852019-1-amir73il@gmail.com/ > > I was waiting for Miklos' patches to land, so that we can report > mnt_id_unique (of mount and its parent mount) in the events. > > The plan is to start with setting a mark on a vfsmount to get > FAN_MOUNT/FAN_UNMOUNT notifications for changes to direct > children of that mount. I'll have a look at what I needed when I was working to implement this in systemd. Without looking at the code I can say I was handling mount, umount and I think remount events so that's probably a minimum. As I mentioned earlier I found I also need event rate management which was a new requirement at the time. > > This part, I was planning to do myself. I cannot say for sure when > I will be able to get to it, but it should be a rather simple patch. > > If anybody else would like to volunteer for the task, I will be > happy to assist. I would like to help with this but I'm not familiar with fanotify so I'll need to spend a bit of time on that. I am just about in a position to do that now. I'll also be looking at the watch queue framework that did get merged back then, I'm not sure how that will turn out. > > Not sure if we are going to need special notifications for mount > move and mount beneath? Yes that will be an interesting question, I have noticed Christians' work on mount beneath. We need to provide the ability to monitor mount tables as is done by using the proc mount lists to start with and I'm pretty sure that includes at least mount, umount and moves perhaps more but I'll check what I was using. > > Not sure if we are going to need notifications on mount attribute > changes? Also an interesting question, we will see in time I guess. You would think that the mount/umount/move events would get what's needed because (assuming mount move maps to remount) mount, umount and remount should cover cases were mounted mount attributes change. > > We may later also implement a mark on a mount namespace > to get events on all mount namespace changes. Monitoring the proc mount tables essentially provides lists of mounts that are present in a mount namespace (as seen by the given process) so this is going to be needed sooner rather than later if we hope to realize improvements from our new system calls. Ian
Implement mount querying syscalls agreed on at LSF/MM 2023. Features: - statx-like want/got mask - allows returning ascii strings (fs type, root, mount point) - returned buffer is relocatable (no pointers) Still missing: - man pages - kselftest Please find the test utility at the end of this mail. Usage: statmnt [-l|-r] [-u] (mnt_id|path) Git tree: git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git#statmount-v4 Changes v3..v4: - incorporate patch moving list of mounts to an rbtree - wire up syscalls for all archs - add LISTMOUNT_RECURSIVE (depth first iteration of mount tree) - add LSMT_ROOT (list root instead of a specific mount ID) - list_for_each_entry_del() moved to a separate patchset Changes v1..v3: - rename statmnt(2) -> statmount(2) - rename listmnt(2) -> listmount(2) - make ABI 32bit compatible by passing 64bit args in a struct (tested on i386 and x32) - only accept new 64bit mount IDs - fix compile on !CONFIG_PROC_FS - call security_sb_statfs() in both syscalls - make lookup_mnt_in_ns() static - add LISTMOUNT_UNREACHABLE flag to listmnt() to explicitly ask for listing unreachable mounts - remove .sb_opts - remove subtype from .fs_type - return the number of bytes used (including strings) in .size - rename .mountpoint -> .mnt_point - point strings by an offset against char[] VLA at the end of the struct. E.g. printf("fs_type: %s\n", st->str + st->fs_type); - don't save string lengths - extend spare space in struct statmnt (complete size is now 512 bytes) Miklos Szeredi (6): add unique mount ID mounts: keep list of mounts in an rbtree namespace: extract show_path() helper add statmount(2) syscall add listmount(2) syscall wire up syscalls for statmount/listmount arch/alpha/kernel/syscalls/syscall.tbl | 3 + arch/arm/tools/syscall.tbl | 3 + arch/arm64/include/asm/unistd32.h | 4 + arch/ia64/kernel/syscalls/syscall.tbl | 3 + arch/m68k/kernel/syscalls/syscall.tbl | 3 + arch/microblaze/kernel/syscalls/syscall.tbl | 3 + arch/mips/kernel/syscalls/syscall_n32.tbl | 3 + arch/mips/kernel/syscalls/syscall_n64.tbl | 3 + arch/mips/kernel/syscalls/syscall_o32.tbl | 3 + arch/parisc/kernel/syscalls/syscall.tbl | 3 + arch/powerpc/kernel/syscalls/syscall.tbl | 3 + arch/s390/kernel/syscalls/syscall.tbl | 3 + arch/sh/kernel/syscalls/syscall.tbl | 3 + arch/sparc/kernel/syscalls/syscall.tbl | 3 + arch/x86/entry/syscalls/syscall_32.tbl | 3 + arch/x86/entry/syscalls/syscall_64.tbl | 2 + arch/xtensa/kernel/syscalls/syscall.tbl | 3 + fs/internal.h | 2 + fs/mount.h | 27 +- fs/namespace.c | 573 ++++++++++++++++---- fs/pnode.c | 2 +- fs/proc_namespace.c | 13 +- fs/stat.c | 9 +- include/linux/mount.h | 5 +- include/linux/syscalls.h | 8 + include/uapi/asm-generic/unistd.h | 8 +- include/uapi/linux/mount.h | 65 +++ include/uapi/linux/stat.h | 1 + 28 files changed, 635 insertions(+), 129 deletions(-)