@@ -142,13 +142,9 @@ static int show_mountinfo(struct seq_file *m, struct vfsmount *mnt)seq_printf(m,"%i %i %u:%u ",r->mnt_id,r->mnt_parent->mnt_id,MAJOR(sb->s_dev),MINOR(sb->s_dev));-if(sb->s_op->show_path){-err=sb->s_op->show_path(m,mnt->mnt_root);-if(err)-gotoout;-}else{-seq_dentry(m,mnt->mnt_root," \t\n\\");-}+err=show_path(m,mnt->mnt_root);+if(err)+gotoout;seq_putc(m,' ');/* mountpoints outside of chroot jail will give SEQ_SKIP on this */
If a mount is released then its mnt_id can immediately be reused. This is
bad news for user interfaces that want to uniquely identify a mount.
Implementing a unique mount ID is trivial (use a 64bit counter).
Unfortunately userspace assumes 32bit size and would overflow after the
counter reaches 2^32.
Introduce a new 64bit ID alongside the old one. Initialize the counter to
2^32, this guarantees that the old and new IDs are never mixed up.
Signed-off-by: Miklos Szeredi <redacted>
---
fs/mount.h | 3 ++-
fs/namespace.c | 4 ++++
fs/stat.c | 9 +++++++--
include/uapi/linux/stat.h | 1 +
4 files changed, 14 insertions(+), 3 deletions(-)
@@ -72,7 +72,8 @@ struct mount {structfsnotify_mark_connector__rcu*mnt_fsnotify_marks;__u32mnt_fsnotify_mask;#endif-intmnt_id;/* mount identifier */+intmnt_id;/* mount identifier, reused */+u64mnt_id_unique;/* mount ID unique until reboot */intmnt_group_id;/* peer group identifier */intmnt_expiry_mark;/* true if marked for expiry */structhlist_headmnt_pins;
@@ -68,6 +68,9 @@ static u64 event;staticDEFINE_IDA(mnt_id_ida);staticDEFINE_IDA(mnt_group_ida);+/* Don't allow confusion with old 32bit mount ID */+staticatomic64_tmnt_id_ctr=ATOMIC64_INIT(1ULL<<32);+staticstructhlist_head*mount_hashtable__read_mostly;staticstructhlist_head*mountpoint_hashtable__read_mostly;staticstructkmem_cache*mnt_cache__read_mostly;
@@ -131,6 +134,7 @@ static int mnt_alloc_id(struct mount *mnt)if(res<0)returnres;mnt->mnt_id=res;+mnt->mnt_id_unique=atomic64_inc_return(&mnt_id_ctr);return0;}
Add a way to query attributes of a single mount instead of having to parse
the complete /proc/$PID/mountinfo, which might be huge.
Lookup the mount the new 64bit mount ID. If a mount needs to be queried
based on path, then statx(2) can be used to first query the mount ID
belonging to the path.
Design is based on a suggestion by Linus:
"So I'd suggest something that is very much like "statfsat()", which gets
a buffer and a length, and returns an extended "struct statfs" *AND*
just a string description at the end."
The interface closely mimics that of statx.
Handle ASCII attributes by appending after the end of the structure (as per
above suggestion). Pointers to strings are stored in u64 members to make
the structure the same regardless of pointer size. Strings are nul
terminated.
Link: https://lore.kernel.org/all/CAHk-=wh5YifP7hzKSbwJj94+DZ2czjrZsczy6GBimiogZws=rg@mail.gmail.com/
Signed-off-by: Miklos Szeredi <redacted>
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
fs/namespace.c | 283 +++++++++++++++++++++++++
fs/statfs.c | 1 +
include/linux/syscalls.h | 5 +
include/uapi/asm-generic/unistd.h | 5 +-
include/uapi/linux/mount.h | 56 +++++
7 files changed, 351 insertions(+), 1 deletion(-)
@@ -375,6 +375,7 @@ 451 common cachestat sys_cachestat 452 common fchmodat2 sys_fchmodat2 453 64 map_shadow_stack sys_map_shadow_stack+454 common statmount sys_statmount # # Due to a historical design error, certain syscalls are numbered differently
@@ -4687,6 +4687,289 @@ int show_path(struct seq_file *m, struct dentry *root)return0;}+staticstructvfsmount*lookup_mnt_in_ns(u64id,structmnt_namespace*ns)+{+structmount*mnt;+structvfsmount*res=NULL;++lock_ns_list(ns);+list_for_each_entry(mnt,&ns->list,mnt_list){+if(!mnt_is_cursor(mnt)&&id==mnt->mnt_id_unique){+res=&mnt->mnt;+break;+}+}+unlock_ns_list(ns);+returnres;+}++structstmt_state{+structstatmnt__user*constbuf;+size_tconstbufsize;+structvfsmount*constmnt;+u64constmask;+structseq_fileseq;+structpathroot;+structstatmntsm;+size_tpos;+interr;+};++typedefint(*stmt_func_t)(structstmt_state*);++staticintstmt_string_seq(structstmt_state*s,stmt_func_tfunc)+{+size_trem=s->bufsize-s->pos-sizeof(s->sm);+structseq_file*seq=&s->seq;+intret;++seq->count=0;+seq->size=min(seq->size,rem);+seq->buf=kvmalloc(seq->size,GFP_KERNEL_ACCOUNT);+if(!seq->buf)+return-ENOMEM;++ret=func(s);+if(ret)+returnret;++if(seq_has_overflowed(seq)){+if(seq->size==rem)+return-EOVERFLOW;+seq->size*=2;+if(seq->size>MAX_RW_COUNT)+return-ENOMEM;+kvfree(seq->buf);+return0;+}++/* Done */+return1;+}++staticvoidstmt_string(structstmt_state*s,u64mask,stmt_func_tfunc,+u32*str)+{+intret=s->pos+sizeof(s->sm)>=s->bufsize?-EOVERFLOW:0;+structstatmnt*sm=&s->sm;+structseq_file*seq=&s->seq;++if(s->err||!(s->mask&mask))+return;++seq->size=PAGE_SIZE;+while(!ret)+ret=stmt_string_seq(s,func);++if(ret<0){+s->err=ret;+}else{+seq->buf[seq->count++]='\0';+if(copy_to_user(s->buf->str+s->pos,seq->buf,seq->count)){+s->err=-EFAULT;+}else{+*str=s->pos;+s->pos+=seq->count;+}+}+kvfree(seq->buf);+sm->mask|=mask;+}++staticvoidstmt_numeric(structstmt_state*s,u64mask,stmt_func_tfunc)+{+if(s->err||!(s->mask&mask))+return;++s->err=func(s);+s->sm.mask|=mask;+}++staticu64mnt_to_attr_flags(structvfsmount*mnt)+{+unsignedintmnt_flags=READ_ONCE(mnt->mnt_flags);+u64attr_flags=0;++if(mnt_flags&MNT_READONLY)+attr_flags|=MOUNT_ATTR_RDONLY;+if(mnt_flags&MNT_NOSUID)+attr_flags|=MOUNT_ATTR_NOSUID;+if(mnt_flags&MNT_NODEV)+attr_flags|=MOUNT_ATTR_NODEV;+if(mnt_flags&MNT_NOEXEC)+attr_flags|=MOUNT_ATTR_NOEXEC;+if(mnt_flags&MNT_NODIRATIME)+attr_flags|=MOUNT_ATTR_NODIRATIME;+if(mnt_flags&MNT_NOSYMFOLLOW)+attr_flags|=MOUNT_ATTR_NOSYMFOLLOW;++if(mnt_flags&MNT_NOATIME)+attr_flags|=MOUNT_ATTR_NOATIME;+elseif(mnt_flags&MNT_RELATIME)+attr_flags|=MOUNT_ATTR_RELATIME;+else+attr_flags|=MOUNT_ATTR_STRICTATIME;++if(is_idmapped_mnt(mnt))+attr_flags|=MOUNT_ATTR_IDMAP;++returnattr_flags;+}++staticu64mnt_to_propagation_flags(structmount*m)+{+u64propagation=0;++if(IS_MNT_SHARED(m))+propagation|=MS_SHARED;+if(IS_MNT_SLAVE(m))+propagation|=MS_SLAVE;+if(IS_MNT_UNBINDABLE(m))+propagation|=MS_UNBINDABLE;+if(!propagation)+propagation|=MS_PRIVATE;++returnpropagation;+}++staticintstmt_sb_basic(structstmt_state*s)+{+structsuper_block*sb=s->mnt->mnt_sb;++s->sm.sb_dev_major=MAJOR(sb->s_dev);+s->sm.sb_dev_minor=MINOR(sb->s_dev);+s->sm.sb_magic=sb->s_magic;+s->sm.sb_flags=sb->s_flags&(SB_RDONLY|SB_SYNCHRONOUS|SB_DIRSYNC|SB_LAZYTIME);++return0;+}++staticintstmt_mnt_basic(structstmt_state*s)+{+structmount*m=real_mount(s->mnt);++s->sm.mnt_id=m->mnt_id_unique;+s->sm.mnt_parent_id=m->mnt_parent->mnt_id_unique;+s->sm.mnt_id_old=m->mnt_id;+s->sm.mnt_parent_id_old=m->mnt_parent->mnt_id;+s->sm.mnt_attr=mnt_to_attr_flags(&m->mnt);+s->sm.mnt_propagation=mnt_to_propagation_flags(m);+s->sm.mnt_peer_group=IS_MNT_SHARED(m)?m->mnt_group_id:0;+s->sm.mnt_master=IS_MNT_SLAVE(m)?m->mnt_master->mnt_group_id:0;++return0;+}++staticintstmt_propagate_from(structstmt_state*s)+{+structmount*m=real_mount(s->mnt);++if(!IS_MNT_SLAVE(m))+return0;++s->sm.propagate_from=get_dominating_id(m,¤t->fs->root);++return0;+}++staticintstmt_mnt_root(structstmt_state*s)+{+structseq_file*seq=&s->seq;+interr=show_path(seq,s->mnt->mnt_root);++if(!err&&!seq_has_overflowed(seq)){+seq->buf[seq->count]='\0';+seq->count=string_unescape_inplace(seq->buf,UNESCAPE_OCTAL);+}+returnerr;+}++staticintstmt_mnt_point(structstmt_state*s)+{+structvfsmount*mnt=s->mnt;+structpathmnt_path={.dentry=mnt->mnt_root,.mnt=mnt};+interr=seq_path_root(&s->seq,&mnt_path,&s->root,"");++returnerr==SEQ_SKIP?0:err;+}++staticintstmt_fs_type(structstmt_state*s)+{+structseq_file*seq=&s->seq;+structsuper_block*sb=s->mnt->mnt_sb;++seq_puts(seq,sb->s_type->name);+return0;+}++staticintdo_statmount(structstmt_state*s)+{+structstatmnt*sm=&s->sm;+structmount*m=real_mount(s->mnt);+size_tcopysize=min_t(size_t,s->bufsize,sizeof(*sm));+interr;++err=security_sb_statfs(s->mnt->mnt_root);+if(err)+returnerr;++if(!capable(CAP_SYS_ADMIN)&&+!is_path_reachable(m,m->mnt.mnt_root,&s->root))+return-EPERM;++stmt_numeric(s,STMT_SB_BASIC,stmt_sb_basic);+stmt_numeric(s,STMT_MNT_BASIC,stmt_mnt_basic);+stmt_numeric(s,STMT_PROPAGATE_FROM,stmt_propagate_from);+stmt_string(s,STMT_FS_TYPE,stmt_fs_type,&sm->fs_type);+stmt_string(s,STMT_MNT_ROOT,stmt_mnt_root,&sm->mnt_root);+stmt_string(s,STMT_MNT_POINT,stmt_mnt_point,&sm->mnt_point);++if(s->err)+returns->err;++/* Return the number of bytes copied to the buffer */+sm->size=copysize+s->pos;++if(copy_to_user(s->buf,sm,copysize))+return-EFAULT;++return0;+}++SYSCALL_DEFINE4(statmount,conststruct__mount_arg__user*,req,+structstatmnt__user*,buf,size_t,bufsize,+unsignedint,flags)+{+structvfsmount*mnt;+struct__mount_argkreq;+intret;++if(flags)+return-EINVAL;++if(copy_from_user(&kreq,req,sizeof(kreq)))+return-EFAULT;++down_read(&namespace_sem);+mnt=lookup_mnt_in_ns(kreq.mnt_id,current->nsproxy->mnt_ns);+ret=-ENOENT;+if(mnt){+structstmt_states={+.mask=kreq.request_mask,+.buf=buf,+.bufsize=bufsize,+.mnt=mnt,+};++get_fs_root(current->fs,&s.root);+ret=do_statmount(&s);+path_put(&s.root);+}+up_read(&namespace_sem);++returnret;+}+staticvoid__initinit_mount_tree(void){structvfsmount*mnt;
@@ -138,4 +138,60 @@ struct mount_attr {/* List of all mount_attr versions. */#define MOUNT_ATTR_SIZE_VER0 32 /* sizeof first published struct */++/*+*Structureforgettingmount/superblock/filesysteminfowithstatmount(2).+*+*Theinterfaceissimilartostatx(2):individualfieldsorgroupscanbe+*selectedwiththe@maskargumentofstatmount().Kernelwillsetthe@mask+*fieldaccordingtothesupportedfields.+*+*Ifstringfieldsareselected,thenthecallerneedstopassabufferthat+*hasspaceafterthefixedpartofthestructure.Nulterminatedstringsare+*copiedthereandoffsetsrelativeto@strarestoredintherelevantfields.+*Ifthebufferistoosmall,thenEOVERFLOWisreturned.Theactuallyused+*sizeisreturnedin@size.+*/+structstatmnt{+__u32size;/* Total size, including strings */+__u32__spare1;+__u64mask;/* What results were written */+__u32sb_dev_major;/* Device ID */+__u32sb_dev_minor;+__u64sb_magic;/* ..._SUPER_MAGIC */+__u32sb_flags;/* MS_{RDONLY,SYNCHRONOUS,DIRSYNC,LAZYTIME} */+__u32fs_type;/* [str] Filesystem type */+__u64mnt_id;/* Unique ID of mount */+__u64mnt_parent_id;/* Unique ID of parent (for root == mnt_id) */+__u32mnt_id_old;/* Reused IDs used in proc/.../mountinfo */+__u32mnt_parent_id_old;+__u64mnt_attr;/* MOUNT_ATTR_... */+__u64mnt_propagation;/* MS_{SHARED,SLAVE,PRIVATE,UNBINDABLE} */+__u64mnt_peer_group;/* ID of shared peer group */+__u64mnt_master;/* Mount receives propagation from this ID */+__u64propagate_from;/* Propagation from in current namespace */+__u32mnt_root;/* [str] Root of mount relative to root of fs */+__u32mnt_point;/* [str] Mountpoint relative to current root */+__u64__spare2[50];+charstr[];/* Variable size part containing strings */+};++/*+*TobeusedonthekernelABIonlyforpassing64bitargumentstostatmount(2)+*/+struct__mount_arg{+__u64mnt_id;+__u64request_mask;+};++/*+*@maskbitsforstatmount(2)+*/+#define STMT_SB_BASIC 0x00000001U /* Want/got sb_... */+#define STMT_MNT_BASIC 0x00000002U /* Want/got mnt_... */+#define STMT_PROPAGATE_FROM 0x00000004U /* Want/got propagate_from */+#define STMT_MNT_ROOT 0x00000008U /* Want/got mnt_root */+#define STMT_MNT_POINT 0x00000010U /* Want/got mnt_point */+#define STMT_FS_TYPE 0x00000020U /* Want/got fs_type */+#endif /* _UAPI_LINUX_MOUNT_H */
Add way to query the children of a particular mount. This is a more
flexible way to iterate the mount tree than having to parse the complete
/proc/self/mountinfo.
Lookup the mount by the new 64bit mount ID. If a mount needs to be queried
based on path, then statx(2) can be used to first query the mount ID
belonging to the path.
Return an array of new (64bit) mount ID's. Without privileges only mounts
are listed which are reachable from the task's root.
Signed-off-by: Miklos Szeredi <redacted>
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
fs/namespace.c | 69 ++++++++++++++++++++++++++
include/linux/syscalls.h | 3 ++
include/uapi/asm-generic/unistd.h | 5 +-
include/uapi/linux/mount.h | 3 ++
6 files changed, 81 insertions(+), 1 deletion(-)
@@ -376,6 +376,7 @@ 452 common fchmodat2 sys_fchmodat2 453 64 map_shadow_stack sys_map_shadow_stack 454 common statmount sys_statmount+455 common listmount sys_listmount # # Due to a historical design error, certain syscalls are numbered differently
From: Ian Kent <raven@themaw.net> Date: 2023-09-29 00:42:44
On 28/9/23 21:01, Miklos Szeredi wrote:
quoted hunk
Add a way to query attributes of a single mount instead of having to parse
the complete /proc/$PID/mountinfo, which might be huge.
Lookup the mount the new 64bit mount ID. If a mount needs to be queried
based on path, then statx(2) can be used to first query the mount ID
belonging to the path.
Design is based on a suggestion by Linus:
"So I'd suggest something that is very much like "statfsat()", which gets
a buffer and a length, and returns an extended "struct statfs" *AND*
just a string description at the end."
The interface closely mimics that of statx.
Handle ASCII attributes by appending after the end of the structure (as per
above suggestion). Pointers to strings are stored in u64 members to make
the structure the same regardless of pointer size. Strings are nul
terminated.
Link: https://lore.kernel.org/all/CAHk-=wh5YifP7hzKSbwJj94+DZ2czjrZsczy6GBimiogZws=rg@mail.gmail.com/
Signed-off-by: Miklos Szeredi <redacted>
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
fs/namespace.c | 283 +++++++++++++++++++++++++
fs/statfs.c | 1 +
include/linux/syscalls.h | 5 +
include/uapi/asm-generic/unistd.h | 5 +-
include/uapi/linux/mount.h | 56 +++++
7 files changed, 351 insertions(+), 1 deletion(-)
@@ -375,6 +375,7 @@ 451 common cachestat sys_cachestat 452 common fchmodat2 sys_fchmodat2 453 64 map_shadow_stack sys_map_shadow_stack+454 common statmount sys_statmount # # Due to a historical design error, certain syscalls are numbered differently
Seems like we might need to consider making (struct mnt_namespace)->list
a hashed list.
The number of mounts could be large, for example people using autofs direct
mount setups.
It's not common for people to have of the order of 8k map entries (for
which there is a trigger mount per entry plus any mounts that have been
automounted) but it does happen. A small setup would be of the order of
1k map entries plus automounted mounts so the benefit is likely still
there to some extent.
Ian
@@ -138,4 +138,60 @@ struct mount_attr {/* List of all mount_attr versions. */#define MOUNT_ATTR_SIZE_VER0 32 /* sizeof first published struct */++/*+*Structureforgettingmount/superblock/filesysteminfowithstatmount(2).+*+*Theinterfaceissimilartostatx(2):individualfieldsorgroupscanbe+*selectedwiththe@maskargumentofstatmount().Kernelwillsetthe@mask+*fieldaccordingtothesupportedfields.+*+*Ifstringfieldsareselected,thenthecallerneedstopassabufferthat+*hasspaceafterthefixedpartofthestructure.Nulterminatedstringsare+*copiedthereandoffsetsrelativeto@strarestoredintherelevantfields.+*Ifthebufferistoosmall,thenEOVERFLOWisreturned.Theactuallyused+*sizeisreturnedin@size.+*/+structstatmnt{+__u32size;/* Total size, including strings */+__u32__spare1;+__u64mask;/* What results were written */+__u32sb_dev_major;/* Device ID */+__u32sb_dev_minor;+__u64sb_magic;/* ..._SUPER_MAGIC */+__u32sb_flags;/* MS_{RDONLY,SYNCHRONOUS,DIRSYNC,LAZYTIME} */+__u32fs_type;/* [str] Filesystem type */+__u64mnt_id;/* Unique ID of mount */+__u64mnt_parent_id;/* Unique ID of parent (for root == mnt_id) */+__u32mnt_id_old;/* Reused IDs used in proc/.../mountinfo */+__u32mnt_parent_id_old;+__u64mnt_attr;/* MOUNT_ATTR_... */+__u64mnt_propagation;/* MS_{SHARED,SLAVE,PRIVATE,UNBINDABLE} */+__u64mnt_peer_group;/* ID of shared peer group */+__u64mnt_master;/* Mount receives propagation from this ID */+__u64propagate_from;/* Propagation from in current namespace */+__u32mnt_root;/* [str] Root of mount relative to root of fs */+__u32mnt_point;/* [str] Mountpoint relative to current root */+__u64__spare2[50];+charstr[];/* Variable size part containing strings */+};++/*+*TobeusedonthekernelABIonlyforpassing64bitargumentstostatmount(2)+*/+struct__mount_arg{+__u64mnt_id;+__u64request_mask;+};++/*+*@maskbitsforstatmount(2)+*/+#define STMT_SB_BASIC 0x00000001U /* Want/got sb_... */+#define STMT_MNT_BASIC 0x00000002U /* Want/got mnt_... */+#define STMT_PROPAGATE_FROM 0x00000004U /* Want/got propagate_from */+#define STMT_MNT_ROOT 0x00000008U /* Want/got mnt_root */+#define STMT_MNT_POINT 0x00000010U /* Want/got mnt_point */+#define STMT_FS_TYPE 0x00000020U /* Want/got fs_type */+#endif /* _UAPI_LINUX_MOUNT_H */
Seems like we might need to consider making (struct mnt_namespace)->list
a hashed list.
Yes, linear search needs to go. A hash table is probably the easiest solution.
But I'd also consider replacing ns->list with an rbtree. Not as
trivial as adding a system hash table and probably also slightly
slower, but it would have some advantages:
- most space efficient (no overhead of hash buckets)
- cursor can go away (f_pos can just contain last ID)
Thanks,
Miklos
Seems like we might need to consider making (struct mnt_namespace)->list
a hashed list.
Yes, linear search needs to go. A hash table is probably the easiest solution.
But I'd also consider replacing ns->list with an rbtree. Not as
trivial as adding a system hash table and probably also slightly
slower, but it would have some advantages:
- most space efficient (no overhead of hash buckets)
- cursor can go away (f_pos can just contain last ID)
I guess that would be ok.
Avoiding the cursor is a big plus.
An rbtree is used in kernfs and its readdir function is rather painful so
I wonder what the implications might be for other enumeration needs.
Ian
From: Paul Moore <paul@paul-moore.com> Date: 2023-10-04 19:26:40
On Thu, Sep 28, 2023 at 9:03 AM Miklos Szeredi [off-list ref] wrote:
Add a way to query attributes of a single mount instead of having to parse
the complete /proc/$PID/mountinfo, which might be huge.
Lookup the mount the new 64bit mount ID. If a mount needs to be queried
based on path, then statx(2) can be used to first query the mount ID
belonging to the path.
Design is based on a suggestion by Linus:
"So I'd suggest something that is very much like "statfsat()", which gets
a buffer and a length, and returns an extended "struct statfs" *AND*
just a string description at the end."
The interface closely mimics that of statx.
Handle ASCII attributes by appending after the end of the structure (as per
above suggestion). Pointers to strings are stored in u64 members to make
the structure the same regardless of pointer size. Strings are nul
terminated.
Link: https://lore.kernel.org/all/CAHk-=wh5YifP7hzKSbwJj94+DZ2czjrZsczy6GBimiogZws=rg@mail.gmail.com/
Signed-off-by: Miklos Szeredi <redacted>
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
fs/namespace.c | 283 +++++++++++++++++++++++++
fs/statfs.c | 1 +
include/linux/syscalls.h | 5 +
include/uapi/asm-generic/unistd.h | 5 +-
include/uapi/linux/mount.h | 56 +++++
7 files changed, 351 insertions(+), 1 deletion(-)
+static int do_statmount(struct stmt_state *s)
+{
+ struct statmnt *sm = &s->sm;
+ struct mount *m = real_mount(s->mnt);
+ size_t copysize = min_t(size_t, s->bufsize, sizeof(*sm));
+ int err;
+
+ err = security_sb_statfs(s->mnt->mnt_root);
+ if (err)
+ return err;
Thank you for adding the security_sb_statfs() call to this operation,
however I believe we want to place it *after* the capability check to
be consistent with other LSM calls.
+ if (!capable(CAP_SYS_ADMIN) &&
+ !is_path_reachable(m, m->mnt.mnt_root, &s->root))
+ return -EPERM;
+
+ stmt_numeric(s, STMT_SB_BASIC, stmt_sb_basic);
+ stmt_numeric(s, STMT_MNT_BASIC, stmt_mnt_basic);
+ stmt_numeric(s, STMT_PROPAGATE_FROM, stmt_propagate_from);
+ stmt_string(s, STMT_FS_TYPE, stmt_fs_type, &sm->fs_type);
+ stmt_string(s, STMT_MNT_ROOT, stmt_mnt_root, &sm->mnt_root);
+ stmt_string(s, STMT_MNT_POINT, stmt_mnt_point, &sm->mnt_point);
+
+ if (s->err)
+ return s->err;
+
+ /* Return the number of bytes copied to the buffer */
+ sm->size = copysize + s->pos;
+
+ if (copy_to_user(s->buf, sm, copysize))
+ return -EFAULT;
+
+ return 0;
+}
From: Paul Moore <paul@paul-moore.com> Date: 2023-10-04 19:37:58
On Thu, Sep 28, 2023 at 9:04 AM Miklos Szeredi [off-list ref] wrote:
Add way to query the children of a particular mount. This is a more
flexible way to iterate the mount tree than having to parse the complete
/proc/self/mountinfo.
Lookup the mount by the new 64bit mount ID. If a mount needs to be queried
based on path, then statx(2) can be used to first query the mount ID
belonging to the path.
Return an array of new (64bit) mount ID's. Without privileges only mounts
are listed which are reachable from the task's root.
Signed-off-by: Miklos Szeredi <redacted>
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
fs/namespace.c | 69 ++++++++++++++++++++++++++
include/linux/syscalls.h | 3 ++
include/uapi/asm-generic/unistd.h | 5 +-
include/uapi/linux/mount.h | 3 ++
6 files changed, 81 insertions(+), 1 deletion(-)
I believe we would want to move the security_sb_statfs() call from
above to down here; something like this I think ...
err = security_sb_statfs(r->mnt.mnt_root);
if (err)
/* if we can't access the mount, pretend it doesn't exist */
continue;
On Wed, 4 Oct 2023 at 21:38, Paul Moore [off-list ref] wrote:
On Thu, Sep 28, 2023 at 9:04 AM Miklos Szeredi [off-list ref] wrote:
quoted
Add way to query the children of a particular mount. This is a more
flexible way to iterate the mount tree than having to parse the complete
/proc/self/mountinfo.
Lookup the mount by the new 64bit mount ID. If a mount needs to be queried
based on path, then statx(2) can be used to first query the mount ID
belonging to the path.
Return an array of new (64bit) mount ID's. Without privileges only mounts
are listed which are reachable from the task's root.
Signed-off-by: Miklos Szeredi <redacted>
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
fs/namespace.c | 69 ++++++++++++++++++++++++++
include/linux/syscalls.h | 3 ++
include/uapi/asm-generic/unistd.h | 5 +-
include/uapi/linux/mount.h | 3 ++
6 files changed, 81 insertions(+), 1 deletion(-)
I believe we would want to move the security_sb_statfs() call from
above to down here; something like this I think ...
err = security_sb_statfs(r->mnt.mnt_root);
if (err)
/* if we can't access the mount, pretend it doesn't exist */
continue;
Hmm. Why is this specific to listing mounts (i.e. why doesn't readdir
have a similar filter)?
Also why hasn't this come up with regards to the proc interfaces that
list mounts?
I just want to understand the big picture here.
Thanks,
Miklos
From: Ian Kent <raven@themaw.net> Date: 2023-10-05 14:43:13
On 5/10/23 12:01, Miklos Szeredi wrote:
On Wed, 4 Oct 2023 at 21:38, Paul Moore [off-list ref] wrote:
quoted
On Thu, Sep 28, 2023 at 9:04 AM Miklos Szeredi [off-list ref] wrote:
quoted
Add way to query the children of a particular mount. This is a more
flexible way to iterate the mount tree than having to parse the complete
/proc/self/mountinfo.
Lookup the mount by the new 64bit mount ID. If a mount needs to be queried
based on path, then statx(2) can be used to first query the mount ID
belonging to the path.
Return an array of new (64bit) mount ID's. Without privileges only mounts
are listed which are reachable from the task's root.
Signed-off-by: Miklos Szeredi <redacted>
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
fs/namespace.c | 69 ++++++++++++++++++++++++++
include/linux/syscalls.h | 3 ++
include/uapi/asm-generic/unistd.h | 5 +-
include/uapi/linux/mount.h | 3 ++
6 files changed, 81 insertions(+), 1 deletion(-)
I believe we would want to move the security_sb_statfs() call from
above to down here; something like this I think ...
err = security_sb_statfs(r->mnt.mnt_root);
if (err)
/* if we can't access the mount, pretend it doesn't exist */
continue;
Hmm. Why is this specific to listing mounts (i.e. why doesn't readdir
have a similar filter)?
Also why hasn't this come up with regards to the proc interfaces that
list mounts?
The proc interfaces essentially use <mount namespace>->list to provide
the mounts that can be seen so it's filtered by mount namespace of the
task that's doing the open().
See fs/namespace.c:mnt_list_next() and just below the m_start(), m_next(),
etc.
Ian
I just want to understand the big picture here.
Thanks,
Miklos
On Thu, 28 Sept 2023 at 15:03, Miklos Szeredi [off-list ref] wrote:
If a mount is released then its mnt_id can immediately be reused. This is
bad news for user interfaces that want to uniquely identify a mount.
Implementing a unique mount ID is trivial (use a 64bit counter).
Unfortunately userspace assumes 32bit size and would overflow after the
counter reaches 2^32.
Introduce a new 64bit ID alongside the old one. Initialize the counter to
2^32, this guarantees that the old and new IDs are never mixed up.
It occurred to me that it might make sense to make this counter
per-namespace. That would allow more separation between namespaces,
like preventing the observation of mount creations in other
namespaces.
Does a global number make any sense?
Thanks,
Miklos
On Thu, 5 Oct 2023 at 06:23, Ian Kent [off-list ref] wrote:
The proc interfaces essentially use <mount namespace>->list to provide
the mounts that can be seen so it's filtered by mount namespace of the
task that's doing the open().
See fs/namespace.c:mnt_list_next() and just below the m_start(), m_next(),
/proc/$PID/mountinfo will list the mount namespace of $PID. Whether
current task has permission to do so is decided at open time.
listmount() will list the children of the given mount ID. The mount
ID is looked up in the task's mount namespace, so this cannot be used
to list mounts of other namespaces. It's a more limited interface.
I sort of understand the reasoning behind calling into a security hook
on entry to statmount() and listmount(). And BTW I also think that if
statmount() and listmount() is limited in this way, then the same
limitation should be applied to the proc interfaces. But that needs
to be done real carefully because it might cause regressions. OTOH if
it's only done on the new interfaces, then what is the point, since
the old interfaces will be available indefinitely?
Also I cannot see the point in hiding some mount ID's from the list.
It seems to me that the list is just an array of numbers that in
itself doesn't carry any information.
Thanks,
Miklos
From: Ian Kent <raven@themaw.net> Date: 2023-10-06 00:27:26
On 5/10/23 23:47, Miklos Szeredi wrote:
On Thu, 5 Oct 2023 at 06:23, Ian Kent [off-list ref] wrote:
quoted
The proc interfaces essentially use <mount namespace>->list to provide
the mounts that can be seen so it's filtered by mount namespace of the
task that's doing the open().
See fs/namespace.c:mnt_list_next() and just below the m_start(), m_next(),
/proc/$PID/mountinfo will list the mount namespace of $PID. Whether
current task has permission to do so is decided at open time.
listmount() will list the children of the given mount ID. The mount
ID is looked up in the task's mount namespace, so this cannot be used
to list mounts of other namespaces. It's a more limited interface.
Yep. But isn't the ability to see these based on task privilege?
Is the proc style restriction actually what we need here (or some variation
of that implementation)?
An privileged task typically has the init namespace as its mount namespace
and mounts should propagate from there so it should be able to see all
mounts.
If the file handle has been opened in a task that is using some other mount
namespace then presumably that's what the program author wants the task
to see.
So I'm not sure I see a problem obeying the namespace of a given task.
Ian
I sort of understand the reasoning behind calling into a security hook
on entry to statmount() and listmount(). And BTW I also think that if
statmount() and listmount() is limited in this way, then the same
limitation should be applied to the proc interfaces. But that needs
to be done real carefully because it might cause regressions. OTOH if
it's only done on the new interfaces, then what is the point, since
the old interfaces will be available indefinitely?
Also I cannot see the point in hiding some mount ID's from the list.
It seems to me that the list is just an array of numbers that in
itself doesn't carry any information.
Thanks,
Miklos
From: Paul Moore <paul@paul-moore.com> Date: 2023-10-06 02:56:26
On Thu, Oct 5, 2023 at 11:47 AM Miklos Szeredi [off-list ref] wrote:
On Thu, 5 Oct 2023 at 06:23, Ian Kent [off-list ref] wrote:
quoted
The proc interfaces essentially use <mount namespace>->list to provide
the mounts that can be seen so it's filtered by mount namespace of the
task that's doing the open().
See fs/namespace.c:mnt_list_next() and just below the m_start(), m_next(),
/proc/$PID/mountinfo will list the mount namespace of $PID. Whether
current task has permission to do so is decided at open time.
listmount() will list the children of the given mount ID. The mount
ID is looked up in the task's mount namespace, so this cannot be used
to list mounts of other namespaces. It's a more limited interface.
I sort of understand the reasoning behind calling into a security hook
on entry to statmount() and listmount(). And BTW I also think that if
statmount() and listmount() is limited in this way, then the same
limitation should be applied to the proc interfaces. But that needs
to be done real carefully because it might cause regressions. OTOH if
it's only done on the new interfaces, then what is the point, since
the old interfaces will be available indefinitely?
LSMs that are designed to enforce access controls on procfs interfaces
typically leverage the fact that the procfs interfaces are file based
and the normal file I/O access controls can be used. In some cases,
e.g. /proc/self/attr, there may also be additional access controls
implemented via a dedicated set of LSM hooks.
Also I cannot see the point in hiding some mount ID's from the list.
It seems to me that the list is just an array of numbers that in
itself doesn't carry any information.
I think it really comes down to the significance of the mount ID, and
I can't say I know enough of the details here to be entirely
comfortable taking a hard stance on this. Can you help me understand
the mount ID concept a bit better?
While I'm reasonably confident that we want a security_sb_statfs()
control point in statmount(), it may turn out that we don't want/need
a call in the listmount() case. Perhaps your original patch was
correct in the sense that we only want a single security_sb_statfs()
call for the root (implying that the child mount IDs are attributes of
the root/parent mount)? Maybe it's something else entirely?
--
paul-moore.com
On Fri, 6 Oct 2023 at 04:56, Paul Moore [off-list ref] wrote:
quoted
Also I cannot see the point in hiding some mount ID's from the list.
It seems to me that the list is just an array of numbers that in
itself doesn't carry any information.
I think it really comes down to the significance of the mount ID, and
I can't say I know enough of the details here to be entirely
comfortable taking a hard stance on this. Can you help me understand
the mount ID concept a bit better?
Mount ID is a descriptor that allows referring to a specific struct
mount from userspace.
The old 32 bit mount id is allocated with IDA from a global pool.
Because it's non-referencing it doesn't allow uniquely identifying a
mount. That was a design mistake that I made back in 2008, thinking
that the same sort of dense descriptor space as used for file
descriptors would work. Originally it was used to identify the mount
and the parent mount in /proc/PID/mountinfo. Later it was also added
to the following interfaces:
- name_to_handle_at(2) returns 32 bit value
- /proc/PID/FD/fdinfo
- statx(2) returns 64 bit value
It was never used on the kernel interfaces as an input argument.
statmount(2) and listmount(2) require the mount to be identified by
userspace, so having a unique ID is important. So the "[1/4] add
unique mount ID" adds a new 64 bit ID (still global) that is allocated
sequentially and only reused after reboot. It is used as an input to
these syscalls. It is returned by statx(2) if requested by
STATX_MNT_ID_UNIQUE and as an array of ID's by listmount(2).
I can see mild security problems with the global allocation, since a
task can observe mounts being done in other namespaces. This doesn't
sound too serious, and the old ID has similar issues. But I think
making the new ID be local to the mount namespace is also feasible.
While I'm reasonably confident that we want a security_sb_statfs()
control point in statmount(), it may turn out that we don't want/need
a call in the listmount() case. Perhaps your original patch was
correct in the sense that we only want a single security_sb_statfs()
call for the root (implying that the child mount IDs are attributes of
the root/parent mount)? Maybe it's something else entirely?
Mounts are arranged in a tree (I think it obvious how) and
listmount(2) just lists the IDs of the immediate children of a mount.
I don't see ID being an attribute of a mount, it's a descriptor.
Thanks,
Miklos
From: Amir Goldstein <amir73il@gmail.com> Date: 2023-10-06 11:44:53
On Thu, Oct 5, 2023 at 6:52 PM Miklos Szeredi [off-list ref] wrote:
On Thu, 28 Sept 2023 at 15:03, Miklos Szeredi [off-list ref] wrote:
quoted
If a mount is released then its mnt_id can immediately be reused. This is
bad news for user interfaces that want to uniquely identify a mount.
Implementing a unique mount ID is trivial (use a 64bit counter).
Unfortunately userspace assumes 32bit size and would overflow after the
counter reaches 2^32.
Introduce a new 64bit ID alongside the old one. Initialize the counter to
2^32, this guarantees that the old and new IDs are never mixed up.
It occurred to me that it might make sense to make this counter
per-namespace. That would allow more separation between namespaces,
like preventing the observation of mount creations in other
namespaces.
Preventing the observation of mount creations in other mount namespaces
is independent of whether a global mntid namespace is used.
Does a global number make any sense?
I think global mntid namepsace makes notifications API a lot easier.
A process (e.g. systemd) may set marks to watch new mounts on
different mount namespaces.
If mntid could collide in different mount namepsaces, we will either
need to describe the mount namespace in the event or worse,
map child mount namespace mntid to parent mount namespace
like with uids.
If we use a global mntid namespace, multi mount namespace
watching becomes much much easier.
Regarding the possible scopes for watching added/removed mounts
we could support:
- watch parent mount for children mounts (akin to inotify directory watch)
- watch all mounts of a filesystem
- watch all mounts in mount namespace
- watch on entire system
Not sure which of the above we will end up implementing, but the
first two can use existing fanotify mount/sb marks.
Thanks,
Amir.
On Fri, 6 Oct 2023 at 13:44, Amir Goldstein [off-list ref] wrote:
On Thu, Oct 5, 2023 at 6:52 PM Miklos Szeredi [off-list ref] wrote:
quoted
On Thu, 28 Sept 2023 at 15:03, Miklos Szeredi [off-list ref] wrote:
quoted
If a mount is released then its mnt_id can immediately be reused. This is
bad news for user interfaces that want to uniquely identify a mount.
Implementing a unique mount ID is trivial (use a 64bit counter).
Unfortunately userspace assumes 32bit size and would overflow after the
counter reaches 2^32.
Introduce a new 64bit ID alongside the old one. Initialize the counter to
2^32, this guarantees that the old and new IDs are never mixed up.
It occurred to me that it might make sense to make this counter
per-namespace. That would allow more separation between namespaces,
like preventing the observation of mount creations in other
namespaces.
Preventing the observation of mount creations in other mount namespaces
is independent of whether a global mntid namespace is used.
A local ID space makes observation of mount creations in other
namespaces impossible. While a global ID space does not. ID
allocation could be designed in a way that makes observation
impossible, but that basically boils down to allocating separate
ranges to each namespace, which is equivalent to identifying mounts by
an {NSID, MNTID} pair.
quoted
Does a global number make any sense?
I think global mntid namepsace makes notifications API a lot easier.
A process (e.g. systemd) may set marks to watch new mounts on
different mount namespaces.
If mntid could collide in different mount namepsaces, we will either
need to describe the mount namespace in the event or worse,
map child mount namespace mntid to parent mount namespace
like with uids.
Mntids are quite different from uids in that a certain ID is only
valid in a certain namespace. There's no inheritance or sharing of
mounts. Also mounts cannot be moved from one namespace to another
(with the exception of mounts created in an anonymous namespace).
If we use a global mntid namespace, multi mount namespace
watching becomes much much easier.
If the watcher wants to interpret the received event it will have to
know the namespace anyway. Otherwise it's just a meaningless number.
Regarding the possible scopes for watching added/removed mounts
we could support:
- watch parent mount for children mounts (akin to inotify directory watch)
This probably makes sense.
- watch all mounts of a filesystem
I don't see a use case.
- watch all mounts in mount namespace
Yes.
- watch on entire system
Again, I don't see how this could make sense. Each time
unshare(CLONE_NEWNS) is invoked a storm of mount creation events will
happen.
I'd imagine that watching mounts is primarily to complement the
directory tree notifications, so that all changes to the path lookup
namespace of a process could be monitored.
Thanks,
Miklos
From: Paul Moore <paul@paul-moore.com> Date: 2023-10-06 23:08:23
On Fri, Oct 6, 2023 at 4:53 AM Miklos Szeredi [off-list ref] wrote:
On Fri, 6 Oct 2023 at 04:56, Paul Moore [off-list ref] wrote:
quoted
quoted
Also I cannot see the point in hiding some mount ID's from the list.
It seems to me that the list is just an array of numbers that in
itself doesn't carry any information.
I think it really comes down to the significance of the mount ID, and
I can't say I know enough of the details here to be entirely
comfortable taking a hard stance on this. Can you help me understand
the mount ID concept a bit better?
Mount ID is a descriptor that allows referring to a specific struct
mount from userspace.
The old 32 bit mount id is allocated with IDA from a global pool.
Because it's non-referencing it doesn't allow uniquely identifying a
mount. That was a design mistake that I made back in 2008, thinking
that the same sort of dense descriptor space as used for file
descriptors would work. Originally it was used to identify the mount
and the parent mount in /proc/PID/mountinfo. Later it was also added
to the following interfaces:
- name_to_handle_at(2) returns 32 bit value
- /proc/PID/FD/fdinfo
- statx(2) returns 64 bit value
It was never used on the kernel interfaces as an input argument.
Thanks for the background.
statmount(2) and listmount(2) require the mount to be identified by
userspace, so having a unique ID is important. So the "[1/4] add
unique mount ID" adds a new 64 bit ID (still global) that is allocated
sequentially and only reused after reboot. It is used as an input to
these syscalls. It is returned by statx(2) if requested by
STATX_MNT_ID_UNIQUE and as an array of ID's by listmount(2).
I can see mild security problems with the global allocation, since a
task can observe mounts being done in other namespaces. This doesn't
sound too serious, and the old ID has similar issues. But I think
making the new ID be local to the mount namespace is also feasible.
The LSM hook API is designed to operate independently from any of the
kernel namespaces; while some LSMs may choose to be aware of
namespaces and adjust their controls accordingly, there is no
requirement that they do so. For that reason, I'm not too bothered
either way if the mount ID is global or tied to a namespace.
quoted
While I'm reasonably confident that we want a security_sb_statfs()
control point in statmount(), it may turn out that we don't want/need
a call in the listmount() case. Perhaps your original patch was
correct in the sense that we only want a single security_sb_statfs()
call for the root (implying that the child mount IDs are attributes of
the root/parent mount)? Maybe it's something else entirely?
Mounts are arranged in a tree (I think it obvious how) and
listmount(2) just lists the IDs of the immediate children of a mount.
I don't see ID being an attribute of a mount, it's a descriptor.
In this case I think the approach you took originally in this thread
is likely what we want, call security_sb_statfs() against the root
mount in listmount(). Just please move it after the capability
checks.
If you look at the two LSMs which implement the security_sb_statfs(),
Smack and SELinux, you see that Smack treats this as a read operation
between the current process and the specified mount and SELinux treats
this as a filesystem:getattr operations between the current process
and the specified mount. In both cases I can see that being the right
approach for reading a list of child mounts off of a root mount.
Does that sound good? I'm guessing that's okay since that was how you
wrote it in your original patch, but there has been a lot of
discussion since then :)
--
paul-moore.com
From: Ian Kent <raven@themaw.net> Date: 2023-10-13 02:40:05
On 6/10/23 08:27, Ian Kent wrote:
On 5/10/23 23:47, Miklos Szeredi wrote:
quoted
On Thu, 5 Oct 2023 at 06:23, Ian Kent [off-list ref] wrote:
quoted
The proc interfaces essentially use <mount namespace>->list to provide
the mounts that can be seen so it's filtered by mount namespace of the
task that's doing the open().
See fs/namespace.c:mnt_list_next() and just below the m_start(),
m_next(),
/proc/$PID/mountinfo will list the mount namespace of $PID. Whether
current task has permission to do so is decided at open time.
listmount() will list the children of the given mount ID. The mount
ID is looked up in the task's mount namespace, so this cannot be used
to list mounts of other namespaces. It's a more limited interface.
Yep. But isn't the ability to see these based on task privilege?
Is the proc style restriction actually what we need here (or some
variation
of that implementation)?
An privileged task typically has the init namespace as its mount
namespace
and mounts should propagate from there so it should be able to see all
mounts.
If the file handle has been opened in a task that is using some other
mount
namespace then presumably that's what the program author wants the
task to see.
So I'm not sure I see a problem obeying the namespace of a given task.
I've had a look through the code we had in the old fsinfo() proposal
because I think we need to consider the use cases that are needed.
IIRC initially we had a flag FSINFO_ATTR_MOUNT_CHILDREN that essentially
enumerated the children of the given mount in much the same way as is
done now in this system call.
But because we needed to enumerate mounts in the same way as the proc file
system mount tables a flag FSINFO_ATTR_MOUNT_ALL was added that essentially
used the mount namespace mounts list in a similar way to the proc file
system so that a list of mounts for a mount namespace could be retrieved.
This later use case is what is used by processes that monitor mounts and
is what's needed more so than enumerating the children as we do now.
I'm still looking at the mount id lookup.
Ian
Ian
quoted
I sort of understand the reasoning behind calling into a security hook
on entry to statmount() and listmount(). And BTW I also think that if
statmount() and listmount() is limited in this way, then the same
limitation should be applied to the proc interfaces. But that needs
to be done real carefully because it might cause regressions. OTOH if
it's only done on the new interfaces, then what is the point, since
the old interfaces will be available indefinitely?
Also I cannot see the point in hiding some mount ID's from the list.
It seems to me that the list is just an array of numbers that in
itself doesn't carry any information.
Thanks,
Miklos
On Fri, 13 Oct 2023 at 04:40, Ian Kent [off-list ref] wrote:
But because we needed to enumerate mounts in the same way as the proc file
system mount tables a flag FSINFO_ATTR_MOUNT_ALL was added that essentially
used the mount namespace mounts list in a similar way to the proc file
system so that a list of mounts for a mount namespace could be retrieved.
Makes sense.
Added a LISTMOUNT_RECURSIVE flag.
Thanks,
Miklos