* [PATCH v7 02/11] proc: add proc_fs_info struct to store proc information
From: Alexey Gladkov @ 2020-01-25 13:05 UTC (permalink / raw)
To: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module
Cc: Akinobu Mita, Alexander Viro, Alexey Dobriyan, Alexey Gladkov,
Andrew Morton, Andy Lutomirski, Daniel Micay, Djalal Harouni,
Dmitry V . Levin, Eric W . Biederman, Greg Kroah-Hartman,
Ingo Molnar, J . Bruce Fields, Jeff Layton, Jonathan Corbet,
Kees Cook, Linus Torvalds, Oleg Nesterov, Solar Designer
In-Reply-To: <20200125130541.450409-1-gladkov.alexey@gmail.com>
This is a preparation patch that adds proc_fs_info to be able to store
different procfs options and informations. Right now some mount options
are stored inside the pid namespace which makes it hard to change or
modernize procfs without affecting pid namespaces. Plus we do want to
treat proc as more of a real mount point and filesystem. procfs is part
of Linux API where it offers some features using filesystem syscalls and
in order to support some features where we are able to have multiple
instances of procfs, each one with its mount options inside the same pid
namespace, we have to separate these procfs instances.
This is the same feature that was also added to other Linux interfaces
like devpts in order to support containers, sandboxes, and to have
multiple instances of devpts filesystem [1].
[1] https://elixir.bootlin.com/linux/v3.4/source/Documentation/filesystems/devpts.txt
Cc: Kees Cook <keescook@chromium.org>
Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
fs/locks.c | 6 +++--
fs/proc/base.c | 8 +++++--
fs/proc/inode.c | 4 ++--
fs/proc/root.c | 49 +++++++++++++++++++++++++++--------------
include/linux/proc_fs.h | 11 ++++++++-
5 files changed, 54 insertions(+), 24 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c
index 6970f55daf54..21200e3005e4 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2795,7 +2795,8 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
{
struct inode *inode = NULL;
unsigned int fl_pid;
- struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb_info(file_inode(f->file)->i_sb);
+ struct pid_namespace *proc_pidns = fs_info->pid_ns;
fl_pid = locks_translate_pid(fl, proc_pidns);
/*
@@ -2873,7 +2874,8 @@ static int locks_show(struct seq_file *f, void *v)
{
struct locks_iterator *iter = f->private;
struct file_lock *fl, *bfl;
- struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb_info(file_inode(f->file)->i_sb);
+ struct pid_namespace *proc_pidns = fs_info->pid_ns;
fl = hlist_entry(v, struct file_lock, fl_link);
diff --git a/fs/proc/base.c b/fs/proc/base.c
index ebea9501afb8..672e71c52dbd 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3243,6 +3243,7 @@ struct dentry *proc_pid_lookup(struct dentry *dentry, unsigned int flags)
{
struct task_struct *task;
unsigned tgid;
+ struct proc_fs_info *fs_info;
struct pid_namespace *ns;
struct dentry *result = ERR_PTR(-ENOENT);
@@ -3250,7 +3251,8 @@ struct dentry *proc_pid_lookup(struct dentry *dentry, unsigned int flags)
if (tgid == ~0U)
goto out;
- ns = dentry->d_sb->s_fs_info;
+ fs_info = proc_sb_info(dentry->d_sb);
+ ns = fs_info->pid_ns;
rcu_read_lock();
task = find_task_by_pid_ns(tgid, ns);
if (task)
@@ -3538,6 +3540,7 @@ static struct dentry *proc_task_lookup(struct inode *dir, struct dentry * dentry
struct task_struct *task;
struct task_struct *leader = get_proc_task(dir);
unsigned tid;
+ struct proc_fs_info *fs_info;
struct pid_namespace *ns;
struct dentry *result = ERR_PTR(-ENOENT);
@@ -3548,7 +3551,8 @@ static struct dentry *proc_task_lookup(struct inode *dir, struct dentry * dentry
if (tid == ~0U)
goto out;
- ns = dentry->d_sb->s_fs_info;
+ fs_info = proc_sb_info(dentry->d_sb);
+ ns = fs_info->pid_ns;
rcu_read_lock();
task = find_task_by_pid_ns(tid, ns);
if (task)
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index dbe43a50caf2..b631608dfbcc 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -104,8 +104,8 @@ void __init proc_init_kmemcache(void)
static int proc_show_options(struct seq_file *seq, struct dentry *root)
{
- struct super_block *sb = root->d_sb;
- struct pid_namespace *pid = sb->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb_info(root->d_sb);
+ struct pid_namespace *pid = fs_info->pid_ns;
if (!gid_eq(pid->pid_gid, GLOBAL_ROOT_GID))
seq_printf(seq, ",gid=%u", from_kgid_munged(&init_user_ns, pid->pid_gid));
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 0b7c8dffc9ae..d449f095f0f7 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -30,7 +30,7 @@
#include "internal.h"
struct proc_fs_context {
- struct pid_namespace *pid_ns;
+ struct proc_fs_info *fs_info;
unsigned int mask;
int hidepid;
int gid;
@@ -97,7 +97,8 @@ static void proc_apply_options(struct super_block *s,
static int proc_fill_super(struct super_block *s, struct fs_context *fc)
{
- struct pid_namespace *pid_ns = get_pid_ns(s->s_fs_info);
+ struct proc_fs_context *ctx = fc->fs_private;
+ struct pid_namespace *pid_ns = get_pid_ns(ctx->fs_info->pid_ns);
struct inode *root_inode;
int ret;
@@ -145,7 +146,8 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
static int proc_reconfigure(struct fs_context *fc)
{
struct super_block *sb = fc->root->d_sb;
- struct pid_namespace *pid = sb->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb_info(sb);
+ struct pid_namespace *pid = fs_info->pid_ns;
sync_filesystem(sb);
@@ -157,14 +159,14 @@ static int proc_get_tree(struct fs_context *fc)
{
struct proc_fs_context *ctx = fc->fs_private;
- return get_tree_keyed(fc, proc_fill_super, ctx->pid_ns);
+ return get_tree_keyed(fc, proc_fill_super, ctx->fs_info);
}
static void proc_fs_context_free(struct fs_context *fc)
{
struct proc_fs_context *ctx = fc->fs_private;
- put_pid_ns(ctx->pid_ns);
+ put_pid_ns(ctx->fs_info->pid_ns);
kfree(ctx);
}
@@ -178,14 +180,27 @@ static const struct fs_context_operations proc_fs_context_ops = {
static int proc_init_fs_context(struct fs_context *fc)
{
struct proc_fs_context *ctx;
+ struct pid_namespace *pid_ns;
ctx = kzalloc(sizeof(struct proc_fs_context), GFP_KERNEL);
if (!ctx)
return -ENOMEM;
- ctx->pid_ns = get_pid_ns(task_active_pid_ns(current));
+ pid_ns = get_pid_ns(task_active_pid_ns(current));
+
+ if (!pid_ns->proc_mnt) {
+ ctx->fs_info = kzalloc(sizeof(struct proc_fs_info), GFP_KERNEL);
+ if (!ctx->fs_info) {
+ kfree(ctx);
+ return -ENOMEM;
+ }
+ ctx->fs_info->pid_ns = pid_ns;
+ } else {
+ ctx->fs_info = proc_sb_info(pid_ns->proc_mnt->mnt_sb);
+ }
+
put_user_ns(fc->user_ns);
- fc->user_ns = get_user_ns(ctx->pid_ns->user_ns);
+ fc->user_ns = get_user_ns(ctx->fs_info->pid_ns->user_ns);
fc->fs_private = ctx;
fc->ops = &proc_fs_context_ops;
return 0;
@@ -193,15 +208,15 @@ static int proc_init_fs_context(struct fs_context *fc)
static void proc_kill_sb(struct super_block *sb)
{
- struct pid_namespace *ns;
+ struct proc_fs_info *fs_info = proc_sb_info(sb);
- ns = (struct pid_namespace *)sb->s_fs_info;
- if (ns->proc_self)
- dput(ns->proc_self);
- if (ns->proc_thread_self)
- dput(ns->proc_thread_self);
+ if (fs_info->pid_ns->proc_self)
+ dput(fs_info->pid_ns->proc_self);
+ if (fs_info->pid_ns->proc_thread_self)
+ dput(fs_info->pid_ns->proc_thread_self);
kill_anon_super(sb);
- put_pid_ns(ns);
+ put_pid_ns(fs_info->pid_ns);
+ kfree(fs_info);
}
static struct file_system_type proc_fs_type = {
@@ -314,10 +329,10 @@ int pid_ns_prepare_proc(struct pid_namespace *ns)
}
ctx = fc->fs_private;
- if (ctx->pid_ns != ns) {
- put_pid_ns(ctx->pid_ns);
+ if (ctx->fs_info->pid_ns != ns) {
+ put_pid_ns(ctx->fs_info->pid_ns);
get_pid_ns(ns);
- ctx->pid_ns = ns;
+ ctx->fs_info->pid_ns = ns;
}
mnt = fc_mount(fc);
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index a705aa2d03f9..2d79489e55aa 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -12,6 +12,15 @@ struct proc_dir_entry;
struct seq_file;
struct seq_operations;
+struct proc_fs_info {
+ struct pid_namespace *pid_ns;
+};
+
+static inline struct proc_fs_info *proc_sb_info(struct super_block *sb)
+{
+ return sb->s_fs_info;
+}
+
#ifdef CONFIG_PROC_FS
typedef int (*proc_write_t)(struct file *, char *, size_t);
@@ -146,7 +155,7 @@ int open_related_ns(struct ns_common *ns,
/* get the associated pid namespace for a file in procfs */
static inline struct pid_namespace *proc_pid_ns(const struct inode *inode)
{
- return inode->i_sb->s_fs_info;
+ return proc_sb_info(inode->i_sb)->pid_ns;
}
#endif /* _LINUX_PROC_FS_H */
--
2.24.1
^ permalink raw reply related
* [PATCH v7 03/11] proc: move /proc/{self|thread-self} dentries to proc_fs_info
From: Alexey Gladkov @ 2020-01-25 13:05 UTC (permalink / raw)
To: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module
Cc: Akinobu Mita, Alexander Viro, Alexey Dobriyan, Alexey Gladkov,
Andrew Morton, Andy Lutomirski, Daniel Micay, Djalal Harouni,
Dmitry V . Levin, Eric W . Biederman, Greg Kroah-Hartman,
Ingo Molnar, J . Bruce Fields, Jeff Layton, Jonathan Corbet,
Kees Cook, Linus Torvalds, Oleg Nesterov, Solar Designer
In-Reply-To: <20200125130541.450409-1-gladkov.alexey@gmail.com>
This is a preparation patch that moves /proc/{self|thread-self} dentries
to be stored inside procfs fs_info struct instead of making them per pid
namespace. Since we want to support multiple procfs instances we need to
make sure that these dentries are also per-superblock instead of
per-pidns, unmounting a private procfs won't clash with other procfs
mounts.
Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
fs/proc/base.c | 5 +++--
fs/proc/root.c | 8 ++++----
fs/proc/self.c | 4 ++--
fs/proc/thread_self.c | 6 +++---
include/linux/pid_namespace.h | 4 +---
include/linux/proc_fs.h | 2 ++
6 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 672e71c52dbd..1eb366ad8b06 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3316,6 +3316,7 @@ static struct tgid_iter next_tgid(struct pid_namespace *ns, struct tgid_iter ite
int proc_pid_readdir(struct file *file, struct dir_context *ctx)
{
struct tgid_iter iter;
+ struct proc_fs_info *fs_info = proc_sb_info(file_inode(file)->i_sb);
struct pid_namespace *ns = proc_pid_ns(file_inode(file));
loff_t pos = ctx->pos;
@@ -3323,13 +3324,13 @@ int proc_pid_readdir(struct file *file, struct dir_context *ctx)
return 0;
if (pos == TGID_OFFSET - 2) {
- struct inode *inode = d_inode(ns->proc_self);
+ struct inode *inode = d_inode(fs_info->proc_self);
if (!dir_emit(ctx, "self", 4, inode->i_ino, DT_LNK))
return 0;
ctx->pos = pos = pos + 1;
}
if (pos == TGID_OFFSET - 1) {
- struct inode *inode = d_inode(ns->proc_thread_self);
+ struct inode *inode = d_inode(fs_info->proc_thread_self);
if (!dir_emit(ctx, "thread-self", 11, inode->i_ino, DT_LNK))
return 0;
ctx->pos = pos = pos + 1;
diff --git a/fs/proc/root.c b/fs/proc/root.c
index d449f095f0f7..637e26cc795e 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -210,10 +210,10 @@ static void proc_kill_sb(struct super_block *sb)
{
struct proc_fs_info *fs_info = proc_sb_info(sb);
- if (fs_info->pid_ns->proc_self)
- dput(fs_info->pid_ns->proc_self);
- if (fs_info->pid_ns->proc_thread_self)
- dput(fs_info->pid_ns->proc_thread_self);
+ if (fs_info->proc_self)
+ dput(fs_info->proc_self);
+ if (fs_info->proc_thread_self)
+ dput(fs_info->proc_thread_self);
kill_anon_super(sb);
put_pid_ns(fs_info->pid_ns);
kfree(fs_info);
diff --git a/fs/proc/self.c b/fs/proc/self.c
index 57c0a1047250..846fc2b7c8a8 100644
--- a/fs/proc/self.c
+++ b/fs/proc/self.c
@@ -36,7 +36,7 @@ static unsigned self_inum __ro_after_init;
int proc_setup_self(struct super_block *s)
{
struct inode *root_inode = d_inode(s->s_root);
- struct pid_namespace *ns = proc_pid_ns(root_inode);
+ struct proc_fs_info *fs_info = proc_sb_info(s);
struct dentry *self;
int ret = -ENOMEM;
@@ -62,7 +62,7 @@ int proc_setup_self(struct super_block *s)
if (ret)
pr_err("proc_fill_super: can't allocate /proc/self\n");
else
- ns->proc_self = self;
+ fs_info->proc_self = self;
return ret;
}
diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
index f61ae53533f5..2493cbbdfa6f 100644
--- a/fs/proc/thread_self.c
+++ b/fs/proc/thread_self.c
@@ -36,7 +36,7 @@ static unsigned thread_self_inum __ro_after_init;
int proc_setup_thread_self(struct super_block *s)
{
struct inode *root_inode = d_inode(s->s_root);
- struct pid_namespace *ns = proc_pid_ns(root_inode);
+ struct proc_fs_info *fs_info = proc_sb_info(s);
struct dentry *thread_self;
int ret = -ENOMEM;
@@ -60,9 +60,9 @@ int proc_setup_thread_self(struct super_block *s)
inode_unlock(root_inode);
if (ret)
- pr_err("proc_fill_super: can't allocate /proc/thread_self\n");
+ pr_err("proc_fill_super: can't allocate /proc/thread-self\n");
else
- ns->proc_thread_self = thread_self;
+ fs_info->proc_thread_self = thread_self;
return ret;
}
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index 49538b172483..f91a8bf6e09e 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -31,9 +31,7 @@ struct pid_namespace {
unsigned int level;
struct pid_namespace *parent;
#ifdef CONFIG_PROC_FS
- struct vfsmount *proc_mnt;
- struct dentry *proc_self;
- struct dentry *proc_thread_self;
+ struct vfsmount *proc_mnt; /* Internal proc mounted during each new pidns */
#endif
#ifdef CONFIG_BSD_PROCESS_ACCT
struct fs_pin *bacct;
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 2d79489e55aa..59162988998e 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -14,6 +14,8 @@ struct seq_operations;
struct proc_fs_info {
struct pid_namespace *pid_ns;
+ struct dentry *proc_self; /* For /proc/self */
+ struct dentry *proc_thread_self; /* For /proc/thread-self */
};
static inline struct proc_fs_info *proc_sb_info(struct super_block *sb)
--
2.24.1
^ permalink raw reply related
* [PATCH v7 04/11] proc: move hide_pid, pid_gid from pid_namespace to proc_fs_info
From: Alexey Gladkov @ 2020-01-25 13:05 UTC (permalink / raw)
To: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module
Cc: Akinobu Mita, Alexander Viro, Alexey Dobriyan, Alexey Gladkov,
Andrew Morton, Andy Lutomirski, Daniel Micay, Djalal Harouni,
Dmitry V . Levin, Eric W . Biederman, Greg Kroah-Hartman,
Ingo Molnar, J . Bruce Fields, Jeff Layton, Jonathan Corbet,
Kees Cook, Linus Torvalds, Oleg Nesterov, Solar Designer
In-Reply-To: <20200125130541.450409-1-gladkov.alexey@gmail.com>
This is a preparation patch that moves hide_pid and pid_gid parameters
to be stored inside procfs fs_info struct instead of making them per pid
namespace. Since we want to support multiple procfs instances we need to
make sure that all proc-specific parameters are also per-superblock.
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
fs/proc/base.c | 18 +++++++++---------
fs/proc/inode.c | 9 ++++-----
fs/proc/root.c | 10 ++++++++--
include/linux/pid_namespace.h | 8 --------
include/linux/proc_fs.h | 9 +++++++++
5 files changed, 30 insertions(+), 24 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 1eb366ad8b06..caca1929fee1 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -695,13 +695,13 @@ int proc_setattr(struct dentry *dentry, struct iattr *attr)
* May current process learn task's sched/cmdline info (for hide_pid_min=1)
* or euid/egid (for hide_pid_min=2)?
*/
-static bool has_pid_permissions(struct pid_namespace *pid,
+static bool has_pid_permissions(struct proc_fs_info *fs_info,
struct task_struct *task,
int hide_pid_min)
{
- if (pid->hide_pid < hide_pid_min)
+ if (fs_info->hide_pid < hide_pid_min)
return true;
- if (in_group_p(pid->pid_gid))
+ if (in_group_p(fs_info->pid_gid))
return true;
return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
}
@@ -709,18 +709,18 @@ static bool has_pid_permissions(struct pid_namespace *pid,
static int proc_pid_permission(struct inode *inode, int mask)
{
- struct pid_namespace *pid = proc_pid_ns(inode);
+ struct proc_fs_info *fs_info = proc_sb_info(inode->i_sb);
struct task_struct *task;
bool has_perms;
task = get_proc_task(inode);
if (!task)
return -ESRCH;
- has_perms = has_pid_permissions(pid, task, HIDEPID_NO_ACCESS);
+ has_perms = has_pid_permissions(fs_info, task, HIDEPID_NO_ACCESS);
put_task_struct(task);
if (!has_perms) {
- if (pid->hide_pid == HIDEPID_INVISIBLE) {
+ if (fs_info->hide_pid == HIDEPID_INVISIBLE) {
/*
* Let's make getdents(), stat(), and open()
* consistent with each other. If a process
@@ -1784,7 +1784,7 @@ int pid_getattr(const struct path *path, struct kstat *stat,
u32 request_mask, unsigned int query_flags)
{
struct inode *inode = d_inode(path->dentry);
- struct pid_namespace *pid = proc_pid_ns(inode);
+ struct proc_fs_info *fs_info = proc_sb_info(inode->i_sb);
struct task_struct *task;
generic_fillattr(inode, stat);
@@ -1794,7 +1794,7 @@ int pid_getattr(const struct path *path, struct kstat *stat,
rcu_read_lock();
task = pid_task(proc_pid(inode), PIDTYPE_PID);
if (task) {
- if (!has_pid_permissions(pid, task, HIDEPID_INVISIBLE)) {
+ if (!has_pid_permissions(fs_info, task, HIDEPID_INVISIBLE)) {
rcu_read_unlock();
/*
* This doesn't prevent learning whether PID exists,
@@ -3344,7 +3344,7 @@ int proc_pid_readdir(struct file *file, struct dir_context *ctx)
unsigned int len;
cond_resched();
- if (!has_pid_permissions(ns, iter.task, HIDEPID_INVISIBLE))
+ if (!has_pid_permissions(fs_info, iter.task, HIDEPID_INVISIBLE))
continue;
len = snprintf(name, sizeof(name), "%u", iter.tgid);
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index b631608dfbcc..b90c233e5968 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -105,12 +105,11 @@ void __init proc_init_kmemcache(void)
static int proc_show_options(struct seq_file *seq, struct dentry *root)
{
struct proc_fs_info *fs_info = proc_sb_info(root->d_sb);
- struct pid_namespace *pid = fs_info->pid_ns;
- if (!gid_eq(pid->pid_gid, GLOBAL_ROOT_GID))
- seq_printf(seq, ",gid=%u", from_kgid_munged(&init_user_ns, pid->pid_gid));
- if (pid->hide_pid != HIDEPID_OFF)
- seq_printf(seq, ",hidepid=%u", pid->hide_pid);
+ if (!gid_eq(fs_info->pid_gid, GLOBAL_ROOT_GID))
+ seq_printf(seq, ",gid=%u", from_kgid_munged(&init_user_ns, fs_info->pid_gid));
+ if (fs_info->hide_pid != HIDEPID_OFF)
+ seq_printf(seq, ",hidepid=%u", fs_info->hide_pid);
return 0;
}
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 637e26cc795e..1ca47d446aa4 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -89,10 +89,16 @@ static void proc_apply_options(struct super_block *s,
{
struct proc_fs_context *ctx = fc->fs_private;
+ if (pid_ns->proc_mnt) {
+ struct proc_fs_info *fs_info = proc_sb_info(pid_ns->proc_mnt->mnt_sb);
+ ctx->fs_info->pid_gid = fs_info->pid_gid;
+ ctx->fs_info->hide_pid = fs_info->hide_pid;
+ }
+
if (ctx->mask & (1 << Opt_gid))
- pid_ns->pid_gid = make_kgid(user_ns, ctx->gid);
+ ctx->fs_info->pid_gid = make_kgid(user_ns, ctx->gid);
if (ctx->mask & (1 << Opt_hidepid))
- pid_ns->hide_pid = ctx->hidepid;
+ ctx->fs_info->hide_pid = ctx->hidepid;
}
static int proc_fill_super(struct super_block *s, struct fs_context *fc)
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index f91a8bf6e09e..66f47f1afe0d 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -15,12 +15,6 @@
struct fs_pin;
-enum { /* definitions for pid_namespace's hide_pid field */
- HIDEPID_OFF = 0,
- HIDEPID_NO_ACCESS = 1,
- HIDEPID_INVISIBLE = 2,
-};
-
struct pid_namespace {
struct kref kref;
struct idr idr;
@@ -39,8 +33,6 @@ struct pid_namespace {
struct user_namespace *user_ns;
struct ucounts *ucounts;
struct work_struct proc_work;
- kgid_t pid_gid;
- int hide_pid;
int reboot; /* group exit code if this pidns was rebooted */
struct ns_common ns;
} __randomize_layout;
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 59162988998e..5f0b1b7e4271 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -12,10 +12,19 @@ struct proc_dir_entry;
struct seq_file;
struct seq_operations;
+/* definitions for hide_pid field */
+enum {
+ HIDEPID_OFF = 0,
+ HIDEPID_NO_ACCESS = 1,
+ HIDEPID_INVISIBLE = 2,
+};
+
struct proc_fs_info {
struct pid_namespace *pid_ns;
struct dentry *proc_self; /* For /proc/self */
struct dentry *proc_thread_self; /* For /proc/thread-self */
+ kgid_t pid_gid;
+ int hide_pid;
};
static inline struct proc_fs_info *proc_sb_info(struct super_block *sb)
--
2.24.1
^ permalink raw reply related
* [PATCH v7 05/11] proc: add helpers to set and get proc hidepid and gid mount options
From: Alexey Gladkov @ 2020-01-25 13:05 UTC (permalink / raw)
To: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module
Cc: Akinobu Mita, Alexander Viro, Alexey Dobriyan, Alexey Gladkov,
Andrew Morton, Andy Lutomirski, Daniel Micay, Djalal Harouni,
Dmitry V . Levin, Eric W . Biederman, Greg Kroah-Hartman,
Ingo Molnar, J . Bruce Fields, Jeff Layton, Jonathan Corbet,
Kees Cook, Linus Torvalds, Oleg Nesterov, Solar Designer
In-Reply-To: <20200125130541.450409-1-gladkov.alexey@gmail.com>
This is a cleaning patch to add helpers to set and get proc mount
options instead of directly using them. This make it easy to track
what's happening and easy to update in future.
Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
fs/proc/base.c | 6 +++---
fs/proc/inode.c | 11 +++++++----
fs/proc/internal.h | 20 ++++++++++++++++++++
fs/proc/root.c | 8 ++++----
4 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index caca1929fee1..4ccb280a3e79 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -699,9 +699,9 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
struct task_struct *task,
int hide_pid_min)
{
- if (fs_info->hide_pid < hide_pid_min)
+ if (proc_fs_hide_pid(fs_info) < hide_pid_min)
return true;
- if (in_group_p(fs_info->pid_gid))
+ if (in_group_p(proc_fs_pid_gid(fs_info)))
return true;
return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
}
@@ -720,7 +720,7 @@ static int proc_pid_permission(struct inode *inode, int mask)
put_task_struct(task);
if (!has_perms) {
- if (fs_info->hide_pid == HIDEPID_INVISIBLE) {
+ if (proc_fs_hide_pid(fs_info) == HIDEPID_INVISIBLE) {
/*
* Let's make getdents(), stat(), and open()
* consistent with each other. If a process
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index b90c233e5968..70b722fb8811 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -105,11 +105,14 @@ void __init proc_init_kmemcache(void)
static int proc_show_options(struct seq_file *seq, struct dentry *root)
{
struct proc_fs_info *fs_info = proc_sb_info(root->d_sb);
+ int hidepid = proc_fs_hide_pid(fs_info);
+ kgid_t gid = proc_fs_pid_gid(fs_info);
- if (!gid_eq(fs_info->pid_gid, GLOBAL_ROOT_GID))
- seq_printf(seq, ",gid=%u", from_kgid_munged(&init_user_ns, fs_info->pid_gid));
- if (fs_info->hide_pid != HIDEPID_OFF)
- seq_printf(seq, ",hidepid=%u", fs_info->hide_pid);
+ if (!gid_eq(gid, GLOBAL_ROOT_GID))
+ seq_printf(seq, ",gid=%u", from_kgid_munged(&init_user_ns, gid));
+
+ if (hidepid != HIDEPID_OFF)
+ seq_printf(seq, ",hidepid=%u", hidepid);
return 0;
}
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index cd0c8d5ce9a1..ff2f274b2e0d 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -121,6 +121,26 @@ static inline struct task_struct *get_proc_task(const struct inode *inode)
return get_pid_task(proc_pid(inode), PIDTYPE_PID);
}
+static inline void proc_fs_set_hide_pid(struct proc_fs_info *fs_info, int hide_pid)
+{
+ fs_info->hide_pid = hide_pid;
+}
+
+static inline void proc_fs_set_pid_gid(struct proc_fs_info *fs_info, kgid_t gid)
+{
+ fs_info->pid_gid = gid;
+}
+
+static inline int proc_fs_hide_pid(struct proc_fs_info *fs_info)
+{
+ return fs_info->hide_pid;
+}
+
+static inline kgid_t proc_fs_pid_gid(struct proc_fs_info *fs_info)
+{
+ return fs_info->pid_gid;
+}
+
void task_dump_owner(struct task_struct *task, umode_t mode,
kuid_t *ruid, kgid_t *rgid);
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 1ca47d446aa4..efd76c004e86 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -91,14 +91,14 @@ static void proc_apply_options(struct super_block *s,
if (pid_ns->proc_mnt) {
struct proc_fs_info *fs_info = proc_sb_info(pid_ns->proc_mnt->mnt_sb);
- ctx->fs_info->pid_gid = fs_info->pid_gid;
- ctx->fs_info->hide_pid = fs_info->hide_pid;
+ proc_fs_set_pid_gid(ctx->fs_info, proc_fs_pid_gid(fs_info));
+ proc_fs_set_hide_pid(ctx->fs_info, proc_fs_hide_pid(fs_info));
}
if (ctx->mask & (1 << Opt_gid))
- ctx->fs_info->pid_gid = make_kgid(user_ns, ctx->gid);
+ proc_fs_set_pid_gid(ctx->fs_info, make_kgid(user_ns, ctx->gid));
if (ctx->mask & (1 << Opt_hidepid))
- ctx->fs_info->hide_pid = ctx->hidepid;
+ proc_fs_set_hide_pid(ctx->fs_info, ctx->hidepid);
}
static int proc_fill_super(struct super_block *s, struct fs_context *fc)
--
2.24.1
^ permalink raw reply related
* [PATCH v7 06/11] proc: support mounting procfs instances inside same pid namespace
From: Alexey Gladkov @ 2020-01-25 13:05 UTC (permalink / raw)
To: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module
Cc: Akinobu Mita, Alexander Viro, Alexey Dobriyan, Alexey Gladkov,
Andrew Morton, Andy Lutomirski, Daniel Micay, Djalal Harouni,
Dmitry V . Levin, Eric W . Biederman, Greg Kroah-Hartman,
Ingo Molnar, J . Bruce Fields, Jeff Layton, Jonathan Corbet,
Kees Cook, Linus Torvalds, Oleg Nesterov, Solar Designer
In-Reply-To: <20200125130541.450409-1-gladkov.alexey@gmail.com>
This patch allows to have multiple procfs instances inside the
same pid namespace. The aim here is lightweight sandboxes, and to allow
that we have to modernize procfs internals.
1) The main aim of this work is to have on embedded systems one
supervisor for apps. Right now we have some lightweight sandbox support,
however if we create pid namespacess we have to manages all the
processes inside too, where our goal is to be able to run a bunch of
apps each one inside its own mount namespace without being able to
notice each other. We only want to use mount namespaces, and we want
procfs to behave more like a real mount point.
2) Linux Security Modules have multiple ptrace paths inside some
subsystems, however inside procfs, the implementation does not guarantee
that the ptrace() check which triggers the security_ptrace_check() hook
will always run. We have the 'hidepid' mount option that can be used to
force the ptrace_may_access() check inside has_pid_permissions() to run.
The problem is that 'hidepid' is per pid namespace and not attached to
the mount point, any remount or modification of 'hidepid' will propagate
to all other procfs mounts.
This also does not allow to support Yama LSM easily in desktop and user
sessions. Yama ptrace scope which restricts ptrace and some other
syscalls to be allowed only on inferiors, can be updated to have a
per-task context, where the context will be inherited during fork(),
clone() and preserved across execve(). If we support multiple private
procfs instances, then we may force the ptrace_may_access() on
/proc/<pids>/ to always run inside that new procfs instances. This will
allow to specifiy on user sessions if we should populate procfs with
pids that the user can ptrace or not.
By using Yama ptrace scope, some restricted users will only be able to see
inferiors inside /proc, they won't even be able to see their other
processes. Some software like Chromium, Firefox's crash handler, Wine
and others are already using Yama to restrict which processes can be
ptracable. With this change this will give the possibility to restrict
/proc/<pids>/ but more importantly this will give desktop users a
generic and usuable way to specifiy which users should see all processes
and which users can not.
Side notes:
* This covers the lack of seccomp where it is not able to parse
arguments, it is easy to install a seccomp filter on direct syscalls
that operate on pids, however /proc/<pid>/ is a Linux ABI using
filesystem syscalls. With this change LSMs should be able to analyze
open/read/write/close...
In the new patchset version I removed the 'newinstance' option
as Eric W. Biederman suggested.
Cc: Kees Cook <keescook@chromium.org>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
fs/proc/root.c | 41 ++++++++++++++++++-----------------------
1 file changed, 18 insertions(+), 23 deletions(-)
diff --git a/fs/proc/root.c b/fs/proc/root.c
index efd76c004e86..5d5cba4c899b 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -82,7 +82,7 @@ static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
return 0;
}
-static void proc_apply_options(struct super_block *s,
+static void proc_apply_options(struct proc_fs_info *fs_info,
struct fs_context *fc,
struct pid_namespace *pid_ns,
struct user_namespace *user_ns)
@@ -90,15 +90,17 @@ static void proc_apply_options(struct super_block *s,
struct proc_fs_context *ctx = fc->fs_private;
if (pid_ns->proc_mnt) {
- struct proc_fs_info *fs_info = proc_sb_info(pid_ns->proc_mnt->mnt_sb);
- proc_fs_set_pid_gid(ctx->fs_info, proc_fs_pid_gid(fs_info));
- proc_fs_set_hide_pid(ctx->fs_info, proc_fs_hide_pid(fs_info));
+ struct proc_fs_info *pidns_fs_info = proc_sb_info(pid_ns->proc_mnt->mnt_sb);
+
+ proc_fs_set_pid_gid(fs_info, proc_fs_pid_gid(pidns_fs_info));
+ proc_fs_set_hide_pid(fs_info, proc_fs_hide_pid(pidns_fs_info));
}
if (ctx->mask & (1 << Opt_gid))
- proc_fs_set_pid_gid(ctx->fs_info, make_kgid(user_ns, ctx->gid));
+ proc_fs_set_pid_gid(fs_info, make_kgid(user_ns, ctx->gid));
+
if (ctx->mask & (1 << Opt_hidepid))
- proc_fs_set_hide_pid(ctx->fs_info, ctx->hidepid);
+ proc_fs_set_hide_pid(fs_info, ctx->hidepid);
}
static int proc_fill_super(struct super_block *s, struct fs_context *fc)
@@ -108,7 +110,7 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
struct inode *root_inode;
int ret;
- proc_apply_options(s, fc, pid_ns, current_user_ns());
+ proc_apply_options(ctx->fs_info, fc, pid_ns, current_user_ns());
/* User space would break if executables or devices appear on proc */
s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
@@ -118,6 +120,7 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
s->s_magic = PROC_SUPER_MAGIC;
s->s_op = &proc_sops;
s->s_time_gran = 1;
+ s->s_fs_info = ctx->fs_info;
/*
* procfs isn't actually a stacking filesystem; however, there is
@@ -157,15 +160,13 @@ static int proc_reconfigure(struct fs_context *fc)
sync_filesystem(sb);
- proc_apply_options(sb, fc, pid, current_user_ns());
+ proc_apply_options(fs_info, fc, pid, current_user_ns());
return 0;
}
static int proc_get_tree(struct fs_context *fc)
{
- struct proc_fs_context *ctx = fc->fs_private;
-
- return get_tree_keyed(fc, proc_fill_super, ctx->fs_info);
+ return get_tree_nodev(fc, proc_fill_super);
}
static void proc_fs_context_free(struct fs_context *fc)
@@ -186,25 +187,19 @@ static const struct fs_context_operations proc_fs_context_ops = {
static int proc_init_fs_context(struct fs_context *fc)
{
struct proc_fs_context *ctx;
- struct pid_namespace *pid_ns;
ctx = kzalloc(sizeof(struct proc_fs_context), GFP_KERNEL);
if (!ctx)
return -ENOMEM;
- pid_ns = get_pid_ns(task_active_pid_ns(current));
-
- if (!pid_ns->proc_mnt) {
- ctx->fs_info = kzalloc(sizeof(struct proc_fs_info), GFP_KERNEL);
- if (!ctx->fs_info) {
- kfree(ctx);
- return -ENOMEM;
- }
- ctx->fs_info->pid_ns = pid_ns;
- } else {
- ctx->fs_info = proc_sb_info(pid_ns->proc_mnt->mnt_sb);
+ ctx->fs_info = kzalloc(sizeof(struct proc_fs_info), GFP_KERNEL);
+ if (!ctx->fs_info) {
+ kfree(ctx);
+ return -ENOMEM;
}
+ ctx->fs_info->pid_ns = get_pid_ns(task_active_pid_ns(current));
+
put_user_ns(fc->user_ns);
fc->user_ns = get_user_ns(ctx->fs_info->pid_ns->user_ns);
fc->fs_private = ctx;
--
2.24.1
^ permalink raw reply related
* [PATCH v7 07/11] proc: flush task dcache entries from all procfs instances
From: Alexey Gladkov @ 2020-01-25 13:05 UTC (permalink / raw)
To: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module
Cc: Akinobu Mita, Alexander Viro, Alexey Dobriyan, Alexey Gladkov,
Andrew Morton, Andy Lutomirski, Daniel Micay, Djalal Harouni,
Dmitry V . Levin, Eric W . Biederman, Greg Kroah-Hartman,
Ingo Molnar, J . Bruce Fields, Jeff Layton, Jonathan Corbet,
Kees Cook, Linus Torvalds, Oleg Nesterov, Solar Designer
In-Reply-To: <20200125130541.450409-1-gladkov.alexey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
This allows to flush dcache entries of a task on multiple procfs mounts
per pid namespace.
Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Djalal Harouni <tixxdz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Alexey Gladkov <gladkov.alexey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
fs/proc/base.c | 23 ++++++++++++++-----
fs/proc/root.c | 14 ++++++++++++
include/linux/pid_namespace.h | 42 +++++++++++++++++++++++++++++++++++
include/linux/proc_fs.h | 2 ++
4 files changed, 76 insertions(+), 5 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 4ccb280a3e79..f4f1bcb28603 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3133,7 +3133,7 @@ static const struct inode_operations proc_tgid_base_inode_operations = {
.permission = proc_pid_permission,
};
-static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)
+static void proc_flush_task_mnt_root(struct dentry *mnt_root, pid_t pid, pid_t tgid)
{
struct dentry *dentry, *leader, *dir;
char buf[10 + 1];
@@ -3142,7 +3142,7 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)
name.name = buf;
name.len = snprintf(buf, sizeof(buf), "%u", pid);
/* no ->d_hash() rejects on procfs */
- dentry = d_hash_and_lookup(mnt->mnt_root, &name);
+ dentry = d_hash_and_lookup(mnt_root, &name);
if (dentry) {
d_invalidate(dentry);
dput(dentry);
@@ -3153,7 +3153,7 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)
name.name = buf;
name.len = snprintf(buf, sizeof(buf), "%u", tgid);
- leader = d_hash_and_lookup(mnt->mnt_root, &name);
+ leader = d_hash_and_lookup(mnt_root, &name);
if (!leader)
goto out;
@@ -3208,14 +3208,27 @@ void proc_flush_task(struct task_struct *task)
int i;
struct pid *pid, *tgid;
struct upid *upid;
+ struct pid_namespace *pid_ns;
+ struct dentry *mnt_root;
+ struct proc_fs_info *fs_info;
pid = task_pid(task);
tgid = task_tgid(task);
for (i = 0; i <= pid->level; i++) {
upid = &pid->numbers[i];
- proc_flush_task_mnt(upid->ns->proc_mnt, upid->nr,
- tgid->numbers[i].nr);
+
+ pid_ns = upid->ns;
+
+ pidns_proc_lock_shared(pid_ns);
+ list_for_each_entry(fs_info, &pid_ns->proc_mounts, pidns_entry) {
+ mnt_root = fs_info->m_super->s_root;
+ proc_flush_task_mnt_root(mnt_root, upid->nr, tgid->numbers[i].nr);
+ }
+ pidns_proc_unlock_shared(pid_ns);
+
+ mnt_root = pid_ns->proc_mnt->mnt_root;
+ proc_flush_task_mnt_root(mnt_root, upid->nr, tgid->numbers[i].nr);
}
}
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 5d5cba4c899b..3bb8df360cf7 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -112,6 +112,12 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
proc_apply_options(ctx->fs_info, fc, pid_ns, current_user_ns());
+ ctx->fs_info->m_super = s;
+
+ pidns_proc_lock(pid_ns);
+ list_add_tail(&ctx->fs_info->pidns_entry, &pid_ns->proc_mounts);
+ pidns_proc_unlock(pid_ns);
+
/* User space would break if executables or devices appear on proc */
s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
s->s_flags |= SB_NODIRATIME | SB_NOSUID | SB_NOEXEC;
@@ -215,6 +221,11 @@ static void proc_kill_sb(struct super_block *sb)
dput(fs_info->proc_self);
if (fs_info->proc_thread_self)
dput(fs_info->proc_thread_self);
+
+ pidns_proc_lock(fs_info->pid_ns);
+ list_del(&fs_info->pidns_entry);
+ pidns_proc_unlock(fs_info->pid_ns);
+
kill_anon_super(sb);
put_pid_ns(fs_info->pid_ns);
kfree(fs_info);
@@ -336,6 +347,9 @@ int pid_ns_prepare_proc(struct pid_namespace *ns)
ctx->fs_info->pid_ns = ns;
}
+ init_rwsem(&ns->rw_proc_mounts);
+ INIT_LIST_HEAD(&ns->proc_mounts);
+
mnt = fc_mount(fc);
put_fs_context(fc);
if (IS_ERR(mnt))
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index 66f47f1afe0d..297b39604312 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -26,6 +26,8 @@ struct pid_namespace {
struct pid_namespace *parent;
#ifdef CONFIG_PROC_FS
struct vfsmount *proc_mnt; /* Internal proc mounted during each new pidns */
+ struct rw_semaphore rw_proc_mounts;
+ struct list_head proc_mounts; /* list of separated procfs mounts */
#endif
#ifdef CONFIG_BSD_PROCESS_ACCT
struct fs_pin *bacct;
@@ -90,4 +92,44 @@ extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk);
void pidhash_init(void);
void pid_idr_init(void);
+#ifdef CONFIG_PROC_FS
+static inline void pidns_proc_lock(struct pid_namespace *pid_ns)
+{
+ down_write(&pid_ns->rw_proc_mounts);
+}
+
+static inline void pidns_proc_unlock(struct pid_namespace *pid_ns)
+{
+ up_write(&pid_ns->rw_proc_mounts);
+}
+
+static inline void pidns_proc_lock_shared(struct pid_namespace *pid_ns)
+{
+ down_read(&pid_ns->rw_proc_mounts);
+}
+
+static inline void pidns_proc_unlock_shared(struct pid_namespace *pid_ns)
+{
+ up_read(&pid_ns->rw_proc_mounts);
+}
+#else /* !CONFIG_PROC_FS */
+
+static inline void pidns_proc_lock(struct pid_namespace *pid_ns)
+{
+}
+
+static inline void pidns_proc_unlock(struct pid_namespace *pid_ns)
+{
+}
+
+static inline void pidns_proc_lock_shared(struct pid_namespace *pid_ns)
+{
+}
+
+static inline void pidns_proc_unlock_shared(struct pid_namespace *pid_ns)
+{
+}
+
+#endif /* CONFIG_PROC_FS */
+
#endif /* _LINUX_PID_NS_H */
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 5f0b1b7e4271..f307940f8311 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -20,6 +20,8 @@ enum {
};
struct proc_fs_info {
+ struct list_head pidns_entry; /* Node in procfs_mounts of a pidns */
+ struct super_block *m_super;
struct pid_namespace *pid_ns;
struct dentry *proc_self; /* For /proc/self */
struct dentry *proc_thread_self; /* For /proc/thread-self */
--
2.24.1
^ permalink raw reply related
* [PATCH v7 08/11] proc: instantiate only pids that we can ptrace on 'hidepid=4' mount option
From: Alexey Gladkov @ 2020-01-25 13:05 UTC (permalink / raw)
To: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module
Cc: Akinobu Mita, Alexander Viro, Alexey Dobriyan, Alexey Gladkov,
Andrew Morton, Andy Lutomirski, Daniel Micay, Djalal Harouni,
Dmitry V . Levin, Eric W . Biederman, Greg Kroah-Hartman,
Ingo Molnar, J . Bruce Fields, Jeff Layton, Jonathan Corbet,
Kees Cook, Linus Torvalds, Oleg Nesterov, Solar Designer
In-Reply-To: <20200125130541.450409-1-gladkov.alexey@gmail.com>
If "hidepid=4" mount option is set then do not instantiate pids that
we can not ptrace. "hidepid=4" means that procfs should only contain
pids that the caller can ptrace.
Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
fs/proc/base.c | 15 +++++++++++++++
fs/proc/root.c | 14 +++++++++++---
include/linux/proc_fs.h | 1 +
3 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index f4f1bcb28603..b55d205a7f6e 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -699,6 +699,14 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
struct task_struct *task,
int hide_pid_min)
{
+ /*
+ * If 'hidpid' mount option is set force a ptrace check,
+ * we indicate that we are using a filesystem syscall
+ * by passing PTRACE_MODE_READ_FSCREDS
+ */
+ if (proc_fs_hide_pid(fs_info) == HIDEPID_NOT_PTRACABLE)
+ return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
+
if (proc_fs_hide_pid(fs_info) < hide_pid_min)
return true;
if (in_group_p(proc_fs_pid_gid(fs_info)))
@@ -3274,7 +3282,14 @@ struct dentry *proc_pid_lookup(struct dentry *dentry, unsigned int flags)
if (!task)
goto out;
+ /* Limit procfs to only ptracable tasks */
+ if (proc_fs_hide_pid(fs_info) == HIDEPID_NOT_PTRACABLE) {
+ if (!has_pid_permissions(fs_info, task, HIDEPID_NO_ACCESS))
+ goto out_put_task;
+ }
+
result = proc_pid_instantiate(dentry, task, NULL);
+out_put_task:
put_task_struct(task);
out:
return result;
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 3bb8df360cf7..57276cb65528 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -52,6 +52,15 @@ static const struct fs_parameter_description proc_fs_parameters = {
.specs = proc_param_specs,
};
+static inline int
+valid_hidepid(unsigned int value)
+{
+ return (value == HIDEPID_OFF ||
+ value == HIDEPID_NO_ACCESS ||
+ value == HIDEPID_INVISIBLE ||
+ value == HIDEPID_NOT_PTRACABLE);
+}
+
static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
{
struct proc_fs_context *ctx = fc->fs_private;
@@ -68,10 +77,9 @@ static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
break;
case Opt_hidepid:
+ if (!valid_hidepid(result.uint_32))
+ return invalf(fc, "proc: unknown value of hidepid.\n");
ctx->hidepid = result.uint_32;
- if (ctx->hidepid < HIDEPID_OFF ||
- ctx->hidepid > HIDEPID_INVISIBLE)
- return invalf(fc, "proc: hidepid value must be between 0 and 2.\n");
break;
default:
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index f307940f8311..6822548405a7 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -17,6 +17,7 @@ enum {
HIDEPID_OFF = 0,
HIDEPID_NO_ACCESS = 1,
HIDEPID_INVISIBLE = 2,
+ HIDEPID_NOT_PTRACABLE = 4, /* Limit pids to only ptracable pids */
};
struct proc_fs_info {
--
2.24.1
^ permalink raw reply related
* [PATCH v7 09/11] proc: add option to mount only a pids subset
From: Alexey Gladkov @ 2020-01-25 13:05 UTC (permalink / raw)
To: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module
Cc: Akinobu Mita, Alexander Viro, Alexey Dobriyan, Alexey Gladkov,
Andrew Morton, Andy Lutomirski, Daniel Micay, Djalal Harouni,
Dmitry V . Levin, Eric W . Biederman, Greg Kroah-Hartman,
Ingo Molnar, J . Bruce Fields, Jeff Layton, Jonathan Corbet,
Kees Cook, Linus Torvalds, Oleg Nesterov, Solar Designer
In-Reply-To: <20200125130541.450409-1-gladkov.alexey@gmail.com>
This allows to hide all files and directories in the procfs that are not
related to tasks.
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
fs/proc/generic.c | 9 +++++++++
fs/proc/inode.c | 7 +++++++
fs/proc/internal.h | 10 ++++++++++
fs/proc/root.c | 36 ++++++++++++++++++++++++++++++++++++
include/linux/proc_fs.h | 7 +++++++
5 files changed, 69 insertions(+)
diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index 64e9ee1b129e..6f6517d63053 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -267,6 +267,11 @@ struct dentry *proc_lookup_de(struct inode *dir, struct dentry *dentry,
struct dentry *proc_lookup(struct inode *dir, struct dentry *dentry,
unsigned int flags)
{
+ struct proc_fs_info *fs_info = proc_sb_info(dir->i_sb);
+
+ if (proc_fs_pidonly(fs_info) == PROC_PIDONLY_ON)
+ return ERR_PTR(-ENOENT);
+
return proc_lookup_de(dir, dentry, PDE(dir));
}
@@ -323,6 +328,10 @@ int proc_readdir_de(struct file *file, struct dir_context *ctx,
int proc_readdir(struct file *file, struct dir_context *ctx)
{
struct inode *inode = file_inode(file);
+ struct proc_fs_info *fs_info = proc_sb_info(inode->i_sb);
+
+ if (proc_fs_pidonly(fs_info) == PROC_PIDONLY_ON)
+ return 1;
return proc_readdir_de(file, ctx, PDE(inode));
}
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 70b722fb8811..f35eef117775 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -114,6 +114,9 @@ static int proc_show_options(struct seq_file *seq, struct dentry *root)
if (hidepid != HIDEPID_OFF)
seq_printf(seq, ",hidepid=%u", hidepid);
+ if (proc_fs_pidonly(fs_info) != PROC_PIDONLY_OFF)
+ seq_printf(seq, ",subset=pidfs");
+
return 0;
}
@@ -333,12 +336,16 @@ proc_reg_get_unmapped_area(struct file *file, unsigned long orig_addr,
static int proc_reg_open(struct inode *inode, struct file *file)
{
+ struct proc_fs_info *fs_info = proc_sb_info(inode->i_sb);
struct proc_dir_entry *pde = PDE(inode);
int rv = 0;
typeof_member(struct file_operations, open) open;
typeof_member(struct file_operations, release) release;
struct pde_opener *pdeo;
+ if (proc_fs_pidonly(fs_info) == PROC_PIDONLY_ON)
+ return -ENOENT;
+
/*
* Ensure that
* 1) PDE's ->release hook will be called no matter what
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index ff2f274b2e0d..e2c729267317 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -126,6 +126,11 @@ static inline void proc_fs_set_hide_pid(struct proc_fs_info *fs_info, int hide_p
fs_info->hide_pid = hide_pid;
}
+static inline void proc_fs_set_pidonly(struct proc_fs_info *fs_info, int value)
+{
+ fs_info->pidonly = value;
+}
+
static inline void proc_fs_set_pid_gid(struct proc_fs_info *fs_info, kgid_t gid)
{
fs_info->pid_gid = gid;
@@ -141,6 +146,11 @@ static inline kgid_t proc_fs_pid_gid(struct proc_fs_info *fs_info)
return fs_info->pid_gid;
}
+static inline int proc_fs_pidonly(struct proc_fs_info *fs_info)
+{
+ return fs_info->pidonly;
+}
+
void task_dump_owner(struct task_struct *task, umode_t mode,
kuid_t *ruid, kgid_t *rgid);
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 57276cb65528..8e8d5c930e32 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -34,16 +34,19 @@ struct proc_fs_context {
unsigned int mask;
int hidepid;
int gid;
+ int pidonly;
};
enum proc_param {
Opt_gid,
Opt_hidepid,
+ Opt_subset,
};
static const struct fs_parameter_spec proc_param_specs[] = {
fsparam_u32("gid", Opt_gid),
fsparam_u32("hidepid", Opt_hidepid),
+ fsparam_string("subset", Opt_subset),
{}
};
@@ -61,6 +64,30 @@ valid_hidepid(unsigned int value)
value == HIDEPID_NOT_PTRACABLE);
}
+static inline int
+proc_parse_subset_param(struct fs_context *fc, char *value)
+{
+ struct proc_fs_context *ctx = fc->fs_private;
+
+ while (value) {
+ char *ptr = strchr(value, ',');
+
+ if (ptr != NULL)
+ *ptr++ = '\0';
+
+ if (*value != '\0') {
+ if (!strcmp(value, "pidfs")) {
+ ctx->pidonly = PROC_PIDONLY_ON;
+ } else {
+ return invalf(fc, "proc: unsupported subset option - %s\n", value);
+ }
+ }
+ value = ptr;
+ }
+
+ return 0;
+}
+
static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
{
struct proc_fs_context *ctx = fc->fs_private;
@@ -82,6 +109,11 @@ static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
ctx->hidepid = result.uint_32;
break;
+ case Opt_subset:
+ if (proc_parse_subset_param(fc, param->string) < 0)
+ return -EINVAL;
+ break;
+
default:
return -EINVAL;
}
@@ -102,6 +134,7 @@ static void proc_apply_options(struct proc_fs_info *fs_info,
proc_fs_set_pid_gid(fs_info, proc_fs_pid_gid(pidns_fs_info));
proc_fs_set_hide_pid(fs_info, proc_fs_hide_pid(pidns_fs_info));
+ proc_fs_set_pidonly(fs_info, proc_fs_pidonly(pidns_fs_info));
}
if (ctx->mask & (1 << Opt_gid))
@@ -109,6 +142,9 @@ static void proc_apply_options(struct proc_fs_info *fs_info,
if (ctx->mask & (1 << Opt_hidepid))
proc_fs_set_hide_pid(fs_info, ctx->hidepid);
+
+ if (ctx->mask & (1 << Opt_subset))
+ proc_fs_set_pidonly(fs_info, ctx->pidonly);
}
static int proc_fill_super(struct super_block *s, struct fs_context *fc)
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 6822548405a7..3ad0a47c3556 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -20,6 +20,12 @@ enum {
HIDEPID_NOT_PTRACABLE = 4, /* Limit pids to only ptracable pids */
};
+/* definitions for proc mount option pidonly */
+enum {
+ PROC_PIDONLY_OFF = 0,
+ PROC_PIDONLY_ON = 1,
+};
+
struct proc_fs_info {
struct list_head pidns_entry; /* Node in procfs_mounts of a pidns */
struct super_block *m_super;
@@ -28,6 +34,7 @@ struct proc_fs_info {
struct dentry *proc_thread_self; /* For /proc/thread-self */
kgid_t pid_gid;
int hide_pid;
+ int pidonly;
};
static inline struct proc_fs_info *proc_sb_info(struct super_block *sb)
--
2.24.1
^ permalink raw reply related
* [PATCH v7 10/11] docs: proc: add documentation for "hidepid=4" and "subset=pidfs" options and new mount behavior
From: Alexey Gladkov @ 2020-01-25 13:05 UTC (permalink / raw)
To: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module
Cc: Akinobu Mita, Alexander Viro, Alexey Dobriyan, Alexey Gladkov,
Andrew Morton, Andy Lutomirski, Daniel Micay, Djalal Harouni,
Dmitry V . Levin, Eric W . Biederman, Greg Kroah-Hartman,
Ingo Molnar, J . Bruce Fields, Jeff Layton, Jonathan Corbet,
Kees Cook, Linus Torvalds, Oleg Nesterov, Solar Designer
In-Reply-To: <20200125130541.450409-1-gladkov.alexey@gmail.com>
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
Documentation/filesystems/proc.txt | 53 ++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index 99ca040e3f90..4741fd092f36 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -50,6 +50,8 @@ Table of Contents
4 Configuring procfs
4.1 Mount options
+ 5 Filesystem behavior
+
------------------------------------------------------------------------------
Preface
------------------------------------------------------------------------------
@@ -2021,6 +2023,7 @@ The following mount options are supported:
hidepid= Set /proc/<pid>/ access mode.
gid= Set the group authorized to learn processes information.
+ subset= Show only the specified subset of procfs.
hidepid=0 means classic mode - everybody may access all /proc/<pid>/ directories
(default).
@@ -2042,6 +2045,56 @@ information about running processes, whether some daemon runs with elevated
privileges, whether other user runs some sensitive program, whether other users
run any program at all, etc.
+hidepid=4 means that procfs should only contain /proc/<pid>/ directories
+that the caller can ptrace.
+
gid= defines a group authorized to learn processes information otherwise
prohibited by hidepid=. If you use some daemon like identd which needs to learn
information about processes information, just add identd to this group.
+
+subset=pidfs hides all top level files and directories in the procfs that
+are not related to tasks.
+
+------------------------------------------------------------------------------
+5 Filesystem behavior
+------------------------------------------------------------------------------
+
+Originally, before the advent of pid namepsace, procfs was a global file
+system. It means that there was only one procfs instance in the system.
+
+When pid namespace was added, a separate procfs instance was mounted in
+each pid namespace. So, procfs mount options are global among all
+mountpoints within the same namespace.
+
+# grep ^proc /proc/mounts
+proc /proc proc rw,relatime,hidepid=2 0 0
+
+# strace -e mount mount -o hidepid=1 -t proc proc /tmp/proc
+mount("proc", "/tmp/proc", "proc", 0, "hidepid=1") = 0
++++ exited with 0 +++
+
+# grep ^proc /proc/mounts
+proc /proc proc rw,relatime,hidepid=2 0 0
+proc /tmp/proc proc rw,relatime,hidepid=2 0 0
+
+and only after remounting procfs mount options will change at all
+mountpoints.
+
+# mount -o remount,hidepid=1 -t proc proc /tmp/proc
+
+# grep ^proc /proc/mounts
+proc /proc proc rw,relatime,hidepid=1 0 0
+proc /tmp/proc proc rw,relatime,hidepid=1 0 0
+
+This behavior is different from the behavior of other filesystems.
+
+The new procfs behavior is more like other filesystems. Each procfs mount
+creates a new procfs instance. Mount options affect own procfs instance.
+It means that it became possible to have several procfs instances
+displaying tasks with different filtering options in one pid namespace.
+
+# mount -o hidepid=2 -t proc proc /proc
+# mount -o hidepid=1 -t proc proc /tmp/proc
+# grep ^proc /proc/mounts
+proc /proc proc rw,relatime,hidepid=2 0 0
+proc /tmp/proc proc rw,relatime,hidepid=1 0 0
--
2.24.1
^ permalink raw reply related
* [PATCH v7 11/11] proc: Move hidepid values to uapi as they are user interface to mount
From: Alexey Gladkov @ 2020-01-25 13:05 UTC (permalink / raw)
To: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module
Cc: Akinobu Mita, Alexander Viro, Alexey Dobriyan, Alexey Gladkov,
Andrew Morton, Andy Lutomirski, Daniel Micay, Djalal Harouni,
Dmitry V . Levin, Eric W . Biederman, Greg Kroah-Hartman,
Ingo Molnar, J . Bruce Fields, Jeff Layton, Jonathan Corbet,
Kees Cook, Linus Torvalds, Oleg Nesterov, Solar Designer
In-Reply-To: <20200125130541.450409-1-gladkov.alexey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Suggested-by: Alexey Dobriyan <adobriyan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Alexey Gladkov <gladkov.alexey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
include/linux/proc_fs.h | 9 +--------
include/uapi/linux/proc_fs.h | 13 +++++++++++++
2 files changed, 14 insertions(+), 8 deletions(-)
create mode 100644 include/uapi/linux/proc_fs.h
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 3ad0a47c3556..f2b4a411d371 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -7,19 +7,12 @@
#include <linux/types.h>
#include <linux/fs.h>
+#include <uapi/linux/proc_fs.h>
struct proc_dir_entry;
struct seq_file;
struct seq_operations;
-/* definitions for hide_pid field */
-enum {
- HIDEPID_OFF = 0,
- HIDEPID_NO_ACCESS = 1,
- HIDEPID_INVISIBLE = 2,
- HIDEPID_NOT_PTRACABLE = 4, /* Limit pids to only ptracable pids */
-};
-
/* definitions for proc mount option pidonly */
enum {
PROC_PIDONLY_OFF = 0,
diff --git a/include/uapi/linux/proc_fs.h b/include/uapi/linux/proc_fs.h
new file mode 100644
index 000000000000..1e3374efffe2
--- /dev/null
+++ b/include/uapi/linux/proc_fs.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_PROC_FS_H
+#define _UAPI_PROC_FS_H
+
+/* definitions for hide_pid field */
+enum {
+ HIDEPID_OFF = 0,
+ HIDEPID_NO_ACCESS = 1,
+ HIDEPID_INVISIBLE = 2,
+ HIDEPID_NOT_PTRACABLE = 4,
+};
+
+#endif
--
2.24.1
^ permalink raw reply related
* Re: [PATCH v7 07/11] proc: flush task dcache entries from all procfs instances
From: Linus Torvalds @ 2020-01-25 18:45 UTC (permalink / raw)
To: Alexey Gladkov
Cc: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module, Akinobu Mita, Alexander Viro,
Alexey Dobriyan, Andrew Morton, Andy Lutomirski, Daniel Micay,
Djalal Harouni, Dmitry V . Levin, Eric W . Biederman,
Greg Kroah-Hartman, Ingo Molnar, J . Bruce Fields, Jeff Layton
In-Reply-To: <20200125130541.450409-8-gladkov.alexey@gmail.com>
On Sat, Jan 25, 2020 at 5:06 AM Alexey Gladkov <gladkov.alexey@gmail.com> wrote:
>
> This allows to flush dcache entries of a task on multiple procfs mounts
> per pid namespace.
>From a quick read-through, this is the only one I really react negatively to.
The locking looks odd. It only seems to protect the new proc_mounts
list, but then it's a whole big rwsem, and it's taken over all of
proc_flush_task_mnt(), and the locking is exported to all over as a
result of that - including the dummy functions for "there is no proc"
case.
And proc_flush_task_mnt() itself should need no locking over any of
it, so it's all just for the silly looping over the list.
So
(a) this looks fishy and feels wrong - I get a very strong feeling
that the locking is wrong to begin with, and could/should have been
done differently
(b) all the locking should have been internal to /proc, and those
wrappers shouldn't exist in a common header file (and certainly not
for the non-proc case).
Yes, (a) is just a feeling, and I don't have any great suggestions.
Maybe make it an RCU list and use a spinlock for updating it?
But (b) is pretty much a non-starter in this form. Those wrappers
shouldn't be in a globally exported core header file. No way.
Linus
^ permalink raw reply
* Re: [PATCH 3/4] seccomp: Add SECCOMP_USER_NOTIF_FLAG_PIDFD to get pidfd on listener trap
From: Aleksa Sarai @ 2020-01-26 4:03 UTC (permalink / raw)
To: Sargun Dhillon
Cc: linux-kernel, containers, linux-api, linux-fsdevel,
christian.brauner
In-Reply-To: <20200124091743.3357-4-sargun@sargun.me>
[-- Attachment #1: Type: text/plain, Size: 5718 bytes --]
On 2020-01-24, Sargun Dhillon <sargun@sargun.me> wrote:
> This introduces the capability for users of seccomp's listener behaviour
> to be able to receive the pidfd of the process that triggered the event.
> Currently, this just opens the group leader of the thread that triggere
> the event, as pidfds (currently) are limited to group leaders.
>
> For actions which do not act on the process outside of the pidfd, there
> is then no need to check the cookie to ensure validity of the request
> throughout the listener's handling of it.
>
> This can be extended later on as well when pidfd capabilities are added
> to be able to have the listener imbue the pidfd with certain capabilities
> when it is delivered to userspace.
>
> It is the responsibility of the user to close the pidfd.
>
> Signed-off-by: Sargun Dhillon <sargun@sargun.me>
> ---
> include/uapi/linux/seccomp.h | 4 +++
> kernel/seccomp.c | 68 ++++++++++++++++++++++++++++++++----
> 2 files changed, 66 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> index be84d87f1f46..64f6fc5c95f1 100644
> --- a/include/uapi/linux/seccomp.h
> +++ b/include/uapi/linux/seccomp.h
> @@ -69,11 +69,15 @@ struct seccomp_notif_sizes {
> __u16 seccomp_data;
> };
>
> +/* Valid flags for struct seccomp_notif */
> +#define SECCOMP_USER_NOTIF_FLAG_PIDFD (1UL << 0) /* populate pidfd */
> +
> struct seccomp_notif {
> __u64 id;
> __u32 pid;
> __u32 flags;
> struct seccomp_data data;
> + __u32 pidfd;
> };
>
> /*
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index b6ea3dcb57bf..93f9cf45ce07 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -1019,21 +1019,61 @@ static int seccomp_notify_release(struct inode *inode, struct file *file)
> return 0;
> }
>
> +
> +static long __seccomp_notify_recv_pidfd(void __user *buf,
> + struct seccomp_notif *unotif,
> + struct task_struct *group_leader)
> +{
> + struct file *pidfd_file;
> + struct pid *pid;
> + int fd;
> +
> + pid = get_task_pid(group_leader, PIDTYPE_PID);
> + pidfd_file = pidfd_create_file(pid);
> + put_pid(pid);
> + if (IS_ERR(pidfd_file))
> + return PTR_ERR(pidfd_file);
> +
> + fd = get_unused_fd_flags(O_RDWR | O_CLOEXEC);
You don't need to pass O_RDWR -- only O_CLOEXEC is checked by
get_unused_fd_flags().
> + if (fd < 0) {
> + fput(pidfd_file);
> + return fd;
> + }
> +
> + unotif->pidfd = fd;
> +
> + if (copy_to_user(buf, unotif, sizeof(*unotif))) {
> + put_unused_fd(fd);
> + fput(pidfd_file);
> + return -EFAULT;
> + }
> +
> + fd_install(fd, pidfd_file);
> +
> + return 0;
> +}
> +
> static long seccomp_notify_recv(struct seccomp_filter *filter,
> void __user *buf)
> {
> struct seccomp_knotif *knotif = NULL, *cur;
> struct seccomp_notif unotif;
> + struct task_struct *group_leader;
> + bool send_pidfd;
> ssize_t ret;
>
> + if (copy_from_user(&unotif, buf, sizeof(unotif)))
> + return -EFAULT;
> /* Verify that we're not given garbage to keep struct extensible. */
> - ret = check_zeroed_user(buf, sizeof(unotif));
> - if (ret < 0)
> - return ret;
> - if (!ret)
> + if (unotif.id ||
> + unotif.pid ||
> + memchr_inv(&unotif.data, 0, sizeof(unotif.data)) ||
> + unotif.pidfd)
> + return -EINVAL;
IMHO this check is more confusing than the original check_zeroed_user().
Something like the following is simpler and less prone to forgetting to
add a new field in the future:
if (memchr_inv(&unotif, 0, sizeof(unotif)))
return -EINVAL;
> + if (unotif.flags & ~(SECCOMP_USER_NOTIF_FLAG_PIDFD))
> return -EINVAL;
>
> - memset(&unotif, 0, sizeof(unotif));
> + send_pidfd = unotif.flags & SECCOMP_USER_NOTIF_FLAG_PIDFD;
>
> ret = down_interruptible(&filter->notif->request);
> if (ret < 0)
> @@ -1057,9 +1097,13 @@ static long seccomp_notify_recv(struct seccomp_filter *filter,
> goto out;
> }
>
> + memset(&unotif, 0, sizeof(unotif));
> +
> unotif.id = knotif->id;
> unotif.pid = task_pid_vnr(knotif->task);
> unotif.data = *(knotif->data);
> + if (send_pidfd)
> + group_leader = get_task_struct(knotif->task->group_leader);
>
> knotif->state = SECCOMP_NOTIFY_SENT;
> wake_up_poll(&filter->notif->wqh, EPOLLOUT | EPOLLWRNORM);
> @@ -1067,9 +1111,21 @@ static long seccomp_notify_recv(struct seccomp_filter *filter,
> out:
> mutex_unlock(&filter->notify_lock);
>
> - if (ret == 0 && copy_to_user(buf, &unotif, sizeof(unotif))) {
> + if (ret)
> + return ret;
> +
> + /*
> + * We've successfully received a notification, let's try to copy it to
> + * userspace.
> + */
> + if (send_pidfd) {
> + ret = __seccomp_notify_recv_pidfd(buf, &unotif, group_leader);
> + put_task_struct(group_leader);
> + } else if (copy_to_user(buf, &unotif, sizeof(unotif))) {
> ret = -EFAULT;
> + }
To my eye, the way this helper is used is a bit ugly -- my first
question when reading this was "why aren't we doing a copy_to_user() for
pidfds?".
Something like the following might be a bit cleaner I think:
struct file *pidfd_file = NULL;
if (send_pidfd) {
// helper allocates the pidfd_file and sets unotify->fd
ret = __seccomp_notify_recv_pidfd(&unotify, &pidfd_file)
if (ret)
goto err; // or whatever
}
if (copy_to_user(buf, &unotif, sizeof(unotif))) {
ret = -EFAULT;
goto err; // or whatever
}
if (send_pidfd)
fd_install(unotif.fd, pidfd_file)
But to be fair, this is also somewhat ugly too.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] seccomp: Add SECCOMP_USER_NOTIF_FLAG_PIDFD to get pidfd on listener trap
From: Aleksa Sarai @ 2020-01-26 4:10 UTC (permalink / raw)
To: Sargun Dhillon
Cc: Tycho Andersen, LKML, Linux Containers, Linux API,
Linux FS-devel Mailing List, Christian Brauner
In-Reply-To: <CAMp4zn_WXwxJ6Md4rgFzdAY_xea4TmVDdQc1iJDObEMm5Yc79g@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1736 bytes --]
On 2020-01-24, Sargun Dhillon <sargun@sargun.me> wrote:
> On Fri, Jan 24, 2020 at 10:03 AM Tycho Andersen <tycho@tycho.ws> wrote:
> >
> > On Fri, Jan 24, 2020 at 01:17:42AM -0800, Sargun Dhillon wrote:
> > > Currently, this just opens the group leader of the thread that triggere
> > > the event, as pidfds (currently) are limited to group leaders.
> >
> > I don't love the semantics of this; when they're not limited to thread
> > group leaders any more, we won't be able to change this. Is that work
> > far off?
> >
> > Tycho
>
> We would be able to change this in the future if we introduced a flag like
> SECCOMP_USER_NOTIF_FLAG_PIDFD_THREAD which would send a
> pidfd that's for the thread, and not just the group leader. The flag could
> either be XOR with SECCOMP_USER_NOTIF_FLAG_PIDFD, or
> could require both. Alternatively, we can rename
> SECCOMP_USER_NOTIF_FLAG_PIDFD to
> SECCOMP_USER_NOTIF_FLAG_GROUP_LEADER_PIDFD.
Possibly unpopular proposal -- would it make sense to just store the
pidfd_open(2) flags rather than coming up with our own set for
SECCOMP_USER_NOTIF? If/when pidfds are expanded to include non-leaders
there will be a corresponding flag for pidfd_open(2). Something like:
struct seccomp_notif {
__u64 id;
__u32 pid;
__u32 flags;
struct seccomp_data data;
__u64 pidfd_flags; // or __u32 -- not sure what Christian plans
__u32 pidfd;
__u32 __padding;
};
This does mean there'll be an additional flags field, but I think it's a
slightly more consistent way to indicate "SECCOMP_USER_NOTIF_FLAG_PIDFD
implies a pidfd_open(2) on the traced task".
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] seccomp: Add SECCOMP_USER_NOTIF_FLAG_PIDFD to get pidfd on listener trap
From: Aleksa Sarai @ 2020-01-26 4:14 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Sargun Dhillon, linux-fsdevel, linux-api, containers,
linux-kernel, christian.brauner
In-Reply-To: <20200126040325.5eimmm7hli5qcqrr@yavin.dot.cyphar.com>
[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]
On 2020-01-26, Aleksa Sarai <cyphar@cyphar.com> wrote:
> On 2020-01-24, Sargun Dhillon <sargun@sargun.me> wrote:
> > static long seccomp_notify_recv(struct seccomp_filter *filter,
> > void __user *buf)
> > {
> > struct seccomp_knotif *knotif = NULL, *cur;
> > struct seccomp_notif unotif;
> > + struct task_struct *group_leader;
> > + bool send_pidfd;
> > ssize_t ret;
> >
> > + if (copy_from_user(&unotif, buf, sizeof(unotif)))
> > + return -EFAULT;
> > /* Verify that we're not given garbage to keep struct extensible. */
> > - ret = check_zeroed_user(buf, sizeof(unotif));
> > - if (ret < 0)
> > - return ret;
> > - if (!ret)
> > + if (unotif.id ||
> > + unotif.pid ||
> > + memchr_inv(&unotif.data, 0, sizeof(unotif.data)) ||
> > + unotif.pidfd)
> > + return -EINVAL;
>
> IMHO this check is more confusing than the original check_zeroed_user().
> Something like the following is simpler and less prone to forgetting to
> add a new field in the future:
>
> if (memchr_inv(&unotif, 0, sizeof(unotif)))
> return -EINVAL;
Also the check in the patch doesn't ensure that any unnamed padding is
zeroed -- memchr_inv(&unotif, 0, sizeof(unotif)) does.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2] mm: Add MREMAP_DONTUNMAP to mremap().
From: Nathan Chancellor @ 2020-01-26 5:16 UTC (permalink / raw)
To: Brian Geffon
Cc: Andrew Morton, Michael S . Tsirkin, Arnd Bergmann,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, linux-api-u79uwXL29TY76Z2rM5mHXA,
Andy Lutomirski, Andrea Arcangeli, Sonny Rao, Minchan Kim,
Joel Fernandes, Yu Zhao, Jesse Barnes,
clang-built-linux-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20200124190625.257659-1-bgeffon-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Hi Brian,
On Fri, Jan 24, 2020 at 11:06:25AM -0800, Brian Geffon wrote:
> When remapping an anonymous, private mapping, if MREMAP_DONTUNMAP is
> set, the source mapping will not be removed. Instead it will be
> cleared as if a brand new anonymous, private mapping had been created
> atomically as part of the mremap() call. If a userfaultfd was watching
> the source, it will continue to watch the new mapping. For a mapping
> that is shared or not anonymous, MREMAP_DONTUNMAP will cause the
> mremap() call to fail. MREMAP_DONTUNMAP implies that MREMAP_FIXED is
> also used. The final result is two equally sized VMAs where the
> destination contains the PTEs of the source.
>
> We hope to use this in Chrome OS where with userfaultfd we could write
> an anonymous mapping to disk without having to STOP the process or worry
> about VMA permission changes.
>
> This feature also has a use case in Android, Lokesh Gidra has said
> that "As part of using userfaultfd for GC, We'll have to move the physical
> pages of the java heap to a separate location. For this purpose mremap
> will be used. Without the MREMAP_DONTUNMAP flag, when I mremap the java
> heap, its virtual mapping will be removed as well. Therefore, we'll
> require performing mmap immediately after. This is not only time consuming
> but also opens a time window where a native thread may call mmap and
> reserve the java heap's address range for its own usage. This flag
> solves the problem."
>
> Signed-off-by: Brian Geffon <bgeffon-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---
> include/uapi/linux/mman.h | 5 +++--
> mm/mremap.c | 37 ++++++++++++++++++++++++++++++-------
> 2 files changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/include/uapi/linux/mman.h b/include/uapi/linux/mman.h
> index fc1a64c3447b..923cc162609c 100644
> --- a/include/uapi/linux/mman.h
> +++ b/include/uapi/linux/mman.h
> @@ -5,8 +5,9 @@
> #include <asm/mman.h>
> #include <asm-generic/hugetlb_encode.h>
>
> -#define MREMAP_MAYMOVE 1
> -#define MREMAP_FIXED 2
> +#define MREMAP_MAYMOVE 1
> +#define MREMAP_FIXED 2
> +#define MREMAP_DONTUNMAP 4
>
> #define OVERCOMMIT_GUESS 0
> #define OVERCOMMIT_ALWAYS 1
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 122938dcec15..bf97c3eb538b 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -318,8 +318,8 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
> static unsigned long move_vma(struct vm_area_struct *vma,
> unsigned long old_addr, unsigned long old_len,
> unsigned long new_len, unsigned long new_addr,
> - bool *locked, struct vm_userfaultfd_ctx *uf,
> - struct list_head *uf_unmap)
> + bool *locked, unsigned long flags,
> + struct vm_userfaultfd_ctx *uf, struct list_head *uf_unmap)
> {
> struct mm_struct *mm = vma->vm_mm;
> struct vm_area_struct *new_vma;
> @@ -408,6 +408,13 @@ static unsigned long move_vma(struct vm_area_struct *vma,
> if (unlikely(vma->vm_flags & VM_PFNMAP))
> untrack_pfn_moved(vma);
>
> + if (unlikely(!err && (flags & MREMAP_DONTUNMAP))) {
> + if (vm_flags & VM_ACCOUNT)
> + vma->vm_flags |= VM_ACCOUNT;
> +
> + goto out;
> + }
> +
> if (do_munmap(mm, old_addr, old_len, uf_unmap) < 0) {
> /* OOM: unable to split vma, just get accounts right */
> vm_unacct_memory(excess >> PAGE_SHIFT);
> @@ -422,6 +429,7 @@ static unsigned long move_vma(struct vm_area_struct *vma,
> vma->vm_next->vm_flags |= VM_ACCOUNT;
> }
>
> +out:
> if (vm_flags & VM_LOCKED) {
> mm->locked_vm += new_len >> PAGE_SHIFT;
> *locked = true;
> @@ -497,7 +505,7 @@ static struct vm_area_struct *vma_to_resize(unsigned long addr,
>
> static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
> unsigned long new_addr, unsigned long new_len, bool *locked,
> - struct vm_userfaultfd_ctx *uf,
> + unsigned long flags, struct vm_userfaultfd_ctx *uf,
> struct list_head *uf_unmap_early,
> struct list_head *uf_unmap)
> {
> @@ -545,6 +553,17 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
> old_len = new_len;
> }
>
> + /*
> + * MREMAP_DONTUNMAP expands by old_len + (new_len - old_len), we will
> + * check that we can expand by old_len and vma_to_resize will handle
> + * the vma growing.
> + */
> + if (unlikely(flags & MREMAP_DONTUNMAP && !may_expand_vm(mm,
> + vma->vm_flags, old_len >> PAGE_SHIFT))) {
We received a Clang build report that vma is used uninitialized here
(they aren't being publicly sent to LKML due to GCC vs Clang
warning/error overlap):
https://groups.google.com/d/msg/clang-built-linux/gE5wRaeHdSI/xVA0MBQVEgAJ
Sure enough, vma is initialized first in the next block. Not sure if
this section should be moved below that initialization or if something
else should be done to resolve it but that dereference will obviously be
fatal.
Cheers,
Nathan
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> vma = vma_to_resize(addr, old_len, new_len, &charged);
> if (IS_ERR(vma)) {
> ret = PTR_ERR(vma);
> @@ -561,7 +580,7 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
> if (IS_ERR_VALUE(ret))
> goto out1;
>
> - ret = move_vma(vma, addr, old_len, new_len, new_addr, locked, uf,
> + ret = move_vma(vma, addr, old_len, new_len, new_addr, locked, flags, uf,
> uf_unmap);
> if (!(offset_in_page(ret)))
> goto out;
> @@ -609,12 +628,15 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
> addr = untagged_addr(addr);
> new_addr = untagged_addr(new_addr);
>
> - if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE))
> + if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE | MREMAP_DONTUNMAP))
> return ret;
>
> if (flags & MREMAP_FIXED && !(flags & MREMAP_MAYMOVE))
> return ret;
>
> + if (flags & MREMAP_DONTUNMAP && !(flags & MREMAP_FIXED))
> + return ret;
> +
> if (offset_in_page(addr))
> return ret;
>
> @@ -634,7 +656,8 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
>
> if (flags & MREMAP_FIXED) {
> ret = mremap_to(addr, old_len, new_addr, new_len,
> - &locked, &uf, &uf_unmap_early, &uf_unmap);
> + &locked, flags, &uf, &uf_unmap_early,
> + &uf_unmap);
> goto out;
> }
>
> @@ -712,7 +735,7 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
> }
>
> ret = move_vma(vma, addr, old_len, new_len, new_addr,
> - &locked, &uf, &uf_unmap);
> + &locked, flags, &uf, &uf_unmap);
> }
> out:
> if (offset_in_page(ret)) {
> --
> 2.25.0.341.g760bfbb309-goog
>
^ permalink raw reply
* Re: [PATCH 3/4] seccomp: Add SECCOMP_USER_NOTIF_FLAG_PIDFD to get pidfd on listener trap
From: Tycho Andersen @ 2020-01-26 5:42 UTC (permalink / raw)
To: Sargun Dhillon
Cc: LKML, Linux Containers, Linux API, Linux FS-devel Mailing List,
Christian Brauner
In-Reply-To: <CAMp4zn_WXwxJ6Md4rgFzdAY_xea4TmVDdQc1iJDObEMm5Yc79g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Jan 24, 2020 at 12:09:37PM -0800, Sargun Dhillon wrote:
> On Fri, Jan 24, 2020 at 10:03 AM Tycho Andersen <tycho-E0fblnxP3wo@public.gmane.org> wrote:
> >
> > On Fri, Jan 24, 2020 at 01:17:42AM -0800, Sargun Dhillon wrote:
> > > Currently, this just opens the group leader of the thread that triggere
> > > the event, as pidfds (currently) are limited to group leaders.
> >
> > I don't love the semantics of this; when they're not limited to thread
> > group leaders any more, we won't be able to change this. Is that work
> > far off?
> >
> > Tycho
>
> We would be able to change this in the future if we introduced a flag like
> SECCOMP_USER_NOTIF_FLAG_PIDFD_THREAD which would send a
> pidfd that's for the thread, and not just the group leader. The flag could
> either be XOR with SECCOMP_USER_NOTIF_FLAG_PIDFD, or
> could require both. Alternatively, we can rename
> SECCOMP_USER_NOTIF_FLAG_PIDFD to
> SECCOMP_USER_NOTIF_FLAG_GROUP_LEADER_PIDFD.
Ok, but then isn't this just another temporary API? Seems like it's
worth waiting until the Right Way exists.
Tycho
^ permalink raw reply
* Re: [RFC PATCH 07/14] Add sample notification program [ver #3]
From: Guenter Roeck @ 2020-01-26 15:47 UTC (permalink / raw)
To: David Howells
Cc: torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Greg Kroah-Hartman,
Casey Schaufler, Stephen Smalley,
nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
raven-PKsaG3nR2I+sTnJN9+BGXg, Christian Brauner,
keyrings-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-block-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <157909509882.20155.1159021562184142124.stgit-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>
On Wed, Jan 15, 2020 at 01:31:38PM +0000, David Howells wrote:
> The sample program is run like:
>
> ./samples/watch_queue/watch_test
>
> and watches "/" for mount changes and the current session keyring for key
> changes:
>
> # keyctl add user a a @s
> 1035096409
> # keyctl unlink 1035096409 @s
>
> producing:
>
> # ./watch_test
> read() = 16
> NOTIFY[000]: ty=000001 sy=02 i=00000110
> KEY 2ffc2e5d change=2[linked] aux=1035096409
> read() = 16
> NOTIFY[000]: ty=000001 sy=02 i=00000110
> KEY 2ffc2e5d change=3[unlinked] aux=1035096409
>
> Other events may be produced, such as with a failing disk:
>
> read() = 22
> NOTIFY[000]: ty=000003 sy=02 i=00000416
> USB 3-7.7 dev-reset e=0 r=0
> read() = 24
> NOTIFY[000]: ty=000002 sy=06 i=00000418
> BLOCK 00800050 e=6[critical medium] s=64000ef8
>
> This corresponds to:
>
> blk_update_request: critical medium error, dev sdf, sector 1677725432 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0
>
> in dmesg.
>
> Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
mips:allmodconfig:
samples/watch_queue/watch_test.c: In function ‘keyctl_watch_key’:
samples/watch_queue/watch_test.c:34:17: error: ‘__NR_keyctl’ undeclared
Guenter
^ permalink raw reply
* Re: [PATCH v2] mm: Add MREMAP_DONTUNMAP to mremap().
From: Kirill A. Shutemov @ 2020-01-26 22:06 UTC (permalink / raw)
To: Brian Geffon
Cc: Andrew Morton, Michael S . Tsirkin, Arnd Bergmann,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, linux-api-u79uwXL29TY76Z2rM5mHXA,
Andy Lutomirski, Andrea Arcangeli, Sonny Rao, Minchan Kim,
Joel Fernandes, Yu Zhao, Jesse Barnes
In-Reply-To: <20200124190625.257659-1-bgeffon-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
On Fri, Jan 24, 2020 at 11:06:25AM -0800, Brian Geffon wrote:
> When remapping an anonymous, private mapping, if MREMAP_DONTUNMAP is
> set, the source mapping will not be removed. Instead it will be
> cleared as if a brand new anonymous, private mapping had been created
> atomically as part of the mremap() call. If a userfaultfd was watching
> the source, it will continue to watch the new mapping. For a mapping
> that is shared or not anonymous, MREMAP_DONTUNMAP will cause the
> mremap() call to fail. MREMAP_DONTUNMAP implies that MREMAP_FIXED is
> also used.
Implies? From code it looks like it requires MREMAP_FIXED. And
MREMAP_FIXED requires MREMAP_MAYMOVE. That's strange flag chaining.
I don't really see need in such dependencies. It maybe indeed implied, not
requied.
> The final result is two equally sized VMAs where the
> destination contains the PTEs of the source.
Could you clarify rmap situation here? How the rmap hierarchy will look
like before and after the operation. Can the new VMA merge with the old
one? Sounds fishy to me.
> We hope to use this in Chrome OS where with userfaultfd we could write
> an anonymous mapping to disk without having to STOP the process or worry
> about VMA permission changes.
>
> This feature also has a use case in Android, Lokesh Gidra has said
> that "As part of using userfaultfd for GC, We'll have to move the physical
> pages of the java heap to a separate location. For this purpose mremap
> will be used. Without the MREMAP_DONTUNMAP flag, when I mremap the java
> heap, its virtual mapping will be removed as well. Therefore, we'll
> require performing mmap immediately after. This is not only time consuming
> but also opens a time window where a native thread may call mmap and
> reserve the java heap's address range for its own usage. This flag
> solves the problem."
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v2] mm: Add MREMAP_DONTUNMAP to mremap().
From: Brian Geffon @ 2020-01-27 2:21 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Andrew Morton, Michael S . Tsirkin, Arnd Bergmann, LKML, linux-mm,
linux-api-u79uwXL29TY76Z2rM5mHXA, Andy Lutomirski,
Andrea Arcangeli, Sonny Rao, Minchan Kim, Joel Fernandes, Yu Zhao,
Jesse Barnes, clang-built-linux-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20200126051642.GA39508@ubuntu-x2-xlarge-x86>
Hi Nathan,
Thank you! That was an oversight on my part. I'll address it in the next patch.
Brian
On Sat, Jan 25, 2020 at 9:16 PM Nathan Chancellor
<natechancellor-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> Hi Brian,
>
> On Fri, Jan 24, 2020 at 11:06:25AM -0800, Brian Geffon wrote:
> > When remapping an anonymous, private mapping, if MREMAP_DONTUNMAP is
> > set, the source mapping will not be removed. Instead it will be
> > cleared as if a brand new anonymous, private mapping had been created
> > atomically as part of the mremap() call. If a userfaultfd was watching
> > the source, it will continue to watch the new mapping. For a mapping
> > that is shared or not anonymous, MREMAP_DONTUNMAP will cause the
> > mremap() call to fail. MREMAP_DONTUNMAP implies that MREMAP_FIXED is
> > also used. The final result is two equally sized VMAs where the
> > destination contains the PTEs of the source.
> >
> > We hope to use this in Chrome OS where with userfaultfd we could write
> > an anonymous mapping to disk without having to STOP the process or worry
> > about VMA permission changes.
> >
> > This feature also has a use case in Android, Lokesh Gidra has said
> > that "As part of using userfaultfd for GC, We'll have to move the physical
> > pages of the java heap to a separate location. For this purpose mremap
> > will be used. Without the MREMAP_DONTUNMAP flag, when I mremap the java
> > heap, its virtual mapping will be removed as well. Therefore, we'll
> > require performing mmap immediately after. This is not only time consuming
> > but also opens a time window where a native thread may call mmap and
> > reserve the java heap's address range for its own usage. This flag
> > solves the problem."
> >
> > Signed-off-by: Brian Geffon <bgeffon-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> > ---
> > include/uapi/linux/mman.h | 5 +++--
> > mm/mremap.c | 37 ++++++++++++++++++++++++++++++-------
> > 2 files changed, 33 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/uapi/linux/mman.h b/include/uapi/linux/mman.h
> > index fc1a64c3447b..923cc162609c 100644
> > --- a/include/uapi/linux/mman.h
> > +++ b/include/uapi/linux/mman.h
> > @@ -5,8 +5,9 @@
> > #include <asm/mman.h>
> > #include <asm-generic/hugetlb_encode.h>
> >
> > -#define MREMAP_MAYMOVE 1
> > -#define MREMAP_FIXED 2
> > +#define MREMAP_MAYMOVE 1
> > +#define MREMAP_FIXED 2
> > +#define MREMAP_DONTUNMAP 4
> >
> > #define OVERCOMMIT_GUESS 0
> > #define OVERCOMMIT_ALWAYS 1
> > diff --git a/mm/mremap.c b/mm/mremap.c
> > index 122938dcec15..bf97c3eb538b 100644
> > --- a/mm/mremap.c
> > +++ b/mm/mremap.c
> > @@ -318,8 +318,8 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
> > static unsigned long move_vma(struct vm_area_struct *vma,
> > unsigned long old_addr, unsigned long old_len,
> > unsigned long new_len, unsigned long new_addr,
> > - bool *locked, struct vm_userfaultfd_ctx *uf,
> > - struct list_head *uf_unmap)
> > + bool *locked, unsigned long flags,
> > + struct vm_userfaultfd_ctx *uf, struct list_head *uf_unmap)
> > {
> > struct mm_struct *mm = vma->vm_mm;
> > struct vm_area_struct *new_vma;
> > @@ -408,6 +408,13 @@ static unsigned long move_vma(struct vm_area_struct *vma,
> > if (unlikely(vma->vm_flags & VM_PFNMAP))
> > untrack_pfn_moved(vma);
> >
> > + if (unlikely(!err && (flags & MREMAP_DONTUNMAP))) {
> > + if (vm_flags & VM_ACCOUNT)
> > + vma->vm_flags |= VM_ACCOUNT;
> > +
> > + goto out;
> > + }
> > +
> > if (do_munmap(mm, old_addr, old_len, uf_unmap) < 0) {
> > /* OOM: unable to split vma, just get accounts right */
> > vm_unacct_memory(excess >> PAGE_SHIFT);
> > @@ -422,6 +429,7 @@ static unsigned long move_vma(struct vm_area_struct *vma,
> > vma->vm_next->vm_flags |= VM_ACCOUNT;
> > }
> >
> > +out:
> > if (vm_flags & VM_LOCKED) {
> > mm->locked_vm += new_len >> PAGE_SHIFT;
> > *locked = true;
> > @@ -497,7 +505,7 @@ static struct vm_area_struct *vma_to_resize(unsigned long addr,
> >
> > static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
> > unsigned long new_addr, unsigned long new_len, bool *locked,
> > - struct vm_userfaultfd_ctx *uf,
> > + unsigned long flags, struct vm_userfaultfd_ctx *uf,
> > struct list_head *uf_unmap_early,
> > struct list_head *uf_unmap)
> > {
> > @@ -545,6 +553,17 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
> > old_len = new_len;
> > }
> >
> > + /*
> > + * MREMAP_DONTUNMAP expands by old_len + (new_len - old_len), we will
> > + * check that we can expand by old_len and vma_to_resize will handle
> > + * the vma growing.
> > + */
> > + if (unlikely(flags & MREMAP_DONTUNMAP && !may_expand_vm(mm,
> > + vma->vm_flags, old_len >> PAGE_SHIFT))) {
>
> We received a Clang build report that vma is used uninitialized here
> (they aren't being publicly sent to LKML due to GCC vs Clang
> warning/error overlap):
>
> https://groups.google.com/d/msg/clang-built-linux/gE5wRaeHdSI/xVA0MBQVEgAJ
>
> Sure enough, vma is initialized first in the next block. Not sure if
> this section should be moved below that initialization or if something
> else should be done to resolve it but that dereference will obviously be
> fatal.
>
> Cheers,
> Nathan
>
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > vma = vma_to_resize(addr, old_len, new_len, &charged);
> > if (IS_ERR(vma)) {
> > ret = PTR_ERR(vma);
> > @@ -561,7 +580,7 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
> > if (IS_ERR_VALUE(ret))
> > goto out1;
> >
> > - ret = move_vma(vma, addr, old_len, new_len, new_addr, locked, uf,
> > + ret = move_vma(vma, addr, old_len, new_len, new_addr, locked, flags, uf,
> > uf_unmap);
> > if (!(offset_in_page(ret)))
> > goto out;
> > @@ -609,12 +628,15 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
> > addr = untagged_addr(addr);
> > new_addr = untagged_addr(new_addr);
> >
> > - if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE))
> > + if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE | MREMAP_DONTUNMAP))
> > return ret;
> >
> > if (flags & MREMAP_FIXED && !(flags & MREMAP_MAYMOVE))
> > return ret;
> >
> > + if (flags & MREMAP_DONTUNMAP && !(flags & MREMAP_FIXED))
> > + return ret;
> > +
> > if (offset_in_page(addr))
> > return ret;
> >
> > @@ -634,7 +656,8 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
> >
> > if (flags & MREMAP_FIXED) {
> > ret = mremap_to(addr, old_len, new_addr, new_len,
> > - &locked, &uf, &uf_unmap_early, &uf_unmap);
> > + &locked, flags, &uf, &uf_unmap_early,
> > + &uf_unmap);
> > goto out;
> > }
> >
> > @@ -712,7 +735,7 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
> > }
> >
> > ret = move_vma(vma, addr, old_len, new_len, new_addr,
> > - &locked, &uf, &uf_unmap);
> > + &locked, flags, &uf, &uf_unmap);
> > }
> > out:
> > if (offset_in_page(ret)) {
> > --
> > 2.25.0.341.g760bfbb309-goog
> >
^ permalink raw reply
* Re: [PATCH] mm: Add MREMAP_DONTUNMAP to mremap().
From: Dan Carpenter @ 2020-01-27 4:46 UTC (permalink / raw)
To: kbuild
Cc: kbuild-all, linux-mm, Andrew Morton, Michael S . Tsirkin,
Arnd Bergmann, Brian Geffon, Sonny Rao, Minchan Kim,
Joel Fernandes, Lokesh Gidra, linux-kernel, linux-api, Yu Zhao,
Jesse Barnes
In-Reply-To: <20200123014627.71720-1-bgeffon@google.com>
Hi Brian,
url: https://github.com/0day-ci/linux/commits/Brian-Geffon/mm-Add-MREMAP_DONTUNMAP-to-mremap/20200125-013342
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 4703d9119972bf586d2cca76ec6438f819ffa30e
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
mm/mremap.c:561 mremap_to() error: potentially dereferencing uninitialized 'vma'.
# https://github.com/0day-ci/linux/commit/98663ca05501623c3da7f0f30be8ba7d632cf010
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 98663ca05501623c3da7f0f30be8ba7d632cf010
vim +/vma +561 mm/mremap.c
81909b842107ef Michel Lespinasse 2013-02-22 506 static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
72f87654c69690 Pavel Emelyanov 2017-02-22 507 unsigned long new_addr, unsigned long new_len, bool *locked,
98663ca0550162 Brian Geffon 2020-01-22 508 unsigned long flags, struct vm_userfaultfd_ctx *uf,
b22823719302e8 Mike Rapoport 2017-08-02 509 struct list_head *uf_unmap_early,
897ab3e0c49e24 Mike Rapoport 2017-02-24 510 struct list_head *uf_unmap)
ecc1a8993751de Al Viro 2009-11-24 511 {
ecc1a8993751de Al Viro 2009-11-24 512 struct mm_struct *mm = current->mm;
ecc1a8993751de Al Viro 2009-11-24 513 struct vm_area_struct *vma;
ecc1a8993751de Al Viro 2009-11-24 514 unsigned long ret = -EINVAL;
ecc1a8993751de Al Viro 2009-11-24 515 unsigned long charged = 0;
097eed103862f9 Al Viro 2009-11-24 516 unsigned long map_flags;
ecc1a8993751de Al Viro 2009-11-24 517
f19cb115a25f3f Alexander Kuleshov 2015-11-05 518 if (offset_in_page(new_addr))
ecc1a8993751de Al Viro 2009-11-24 519 goto out;
ecc1a8993751de Al Viro 2009-11-24 520
ecc1a8993751de Al Viro 2009-11-24 521 if (new_len > TASK_SIZE || new_addr > TASK_SIZE - new_len)
ecc1a8993751de Al Viro 2009-11-24 522 goto out;
ecc1a8993751de Al Viro 2009-11-24 523
9943242ca46814 Oleg Nesterov 2015-09-04 524 /* Ensure the old/new locations do not overlap */
9943242ca46814 Oleg Nesterov 2015-09-04 525 if (addr + old_len > new_addr && new_addr + new_len > addr)
ecc1a8993751de Al Viro 2009-11-24 526 goto out;
ecc1a8993751de Al Viro 2009-11-24 527
ea2c3f6f554561 Oscar Salvador 2019-03-05 528 /*
ea2c3f6f554561 Oscar Salvador 2019-03-05 529 * move_vma() need us to stay 4 maps below the threshold, otherwise
ea2c3f6f554561 Oscar Salvador 2019-03-05 530 * it will bail out at the very beginning.
ea2c3f6f554561 Oscar Salvador 2019-03-05 531 * That is a problem if we have already unmaped the regions here
ea2c3f6f554561 Oscar Salvador 2019-03-05 532 * (new_addr, and old_addr), because userspace will not know the
ea2c3f6f554561 Oscar Salvador 2019-03-05 533 * state of the vma's after it gets -ENOMEM.
ea2c3f6f554561 Oscar Salvador 2019-03-05 534 * So, to avoid such scenario we can pre-compute if the whole
ea2c3f6f554561 Oscar Salvador 2019-03-05 535 * operation has high chances to success map-wise.
ea2c3f6f554561 Oscar Salvador 2019-03-05 536 * Worst-scenario case is when both vma's (new_addr and old_addr) get
ea2c3f6f554561 Oscar Salvador 2019-03-05 537 * split in 3 before unmaping it.
ea2c3f6f554561 Oscar Salvador 2019-03-05 538 * That means 2 more maps (1 for each) to the ones we already hold.
ea2c3f6f554561 Oscar Salvador 2019-03-05 539 * Check whether current map count plus 2 still leads us to 4 maps below
ea2c3f6f554561 Oscar Salvador 2019-03-05 540 * the threshold, otherwise return -ENOMEM here to be more safe.
ea2c3f6f554561 Oscar Salvador 2019-03-05 541 */
ea2c3f6f554561 Oscar Salvador 2019-03-05 542 if ((mm->map_count + 2) >= sysctl_max_map_count - 3)
ea2c3f6f554561 Oscar Salvador 2019-03-05 543 return -ENOMEM;
ea2c3f6f554561 Oscar Salvador 2019-03-05 544
b22823719302e8 Mike Rapoport 2017-08-02 545 ret = do_munmap(mm, new_addr, new_len, uf_unmap_early);
ecc1a8993751de Al Viro 2009-11-24 546 if (ret)
ecc1a8993751de Al Viro 2009-11-24 547 goto out;
ecc1a8993751de Al Viro 2009-11-24 548
ecc1a8993751de Al Viro 2009-11-24 549 if (old_len >= new_len) {
897ab3e0c49e24 Mike Rapoport 2017-02-24 550 ret = do_munmap(mm, addr+new_len, old_len - new_len, uf_unmap);
ecc1a8993751de Al Viro 2009-11-24 551 if (ret && old_len != new_len)
ecc1a8993751de Al Viro 2009-11-24 552 goto out;
ecc1a8993751de Al Viro 2009-11-24 553 old_len = new_len;
ecc1a8993751de Al Viro 2009-11-24 554 }
ecc1a8993751de Al Viro 2009-11-24 555
98663ca0550162 Brian Geffon 2020-01-22 556 /*
98663ca0550162 Brian Geffon 2020-01-22 557 * MREMAP_DONTUNMAP expands by old_len + (new_len - old_len), we will
98663ca0550162 Brian Geffon 2020-01-22 558 * check that we can expand by old_len and vma_to_resize will handle
98663ca0550162 Brian Geffon 2020-01-22 559 * the vma growing.
98663ca0550162 Brian Geffon 2020-01-22 560 */
98663ca0550162 Brian Geffon 2020-01-22 @561 if (unlikely(flags & MREMAP_DONTUNMAP && !may_expand_vm(mm,
98663ca0550162 Brian Geffon 2020-01-22 562 vma->vm_flags, old_len >> PAGE_SHIFT))) {
^^^^^^^^^^^^^
98663ca0550162 Brian Geffon 2020-01-22 563 ret = -ENOMEM;
98663ca0550162 Brian Geffon 2020-01-22 564 goto out;
98663ca0550162 Brian Geffon 2020-01-22 565 }
98663ca0550162 Brian Geffon 2020-01-22 566
ecc1a8993751de Al Viro 2009-11-24 567 vma = vma_to_resize(addr, old_len, new_len, &charged);
^^^^^^^^^^^^^^^^^^^^
ecc1a8993751de Al Viro 2009-11-24 568 if (IS_ERR(vma)) {
ecc1a8993751de Al Viro 2009-11-24 569 ret = PTR_ERR(vma);
ecc1a8993751de Al Viro 2009-11-24 570 goto out;
ecc1a8993751de Al Viro 2009-11-24 571 }
ecc1a8993751de Al Viro 2009-11-24 572
097eed103862f9 Al Viro 2009-11-24 573 map_flags = MAP_FIXED;
^ permalink raw reply
* Re: [PATCH 3/4] seccomp: Add SECCOMP_USER_NOTIF_FLAG_PIDFD to get pidfd on listener trap
From: Sargun Dhillon @ 2020-01-27 5:06 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Aleksa Sarai, linux-fsdevel, linux-api, containers, linux-kernel,
christian.brauner
In-Reply-To: <20200126041439.liwfmb4h74zmhi76@yavin.dot.cyphar.com>
On Sun, Jan 26, 2020 at 03:14:39PM +1100, Aleksa Sarai wrote:
> On 2020-01-26, Aleksa Sarai <cyphar@cyphar.com> wrote:
> > On 2020-01-24, Sargun Dhillon <sargun@sargun.me> wrote:
> > > static long seccomp_notify_recv(struct seccomp_filter *filter,
> > > void __user *buf)
> > > {
> > > struct seccomp_knotif *knotif = NULL, *cur;
> > > struct seccomp_notif unotif;
> > > + struct task_struct *group_leader;
> > > + bool send_pidfd;
> > > ssize_t ret;
> > >
> > > + if (copy_from_user(&unotif, buf, sizeof(unotif)))
> > > + return -EFAULT;
> > > /* Verify that we're not given garbage to keep struct extensible. */
> > > - ret = check_zeroed_user(buf, sizeof(unotif));
> > > - if (ret < 0)
> > > - return ret;
> > > - if (!ret)
> > > + if (unotif.id ||
> > > + unotif.pid ||
> > > + memchr_inv(&unotif.data, 0, sizeof(unotif.data)) ||
> > > + unotif.pidfd)
> > > + return -EINVAL;
> >
> > IMHO this check is more confusing than the original check_zeroed_user().
> > Something like the following is simpler and less prone to forgetting to
> > add a new field in the future:
> >
I'm all for this, originally my patch read:
__u32 flags = 0;
swap(unotif.flags, flags);
if (memchr(&unotif, 0, sizeof(unotif))
return -EINVAL;
--- And then check flags appropriately. I'm not sure if this is "better",
as I didn't see any other implementations that look like this in the
kernel. What do you think? It could even look "simpler", as in:
__u32 flags;
if (copy_from_user(....))
return -EFAULT;
flags = unotif.flags;
unotif.flags = 0;
if (memchr_inv(&unotif, 0, sizeof(unotif)))
return -EINVAL;
Are either of those preferential, reasonable, or at a minimum inoffensive?
> > if (memchr_inv(&unotif, 0, sizeof(unotif)))
> > return -EINVAL;
>
Wouldn't this fail if flags was set to any value? We either need to zero
out flags prior to checking, or split it into range checks that exclude
flags.
> Also the check in the patch doesn't ensure that any unnamed padding is
> zeroed -- memchr_inv(&unotif, 0, sizeof(unotif)) does.
>
> --
> Aleksa Sarai
> Senior Software Engineer (Containers)
> SUSE Linux GmbH
> <https://www.cyphar.com/>
^ permalink raw reply
* [PATCH v3] mm: Add MREMAP_DONTUNMAP to mremap().
From: Brian Geffon @ 2020-01-27 5:30 UTC (permalink / raw)
To: Andrew Morton
Cc: Michael S . Tsirkin, Brian Geffon, Arnd Bergmann,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, linux-api-u79uwXL29TY76Z2rM5mHXA,
Andy Lutomirski, Andrea Arcangeli, Sonny Rao, Minchan Kim,
Joel Fernandes, Yu Zhao, Jesse Barnes, Nathan Chancellor
In-Reply-To: <20200123014627.71720-1-bgeffon-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
When remapping an anonymous, private mapping, if MREMAP_DONTUNMAP is
set, the source mapping will not be removed. Instead it will be
cleared as if a brand new anonymous, private mapping had been created
atomically as part of the mremap() call. If a userfaultfd was watching
the source, it will continue to watch the new mapping. For a mapping
that is shared or not anonymous, MREMAP_DONTUNMAP will cause the
mremap() call to fail. MREMAP_DONTUNMAP requires that MREMAP_FIXED is
also used. The final result is two equally sized VMAs where the
destination contains the PTEs of the source.
We hope to use this in Chrome OS where with userfaultfd we could write
an anonymous mapping to disk without having to STOP the process or worry
about VMA permission changes.
This feature also has a use case in Android, Lokesh Gidra has said
that "As part of using userfaultfd for GC, We'll have to move the physical
pages of the java heap to a separate location. For this purpose mremap
will be used. Without the MREMAP_DONTUNMAP flag, when I mremap the java
heap, its virtual mapping will be removed as well. Therefore, we'll
require performing mmap immediately after. This is not only time consuming
but also opens a time window where a native thread may call mmap and
reserve the java heap's address range for its own usage. This flag
solves the problem."
Signed-off-by: Brian Geffon <bgeffon-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
include/uapi/linux/mman.h | 5 +++--
mm/mremap.c | 38 +++++++++++++++++++++++++++++++-------
2 files changed, 34 insertions(+), 9 deletions(-)
diff --git a/include/uapi/linux/mman.h b/include/uapi/linux/mman.h
index fc1a64c3447b..923cc162609c 100644
--- a/include/uapi/linux/mman.h
+++ b/include/uapi/linux/mman.h
@@ -5,8 +5,9 @@
#include <asm/mman.h>
#include <asm-generic/hugetlb_encode.h>
-#define MREMAP_MAYMOVE 1
-#define MREMAP_FIXED 2
+#define MREMAP_MAYMOVE 1
+#define MREMAP_FIXED 2
+#define MREMAP_DONTUNMAP 4
#define OVERCOMMIT_GUESS 0
#define OVERCOMMIT_ALWAYS 1
diff --git a/mm/mremap.c b/mm/mremap.c
index 122938dcec15..1d164e5fdff0 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -318,8 +318,8 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
static unsigned long move_vma(struct vm_area_struct *vma,
unsigned long old_addr, unsigned long old_len,
unsigned long new_len, unsigned long new_addr,
- bool *locked, struct vm_userfaultfd_ctx *uf,
- struct list_head *uf_unmap)
+ bool *locked, unsigned long flags,
+ struct vm_userfaultfd_ctx *uf, struct list_head *uf_unmap)
{
struct mm_struct *mm = vma->vm_mm;
struct vm_area_struct *new_vma;
@@ -408,6 +408,13 @@ static unsigned long move_vma(struct vm_area_struct *vma,
if (unlikely(vma->vm_flags & VM_PFNMAP))
untrack_pfn_moved(vma);
+ if (unlikely(!err && (flags & MREMAP_DONTUNMAP))) {
+ if (vm_flags & VM_ACCOUNT)
+ vma->vm_flags |= VM_ACCOUNT;
+
+ goto out;
+ }
+
if (do_munmap(mm, old_addr, old_len, uf_unmap) < 0) {
/* OOM: unable to split vma, just get accounts right */
vm_unacct_memory(excess >> PAGE_SHIFT);
@@ -422,6 +429,7 @@ static unsigned long move_vma(struct vm_area_struct *vma,
vma->vm_next->vm_flags |= VM_ACCOUNT;
}
+out:
if (vm_flags & VM_LOCKED) {
mm->locked_vm += new_len >> PAGE_SHIFT;
*locked = true;
@@ -497,7 +505,7 @@ static struct vm_area_struct *vma_to_resize(unsigned long addr,
static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
unsigned long new_addr, unsigned long new_len, bool *locked,
- struct vm_userfaultfd_ctx *uf,
+ unsigned long flags, struct vm_userfaultfd_ctx *uf,
struct list_head *uf_unmap_early,
struct list_head *uf_unmap)
{
@@ -551,6 +559,17 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
goto out;
}
+ /*
+ * MREMAP_DONTUNMAP expands by old_len + (new_len - old_len), we will
+ * check that we can expand by old_len and vma_to_resize will handle
+ * the vma growing.
+ */
+ if (unlikely(flags & MREMAP_DONTUNMAP && !may_expand_vm(mm,
+ vma->vm_flags, old_len >> PAGE_SHIFT))) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
map_flags = MAP_FIXED;
if (vma->vm_flags & VM_MAYSHARE)
map_flags |= MAP_SHARED;
@@ -561,7 +580,7 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
if (IS_ERR_VALUE(ret))
goto out1;
- ret = move_vma(vma, addr, old_len, new_len, new_addr, locked, uf,
+ ret = move_vma(vma, addr, old_len, new_len, new_addr, locked, flags, uf,
uf_unmap);
if (!(offset_in_page(ret)))
goto out;
@@ -609,12 +628,16 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
addr = untagged_addr(addr);
new_addr = untagged_addr(new_addr);
- if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE))
+ if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE | MREMAP_DONTUNMAP)) {
return ret;
+ }
if (flags & MREMAP_FIXED && !(flags & MREMAP_MAYMOVE))
return ret;
+ if (flags & MREMAP_DONTUNMAP && !(flags & MREMAP_FIXED))
+ return ret;
+
if (offset_in_page(addr))
return ret;
@@ -634,7 +657,8 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
if (flags & MREMAP_FIXED) {
ret = mremap_to(addr, old_len, new_addr, new_len,
- &locked, &uf, &uf_unmap_early, &uf_unmap);
+ &locked, flags, &uf, &uf_unmap_early,
+ &uf_unmap);
goto out;
}
@@ -712,7 +736,7 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
}
ret = move_vma(vma, addr, old_len, new_len, new_addr,
- &locked, &uf, &uf_unmap);
+ &locked, flags, &uf, &uf_unmap);
}
out:
if (offset_in_page(ret)) {
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related
* Re: [PATCH v2] mm: Add MREMAP_DONTUNMAP to mremap().
From: Florian Weimer @ 2020-01-27 10:13 UTC (permalink / raw)
To: Brian Geffon
Cc: Andrew Morton, Michael S . Tsirkin, Arnd Bergmann,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, linux-api-u79uwXL29TY76Z2rM5mHXA,
Andy Lutomirski, Andrea Arcangeli, Sonny Rao, Minchan Kim,
Joel Fernandes, Yu Zhao, Jesse Barnes
In-Reply-To: <20200124190625.257659-1-bgeffon-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
* Brian Geffon:
> When remapping an anonymous, private mapping, if MREMAP_DONTUNMAP is
> set, the source mapping will not be removed. Instead it will be
> cleared as if a brand new anonymous, private mapping had been created
> atomically as part of the mremap() call. If a userfaultfd was watching
> the source, it will continue to watch the new mapping. For a mapping
> that is shared or not anonymous, MREMAP_DONTUNMAP will cause the
> mremap() call to fail. MREMAP_DONTUNMAP implies that MREMAP_FIXED is
> also used. The final result is two equally sized VMAs where the
> destination contains the PTEs of the source.
What will be the protection flags of the source mapping? Will they
remain unchanged? Or PROT_NONE?
Thanks,
Florian
^ permalink raw reply
* Re: [PATCH] Add prctl support for controlling mem reclaim V4
From: Christian Brauner @ 2020-01-27 13:02 UTC (permalink / raw)
To: Dave Chinner
Cc: Mike Christie, Shakeel Butt, Andrew Morton, linux-api, idryomov,
Michal Hocko, Linux MM, LKML, linux-scsi, linux-fsdevel,
linux-block, martin, Damien.LeMoal, Michal Hocko, Masato Suzuki
In-Reply-To: <20200124211642.GB7216@dread.disaster.area>
On Sat, Jan 25, 2020 at 08:16:42AM +1100, Dave Chinner wrote:
> On Fri, Jan 24, 2020 at 10:22:33AM -0600, Mike Christie wrote:
> > On 12/05/2019 04:43 PM, Shakeel Butt wrote:
> > > On Mon, Nov 11, 2019 at 4:19 PM Mike Christie <mchristi@redhat.com> wrote:
> > >> This patch adds a new prctl command that daemons can use after they have
> > >> done their initial setup, and before they start to do allocations that
> > >> are in the IO path. It sets the PF_MEMALLOC_NOIO and PF_LESS_THROTTLE
> > >> flags so both userspace block and FS threads can use it to avoid the
> > >> allocation recursion and try to prevent from being throttled while
> > >> writing out data to free up memory.
> > >>
> > >> Signed-off-by: Mike Christie <mchristi@redhat.com>
> > >> Acked-by: Michal Hocko <mhocko@suse.com>
> > >> Tested-by: Masato Suzuki <masato.suzuki@wdc.com>
> > >> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
> > >
> > > I suppose this patch should be routed through MM tree, so, CCing Andrew.
> > >
> >
> > Andrew and other mm/storage developers,
> >
> > Do I need to handle anything else for this patch, or are there any other
> > concerns? Is this maybe something we want to talk about at a quick LSF
> > session?
> >
> > I have retested it with Linus's current tree. It still applies cleanly
> > (just some offsets), and fixes the problem described above we have been
> > hitting.
>
> I must have missed this version being posted (just looked it up on
> lore.kernel.org). As far as I'm concerned this is good to go and it
> is absolutely necessary for userspace IO stacks to function
> correctly.
>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
>
> If no manintainer picks it up before the next merge window, then I
Since prctl() is thread-management and fs people seem to be happy and
have acked it I can pick this up too if noone objects and send this
along with the rest of process management.
Christian
^ permalink raw reply
* Re: [PATCHv8 03/34] ns: Introduce Time Namespace
From: Dmitry Vyukov @ 2020-01-27 14:12 UTC (permalink / raw)
To: Dmitry Safonov
Cc: LKML, Dmitry Safonov, Andrei Vagin, Adrian Reber, Andy Lutomirski,
Arnd Bergmann, Christian Brauner, Cyrill Gorcunov,
Eric W. Biederman, H. Peter Anvin, Ingo Molnar, Jann Horn,
Jeff Dike, Oleg Nesterov, Pavel Emelyanov, Shuah Khan,
Thomas Gleixner, Vincenzo Frascino,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
criu-GEFAQzZX7r8dnm+yROfE0A, Linux API
In-Reply-To: <20191112012724.250792-4-dima-nzgTgzXrdUbQT0dZR+AlfA@public.gmane.org>
On Tue, Nov 12, 2019 at 2:30 AM Dmitry Safonov <dima-nzgTgzXrdUbQT0dZR+AlfA@public.gmane.org> wrote:
>
> From: Andrei Vagin <avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
>
> Time Namespace isolates clock values.
>
> The kernel provides access to several clocks CLOCK_REALTIME,
> CLOCK_MONOTONIC, CLOCK_BOOTTIME, etc.
>
> CLOCK_REALTIME
> System-wide clock that measures real (i.e., wall-clock) time.
>
> CLOCK_MONOTONIC
> Clock that cannot be set and represents monotonic time since
> some unspecified starting point.
>
> CLOCK_BOOTTIME
> Identical to CLOCK_MONOTONIC, except it also includes any time
> that the system is suspended.
>
> For many users, the time namespace means the ability to changes date and
> time in a container (CLOCK_REALTIME).
>
> But in a context of the checkpoint/restore functionality, monotonic and
> bootime clocks become interesting. Both clocks are monotonic with
> unspecified staring points. These clocks are widely used to measure time
> slices and set timers. After restoring or migrating processes, we have to
> guarantee that they never go backward. In an ideal case, the behavior of
> these clocks should be the same as for a case when a whole system is
> suspended. All this means that we need to be able to set CLOCK_MONOTONIC
> and CLOCK_BOOTTIME clocks, what can be done by adding per-namespace
> offsets for clocks.
>
> A time namespace is similar to a pid namespace in a way how it is
> created: unshare(CLONE_NEWTIME) system call creates a new time namespace,
> but doesn't set it to the current process. Then all children of
> the process will be born in the new time namespace, or a process can
> use the setns() system call to join a namespace.
>
> This scheme allows setting clock offsets for a namespace, before any
> processes appear in it.
>
> All available clone flags have been used, so CLONE_NEWTIME uses the
> highest bit of CSIGNAL. It means that we can use it with the unshare()
> system call only. Rith now, this works for us, because time namespace
> offsets can be set only when a new time namespace is not populated. In a
> future, we will have the clone3() system call [1] which will allow to use
> the CSIGNAL mask for clone flags.
>
> [1]: httmps://lkml.kernel.org/r/20190604160944.4058-1-christian-STijNZzMWpgWenYVfaLwtA@public.gmane.org
>
> Link: https://criu.org/Time_namespace
> Link: https://lists.openvz.org/pipermail/criu/2018-June/041504.html
> Signed-off-by: Andrei Vagin <avagin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Co-developed-by: Dmitry Safonov <dima-nzgTgzXrdUbQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Dmitry Safonov <dima-nzgTgzXrdUbQT0dZR+AlfA@public.gmane.org>
> ---
> MAINTAINERS | 2 +
> fs/proc/namespaces.c | 4 +
> include/linux/nsproxy.h | 2 +
> include/linux/proc_ns.h | 3 +
> include/linux/time_namespace.h | 66 ++++++++++
> include/linux/user_namespace.h | 1 +
> include/uapi/linux/sched.h | 6 +
> init/Kconfig | 7 ++
> kernel/fork.c | 16 ++-
> kernel/nsproxy.c | 41 +++++--
> kernel/time/Makefile | 1 +
> kernel/time/namespace.c | 217 +++++++++++++++++++++++++++++++++
> 12 files changed, 356 insertions(+), 10 deletions(-)
> create mode 100644 include/linux/time_namespace.h
> create mode 100644 kernel/time/namespace.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3f7f8cdbc471..037abc28c414 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13172,6 +13172,8 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers/core
> S: Maintained
> F: fs/timerfd.c
> F: include/linux/timer*
> +F: include/linux/time_namespace.h
> +F: kernel/time_namespace.c
Is it supposed to be kernel/time/namespace.c?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox