# \[PATCH RFC v3 1/2\] nsfs: replace proc\_ns\_fget() with proc\_ns\_fdget() - Subsystems: FILESYSTEMS (VFS and infrastructure), NETWORKING \[GENERAL\], THE REST - Maintainers: Alexander Viro, Christian Brauner, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds - List: linux-api - 6 messages, 3 authors - Page: https://ratatoskr.run/linux-api/2015/09/4583645/t ## \[PATCH RFC v3 1/2\] nsfs: replace proc\_ns\_fget() with proc\_ns\_fdget() - From: Konstantin Khlebnikov \ - Date: 2015-09-25 13:52:56 - Message: https://ratatoskr.run/linux-api/2015/09/4583645.md ```text New helper function returns pointer to struct ns_common, can check namespace type and uses struct fd for returning file reference: this saves couple atomic operations for single-threaded applications. Signed-off-by: Konstantin Khlebnikov --- fs/nsfs.c | 20 +++++++++++++------- include/linux/proc_ns.h | 4 +++- kernel/nsproxy.c | 15 +++++---------- net/core/net_namespace.c | 18 ++++++------------ 4 files changed, 27 insertions(+), 30 deletions(-) diff --git a/fs/nsfs.c b/fs/nsfs.c index 8f20d6016e20..a2e803c97960 100644 --- a/fs/nsfs.c +++ b/fs/nsfs.c @@ -119,21 +119,27 @@ int ns_get_name(char *buf, size_t size, struct task_struct *task, return res; } -struct file *proc_ns_fget(int fd) +struct ns_common *proc_ns_fdget(int fd, int nstype, struct fd *fd_ref) { - struct file *file; + struct ns_common *ns; + struct fd f; - file = fget(fd); - if (!file) + f = fdget(fd); + if (!f.file) return ERR_PTR(-EBADF); - if (file->f_op != &ns_file_operations) + if (f.file->f_op != &ns_file_operations) + goto out_invalid; + + ns = get_proc_ns(file_inode(f.file)); + if (nstype && (ns->ops->type != nstype)) goto out_invalid; - return file; + *fd_ref = f; + return ns; out_invalid: - fput(file); + fdput(f); return ERR_PTR(-EINVAL); } diff --git a/include/linux/proc_ns.h b/include/linux/proc_ns.h index 42dfc615dbf8..406ea4617606 100644 --- a/include/linux/proc_ns.h +++ b/include/linux/proc_ns.h @@ -9,6 +9,7 @@ struct pid_namespace; struct nsproxy; struct path; +struct fd; struct proc_ns_operations { const char *name; @@ -65,7 +66,8 @@ static inline int ns_alloc_inum(struct ns_common *ns) #define ns_free_inum(ns) proc_free_inum((ns)->inum) -extern struct file *proc_ns_fget(int fd); +extern struct ns_common *proc_ns_fdget(int fd, int nstype, struct fd *fd_ref); + #define get_proc_ns(inode) ((struct ns_common *)(inode)->i_private) extern void *ns_get_path(struct path *path, struct task_struct *task, const struct proc_ns_operations *ns_ops); diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c index 49746c81ad8d..689b929a0fcb 100644 --- a/kernel/nsproxy.c +++ b/kernel/nsproxy.c @@ -222,18 +222,13 @@ SYSCALL_DEFINE2(setns, int, fd, int, nstype) { struct task_struct *tsk = current; struct nsproxy *new_nsproxy; - struct file *file; + struct fd fd_ref; struct ns_common *ns; int err; - file = proc_ns_fget(fd); - if (IS_ERR(file)) - return PTR_ERR(file); - - err = -EINVAL; - ns = get_proc_ns(file_inode(file)); - if (nstype && (ns->ops->type != nstype)) - goto out; + ns = proc_ns_fdget(fd, nstype, &fd_ref); + if (IS_ERR(ns)) + return PTR_ERR(ns); new_nsproxy = create_new_namespaces(0, tsk, current_user_ns(), tsk->fs); if (IS_ERR(new_nsproxy)) { @@ -248,7 +243,7 @@ SYSCALL_DEFINE2(setns, int, fd, int, nstype) } switch_task_namespaces(tsk, new_nsproxy); out: - fput(file); + fdput(fd_ref); return err; } diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index 2c2eb1b629b1..ffc5e6df86dc 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c @@ -465,21 +465,15 @@ EXPORT_SYMBOL_GPL(__put_net); struct net *get_net_ns_by_fd(int fd) { - struct file *file; struct ns_common *ns; + struct fd fd_ref; struct net *net; - file = proc_ns_fget(fd); - if (IS_ERR(file)) - return ERR_CAST(file); - - ns = get_proc_ns(file_inode(file)); - if (ns->ops == &netns_operations) - net = get_net(container_of(ns, struct net, ns)); - else - net = ERR_PTR(-EINVAL); - - fput(file); + ns = proc_ns_fdget(fd, CLONE_NEWNET, &fd_ref); + if (IS_ERR(ns)) + return ERR_CAST(ns); + net = get_net(container_of(ns, struct net, ns)); + fdput(fd_ref); return net; } ``` ## \[PATCH RFC v3 2/2\] pidns: introduce syscall getvpid - From: Konstantin Khlebnikov \ - Date: 2015-09-25 13:52:59 - Message: https://ratatoskr.run/linux-api/2015/09/4583646.md ```text pid_t getvpid(pid_t pid, int source, int target); This syscall converts pid from source pid-namespace into pid visible in target pid-namespace. If pid is unreachable from target namespace then getvpid() returns zero. Namespaces are defined by file descriptors pointing to entries in proc (/proc/[pid]/ns/pid). If argument is negative then current pid namespace is used. If pid is negative then getvpid() returns pid of parent task for -pid. Possible error codes: ESRCH - task not found EBADF - closed file descriptor EINVAL - not pid-namespace file descriptor Such conversion is required for interaction between processes from different pid-namespaces. For example system service at host system who provide access to restricted set of privileged operations for clients from containers have to convert pids back and forward. Recent kernels expose virtual pids in /proc/[pid]/status:NSpid, but this interface works only in one way and even that is non-trivial. Other option is passing pids with credentials via unix socket, but this solution requires a lot of preparation and CAP_SYS_ADMIN for sending arbitrary pids. This syscall works in both directions, it's fast and simple. Examples: getvpid(pid, ns, -1) - get pid in our pid namespace getvpid(pid, -1, ns) - get pid in container getvpid(pid, -1, ns) > 0 - is pid is reachable from container? getvpid(1, ns1, ns2) > 0 - is ns1 inside ns2? getvpid(1, ns1, ns2) == 0 - is ns1 outside ns2? getvpid(1, ns, -1) - get init task of pid-namespace getvpid(-1, ns, -1) - get reaper of init task in parent pid-namespace getvpid(-pid, -1, -1) - get ppid by pid Signed-off-by: Konstantin Khlebnikov -- v1: https://lkml.org/lkml/2015/9/15/411 v2: https://lkml.org/lkml/2015/9/24/278 v3: * use proc_ns_fdget() * update description * rebase to next-20150925 * fix conflict with mlock2 --- arch/x86/entry/syscalls/syscall_32.tbl | 1 + arch/x86/entry/syscalls/syscall_64.tbl | 1 + include/linux/syscalls.h | 1 + include/uapi/asm-generic/unistd.h | 4 ++- kernel/sys.c | 51 ++++++++++++++++++++++++++++++++ 5 files changed, 57 insertions(+), 1 deletion(-) diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl index 143ef9f37932..c36c2c65d204 100644 --- a/arch/x86/entry/syscalls/syscall_32.tbl +++ b/arch/x86/entry/syscalls/syscall_32.tbl @@ -383,3 +383,4 @@ 374 i386 userfaultfd sys_userfaultfd 375 i386 membarrier sys_membarrier 376 i386 mlock2 sys_mlock2 +377 i386 getvpid sys_getvpid diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl index 314a90bfc09c..90bbbc7fdbe0 100644 --- a/arch/x86/entry/syscalls/syscall_64.tbl +++ b/arch/x86/entry/syscalls/syscall_64.tbl @@ -332,6 +332,7 @@ 323 common userfaultfd sys_userfaultfd 324 common membarrier sys_membarrier 325 common mlock2 sys_mlock2 +326 common getvpid sys_getvpid # # x32-specific system call numbers start at 512 to avoid cache impact diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h index a156b82dd14c..dbb5638260b5 100644 --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -222,6 +222,7 @@ asmlinkage long sys_nanosleep(struct timespec __user *rqtp, struct timespec __us asmlinkage long sys_alarm(unsigned int seconds); asmlinkage long sys_getpid(void); asmlinkage long sys_getppid(void); +asmlinkage long sys_getvpid(pid_t pid, int source, int target); asmlinkage long sys_getuid(void); asmlinkage long sys_geteuid(void); asmlinkage long sys_getgid(void); diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h index 1324b0292ec2..2c1123130f90 100644 --- a/include/uapi/asm-generic/unistd.h +++ b/include/uapi/asm-generic/unistd.h @@ -715,9 +715,11 @@ __SYSCALL(__NR_userfaultfd, sys_userfaultfd) __SYSCALL(__NR_membarrier, sys_membarrier) #define __NR_mlock2 284 __SYSCALL(__NR_mlock2, sys_mlock2) +#define __NR_mlock2 285 +__SYSCALL(__NR_getvpid, sys_getvpid) #undef __NR_syscalls -#define __NR_syscalls 285 +#define __NR_syscalls 286 /* * All syscalls below here should go away really, diff --git a/kernel/sys.c b/kernel/sys.c index fa2f2f671a5c..1e28a36b84fa 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -46,6 +46,7 @@ #include #include #include +#include #include #include @@ -855,6 +856,56 @@ SYSCALL_DEFINE0(getppid) return pid; } +SYSCALL_DEFINE3(getvpid, pid_t, pid, int, source, int, target) +{ + struct pid_namespace *source_ns, *target_ns; + struct fd source_fd = {}, target_fd = {}; + struct pid *struct_pid; + struct ns_common *ns; + pid_t result; + + if (source >= 0) { + ns = proc_ns_fdget(source, CLONE_NEWPID, &source_fd); + result = PTR_ERR(ns); + if (IS_ERR(ns)) + goto out; + source_ns = container_of(ns, struct pid_namespace, ns); + } else + source_ns = task_active_pid_ns(current); + + if (target >= 0) { + ns = proc_ns_fdget(target, CLONE_NEWPID, &target_fd); + result = PTR_ERR(ns); + if (IS_ERR(ns)) + goto out; + target_ns = container_of(ns, struct pid_namespace, ns); + } else + target_ns = task_active_pid_ns(current); + + rcu_read_lock(); + struct_pid = find_pid_ns(abs(pid), source_ns); + + if (struct_pid && pid < 0) { + struct task_struct *task; + + task = pid_task(struct_pid, PIDTYPE_PID); + if (task) + task = rcu_dereference(task->real_parent); + struct_pid = task ? task_pid(task) : NULL; + } + + if (struct_pid) + result = pid_nr_ns(struct_pid, target_ns); + else + result = -ESRCH; + rcu_read_unlock(); + +out: + fdput(target_fd); + fdput(source_fd); + return result; +} + SYSCALL_DEFINE0(getuid) { /* Only we change this so SMP safe */ ``` ## \[PATCH 0/1\] ns: introduce proc\_get\_ns\_by\_fd() - From: Oleg Nesterov \ - Date: 2015-09-25 18:00:05 - Message: https://ratatoskr.run/linux-api/2015/09/4583792.md ```text On 09/25, Konstantin Khlebnikov wrote: > > +struct ns_common *proc_ns_fdget(int fd, int nstype, struct fd *fd_ref) > { > - struct file *file; > + struct ns_common *ns; > + struct fd f; > > - file = fget(fd); > - if (!file) > + f = fdget(fd); > + if (!f.file) > return ERR_PTR(-EBADF); > > - if (file->f_op != &ns_file_operations) > + if (f.file->f_op != &ns_file_operations) > + goto out_invalid; > + > + ns = get_proc_ns(file_inode(f.file)); > + if (nstype && (ns->ops->type != nstype)) > goto out_invalid; > > - return file; > + *fd_ref = f; > + return ns; > > out_invalid: > - fput(file); > + fdput(f); > return ERR_PTR(-EINVAL); > } Well yes, fdget() makes sense but this is minor. Honestly, I do not really like the new helper... I understand this is subjective, so I won't insist. But how about 1/1? We do not need fd/file at all. With this patch your sys_getvpid() can just use proc_get_ns_by_fd(fd, CLONE_NEWPID) and put_pid_ns(). Eric, what do you think? See also "TODO" in the changelog. Oleg. ``` ## \[PATCH 1/1\] ns: introduce proc\_get\_ns\_by\_fd() - From: Oleg Nesterov \ - Date: 2015-09-25 18:00:23 - Message: https://ratatoskr.run/linux-api/2015/09/4583801.md ```text Introduce proc_get_ns_by_fd() so that get_net_ns_by_fd() becomes one-liner. It will have another CLONE_NEWPID user soon. TODO: proc_get_ns_by_fd() can share some code with proc_ns_fget(). Signed-off-by: Oleg Nesterov --- fs/nsfs.c | 24 ++++++++++++++++++++++++ include/linux/proc_ns.h | 3 +++ kernel/pid_namespace.c | 6 ++++++ net/core/net_namespace.c | 23 +++++++---------------- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/fs/nsfs.c b/fs/nsfs.c index 99521e7..72474ca 100644 --- a/fs/nsfs.c +++ b/fs/nsfs.c @@ -136,6 +136,30 @@ out_invalid: return ERR_PTR(-EINVAL); } +void *proc_get_ns_by_fd(int fd, int type) +{ + struct fd f; + struct ns_common *ns; + void *ret; + + f = fdget(fd); + if (!f.file) + return ERR_PTR(-EBADF); + + ret = ERR_PTR(-EINVAL); + if (f.file->f_op != &ns_file_operations) + goto put; + + ns = get_proc_ns(file_inode(f.file)); + if (ns->ops->type != type || !ns->ops->get_type) + goto put; + + ret = ns->ops->get_type(ns); +put: + fdput(f); + return ret; +} + static const struct super_operations nsfs_ops = { .statfs = simple_statfs, .evict_inode = nsfs_evict, diff --git a/include/linux/proc_ns.h b/include/linux/proc_ns.h index 42dfc61..d956f89 100644 --- a/include/linux/proc_ns.h +++ b/include/linux/proc_ns.h @@ -16,6 +16,7 @@ struct proc_ns_operations { struct ns_common *(*get)(struct task_struct *task); void (*put)(struct ns_common *ns); int (*install)(struct nsproxy *nsproxy, struct ns_common *ns); + void *(*get_type)(struct ns_common *ns); }; extern const struct proc_ns_operations netns_operations; @@ -66,6 +67,8 @@ static inline int ns_alloc_inum(struct ns_common *ns) #define ns_free_inum(ns) proc_free_inum((ns)->inum) extern struct file *proc_ns_fget(int fd); +extern void *proc_get_ns_by_fd(int fd, int type); + #define get_proc_ns(inode) ((struct ns_common *)(inode)->i_private) extern void *ns_get_path(struct path *path, struct task_struct *task, const struct proc_ns_operations *ns_ops); diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c index a65ba13..0c87393 100644 --- a/kernel/pid_namespace.c +++ b/kernel/pid_namespace.c @@ -388,12 +388,18 @@ static int pidns_install(struct nsproxy *nsproxy, struct ns_common *ns) return 0; } +static void *pidns_get_type(struct ns_common *ns) +{ + return get_pid_ns(to_pid_ns(ns)); +} + const struct proc_ns_operations pidns_operations = { .name = "pid", .type = CLONE_NEWPID, .get = pidns_get, .put = pidns_put, .install = pidns_install, + .get_type = pidns_get_type, }; static __init int pid_namespaces_init(void) diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index 572af00..6465dc0 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c @@ -420,22 +420,7 @@ EXPORT_SYMBOL_GPL(__put_net); struct net *get_net_ns_by_fd(int fd) { - struct file *file; - struct ns_common *ns; - struct net *net; - - file = proc_ns_fget(fd); - if (IS_ERR(file)) - return ERR_CAST(file); - - ns = get_proc_ns(file_inode(file)); - if (ns->ops == &netns_operations) - net = get_net(container_of(ns, struct net, ns)); - else - net = ERR_PTR(-EINVAL); - - fput(file); - return net; + return proc_get_ns_by_fd(fd, CLONE_NEWNET); } #else @@ -955,11 +940,17 @@ static int netns_install(struct nsproxy *nsproxy, struct ns_common *ns) return 0; } +static void *netns_get_type(struct ns_common *ns) +{ + return get_net(to_net_ns(ns)); +} + const struct proc_ns_operations netns_operations = { .name = "net", .type = CLONE_NEWNET, .get = netns_get, .put = netns_put, .install = netns_install, + .get_type = netns_get_type, }; #endif -- 1.5.5.1 ``` ## Re: \[PATCH RFC v3 2/2\] pidns: introduce syscall getvpid - From: kbuild test robot \ - Date: 2015-09-28 04:13:38 - Message: https://ratatoskr.run/linux-api/2015/09/4584595.md ```text Hi Konstantin, [auto build test results on next-20150925 -- if it's inappropriate base, please ignore] config: openrisc-or1ksim_defconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 7bea85b3c058d69527d79b94338025f218a0655e # save the attached .config to linux build tree make.cross ARCH=openrisc All warnings (new ones prefixed by >>): In file included from include/asm-generic/unistd.h:1:0, from arch/openrisc/include/uapi/asm/unistd.h:26, from :2: >> include/uapi/asm-generic/unistd.h:718:0: warning: "__NR_mlock2" redefined include/uapi/asm-generic/unistd.h:716:0: note: this is the location of the previous definition :1307:2: warning: #warning syscall getvpid not implemented -- In file included from include/asm-generic/unistd.h:1:0, from arch/openrisc/include/uapi/asm/unistd.h:26, from include/uapi/linux/unistd.h:7, from arch/openrisc/kernel/setup.c:25: >> include/uapi/asm-generic/unistd.h:718:0: warning: "__NR_mlock2" redefined include/uapi/asm-generic/unistd.h:716:0: note: this is the location of the previous definition -- In file included from include/asm-generic/unistd.h:1:0, from arch/openrisc/include/uapi/asm/unistd.h:26, from include/uapi/linux/unistd.h:7, from include/linux/syscalls.h:78, from arch/openrisc/kernel/sys_call_table.c:17: >> include/uapi/asm-generic/unistd.h:718:0: warning: "__NR_mlock2" redefined include/uapi/asm-generic/unistd.h:716:0: note: this is the location of the previous definition In file included from include/asm-generic/unistd.h:1:0, from arch/openrisc/include/uapi/asm/unistd.h:26, from arch/openrisc/kernel/sys_call_table.c:27: include/uapi/asm-generic/unistd.h:716:0: warning: "__NR_mlock2" redefined include/uapi/asm-generic/unistd.h:718:0: note: this is the location of the previous definition >> include/uapi/asm-generic/unistd.h:718:0: warning: "__NR_mlock2" redefined include/uapi/asm-generic/unistd.h:716:0: note: this is the location of the previous definition include/uapi/asm-generic/unistd.h:719:1: error: '__NR_getvpid' undeclared here (not in a function) include/uapi/asm-generic/unistd.h:719:1: error: array index in initializer not of integer type include/uapi/asm-generic/unistd.h:719:1: error: (near initialization for 'sys_call_table') -- kernel/time/Kconfig:155:warning: range is invalid In file included from include/asm-generic/unistd.h:1:0, from arch/openrisc/include/uapi/asm/unistd.h:26, from :2: >> include/uapi/asm-generic/unistd.h:718:0: warning: "__NR_mlock2" redefined include/uapi/asm-generic/unistd.h:716:0: note: this is the location of the previous definition :1307:2: warning: #warning syscall getvpid not implemented vim +/__NR_mlock2 +718 include/uapi/asm-generic/unistd.h 702 #define __NR_seccomp 277 703 __SYSCALL(__NR_seccomp, sys_seccomp) 704 #define __NR_getrandom 278 705 __SYSCALL(__NR_getrandom, sys_getrandom) 706 #define __NR_memfd_create 279 707 __SYSCALL(__NR_memfd_create, sys_memfd_create) 708 #define __NR_bpf 280 709 __SYSCALL(__NR_bpf, sys_bpf) 710 #define __NR_execveat 281 711 __SC_COMP(__NR_execveat, sys_execveat, compat_sys_execveat) 712 #define __NR_userfaultfd 282 713 __SYSCALL(__NR_userfaultfd, sys_userfaultfd) 714 #define __NR_membarrier 283 715 __SYSCALL(__NR_membarrier, sys_membarrier) 716 #define __NR_mlock2 284 717 __SYSCALL(__NR_mlock2, sys_mlock2) > 718 #define __NR_mlock2 285 719 __SYSCALL(__NR_getvpid, sys_getvpid) 720 721 #undef __NR_syscalls 722 #define __NR_syscalls 286 723 724 /* 725 * All syscalls below here should go away really, 726 * these are provided for both review and as a porting --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ``` ## Re: \[PATCH 0/1\] ns: introduce proc\_get\_ns\_by\_fd() - From: Konstantin Khlebnikov \ - Date: 2015-09-28 08:21:15 - Message: https://ratatoskr.run/linux-api/2015/09/4584708.md ```text On 25.09.2015 20:56, Oleg Nesterov wrote: > On 09/25, Konstantin Khlebnikov wrote: >> >> +struct ns_common *proc_ns_fdget(int fd, int nstype, struct fd *fd_ref) >> { >> - struct file *file; >> + struct ns_common *ns; >> + struct fd f; >> >> - file = fget(fd); >> - if (!file) >> + f = fdget(fd); >> + if (!f.file) >> return ERR_PTR(-EBADF); >> >> - if (file->f_op != &ns_file_operations) >> + if (f.file->f_op != &ns_file_operations) >> + goto out_invalid; >> + >> + ns = get_proc_ns(file_inode(f.file)); >> + if (nstype && (ns->ops->type != nstype)) >> goto out_invalid; >> >> - return file; >> + *fd_ref = f; >> + return ns; >> >> out_invalid: >> - fput(file); >> + fdput(f); >> return ERR_PTR(-EINVAL); >> } > > Well yes, fdget() makes sense but this is minor. > > Honestly, I do not really like the new helper... I understand this > is subjective, so I won't insist. But how about 1/1? We do not need > fd/file at all. With this patch your sys_getvpid() can just use > proc_get_ns_by_fd(fd, CLONE_NEWPID) and put_pid_ns(). Hmm. My version has 0 or 2 atomic ops per get-put sequence. Your version: 2 or 4 atomic ops. Plus even in worst case pinning by struct fd theoretically scales better because it touches refcount at struct file: there're might be many of them for one namespace. > > Eric, what do you think? > > See also "TODO" in the changelog. > > Oleg. > -- Konstantin ```