[Devel] [PATCH hci-8.1 7/8] exec: protect preserved memory against surviving shared-mm tasks

[Konstantin Khorenko <khorenko@virtuozzo.com>]

From: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>

exec_mmap() copies the MADV_DOEXEC ranges into the new mm and then zaps
them in the old one, so that the copies are the only mappings left.
de_thread() removes the thread group, but a task sharing the mm via
CLONE_VM without CLONE_THREAD survives exec. It races with the copy,
writing into pages the new image takes over, and afterwards finds the
ranges empty and silently goes on with fresh memory.

Refuse such an exec before the point of no return:

- begin_new_exec(): when the binary accepts preserved memory and the
  mm has a VM_EXEC_KEEP VMA, mark the mm as being preserved and return
  EBUSY if a task outside the thread group has it as its mm. Sibling
  threads, which de_thread() removes, and plain mm references such as
  /proc readers are allowed.
- copy_mm(): refuse a CLONE_VM clone that is not a thread while the mm
  is marked, and count such clones in flight until the child is on the
  task list, so the check above cannot miss one. The check walks the
  task list under RCU; it sees every child whose count was dropped,
  since the drop follows the insertion and both are ordered by
  page_table_lock.
- exec_mmap() and free_bprm(): clear the mark once the mm is switched
  or the exec fails.
- exec_mmap(): take the preserve path, including the source mmap
  write lock, only when the mark was set (bprm->preserving_mem), so an
  accepting binary without a VM_EXEC_KEEP VMA takes only the read lock,
  as an exec without preserved memory does.

A kernel thread running on the mm through kthread_use_mm() is outside
the thread group as well, so it makes the exec fail the same way. This
covers a vhost worker in kthread mode, the default here, for as long as
its device has an owner: the caller has to reset the owner first, as
the worker would otherwise go on writing guest memory into the old mm.
A kernel thread that takes the mm only after the check, for instance
to complete an AIO or fence request made before the exec, is not
refused: a write before the zap of the old ranges reaches the preserved
pages, as one made just before the exec would, and a later one lands in
fresh memory of the old mm and is lost, as after any exec.

Fixes: 37d9f55655ea ("oracle/mm: do not write protect COW mappings when preserving across exec")
https://virtuozzo.atlassian.net/browse/VSTOR-150231

Feature: oracle/mm: MADV_DOEXEC madvise() flag
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
[khorenko: rationale after the zap of the old ranges, count only clones
 that are not threads, walk the task list under RCU, keep the upstream
 begin_new_exec() comment, note kthread_use_mm() users]
Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 fs/exec.c                | 82 ++++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/binfmts.h  |  4 ++-
 include/linux/mm_types.h |  4 +++
 kernel/fork.c            | 37 ++++++++++++++++++++++
 4 files changed, 124 insertions(+), 3 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 32567010a2f27..5bf856e829af3 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -978,6 +978,78 @@ ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
 EXPORT_SYMBOL(read_code);
 #endif
 
+/* The reservation owns no mm reference; release it before dropping old_mm. */
+static void release_preserved_mem(struct linux_binprm *bprm, struct mm_struct *mm)
+{
+	if (!bprm->preserving_mem)
+		return;
+	spin_lock(&mm->page_table_lock);
+	mm->exec_keep_active = false;
+	spin_unlock(&mm->page_table_lock);
+	bprm->preserving_mem = false;
+}
+
+/*
+ * Reserve the marked address space before exec becomes irreversible. Tasks
+ * outside our thread group survive de_thread() and would find the preserved
+ * ranges empty once exec_mmap() zaps them in this mm.
+ */
+static int reserve_preserved_mem(struct linux_binprm *bprm)
+{
+	struct mm_struct *mm = current->mm;
+	struct vm_area_struct *vma;
+	struct task_struct *p, *t;
+	bool preserve = false;
+	int ret = 0;
+
+	if (!bprm->accepts_preserved_mem || !mm)
+		return 0;
+
+	if (mmap_read_lock_killable(mm))
+		return -EINTR;
+	{
+		VMA_ITERATOR(vmi, mm, 0);
+
+		for_each_vma(vmi, vma) {
+			if (vma->vm_flags & VM_EXEC_KEEP) {
+				preserve = true;
+				break;
+			}
+		}
+	}
+	mmap_read_unlock(mm);
+	if (!preserve)
+		return 0;
+
+	/* A clone is either counted here or already visible in the task list. */
+	spin_lock(&mm->page_table_lock);
+	if (mm->exec_keep_active || mm->exec_keep_clones)
+		ret = -EBUSY;
+	else
+		mm->exec_keep_active = true;
+	spin_unlock(&mm->page_table_lock);
+	if (ret)
+		return ret;
+	bprm->preserving_mem = true;
+
+	rcu_read_lock();
+	for_each_process_thread(p, t) {
+		if (same_thread_group(current, t))
+			continue;
+		task_lock(t);
+		if (t->mm == mm)
+			ret = -EBUSY;
+		task_unlock(t);
+		if (ret)
+			goto out;
+	}
+out:
+	rcu_read_unlock();
+	if (ret)
+		release_preserved_mem(bprm, mm);
+	return ret;
+}
+
 static int vma_dup_some(struct mm_struct *old_mm, struct mm_struct *new_mm)
 {
 	struct vm_area_struct *vma;
@@ -1040,7 +1112,7 @@ static int exec_mmap(struct linux_binprm *bprm)
 			up_write(&tsk->signal->exec_update_lock);
 			return ret;
 		}
-		if (unlikely(bprm->accepts_preserved_mem)) {
+		if (unlikely(bprm->preserving_mem)) {
 			/*
 			 * Acquire the write lock to avoid changes to page
 			 * table entries while they are copied.
@@ -1085,13 +1157,14 @@ static int exec_mmap(struct linux_binprm *bprm)
 	task_unlock(tsk);
 	lru_gen_use_mm(mm);
 	if (old_mm) {
-		if (unlikely(bprm->accepts_preserved_mem))
+		if (unlikely(bprm->preserving_mem))
 			mmap_write_unlock(old_mm);
 		else
 			mmap_read_unlock(old_mm);
 		BUG_ON(active_mm != old_mm);
 		setmax_mm_hiwater_rss(&tsk->signal->maxrss, old_mm);
 		mm_update_next_owner(old_mm);
+		release_preserved_mem(bprm, old_mm);
 		mmput(old_mm);
 		return 0;
 	}
@@ -1300,6 +1373,10 @@ int begin_new_exec(struct linux_binprm * bprm)
 	if (retval)
 		return retval;
 
+	retval = reserve_preserved_mem(bprm);
+	if (retval)
+		return retval;
+
 	/*
 	 * This tracepoint marks the point before flushing the old exec where
 	 * the current task is still unchanged, but errors are fatal (point of
@@ -1552,6 +1629,7 @@ static void do_close_execat(struct file *file)
 
 static void free_bprm(struct linux_binprm *bprm)
 {
+	release_preserved_mem(bprm, current->mm);
 	if (bprm->mm) {
 		acct_arg_size(bprm, 0);
 		mmput(bprm->mm);
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 9f03c39498e92..f42487c043178 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -47,7 +47,9 @@ struct linux_binprm {
 		 * Set if the binary being exec'd will accept memory marked
 		 * for preservation by the outgoing process.
 		 */
-		accepts_preserved_mem:1;
+		accepts_preserved_mem:1,
+		/* Outgoing mm has VM_EXEC_KEEP VMAs and is reserved for exec. */
+		preserving_mem:1;
 #ifdef __alpha__
 	unsigned int taso:1;
 #endif
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index cf50c74516b09..befdf38ef855b 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -994,6 +994,10 @@ struct mm_struct {
 		unsigned long flags; /* Must use atomic bitops to access */
 
 		unsigned int vps_dumpable:2;
+
+		/* Shared-mm clone publication, serialized by page_table_lock. */
+		unsigned int exec_keep_clones;
+		bool exec_keep_active;
 #ifdef CONFIG_AIO
 		spinlock_t			ioctx_lock;
 		struct kioctx_table __rcu	*ioctx_table;
diff --git a/kernel/fork.c b/kernel/fork.c
index 00d9b4febd85b..92a7e9aae9278 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1270,6 +1270,8 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
 	mt_init_flags(&mm->mm_mt, MM_MT_FLAGS);
 	mt_set_external_lock(&mm->mm_mt, &mm->mmap_lock);
 	atomic_set(&mm->mm_users, 1);
+	mm->exec_keep_clones = 0;
+	mm->exec_keep_active = false;
 	atomic_set(&mm->mm_count, 1);
 	seqcount_init(&mm->write_protect_seq);
 	mmap_init_lock(mm);
@@ -1710,9 +1712,32 @@ static struct mm_struct *dup_mm(struct task_struct *tsk,
 	return NULL;
 }
 
+/* Keep unpublished CLONE_VM tasks visible to preserving-exec admission. */
+static int exec_keep_clone_begin(struct mm_struct *mm)
+{
+	int ret = 0;
+
+	spin_lock(&mm->page_table_lock);
+	if (mm->exec_keep_active)
+		ret = -EBUSY;
+	else
+		mm->exec_keep_clones++;
+	spin_unlock(&mm->page_table_lock);
+	return ret;
+}
+
+static void exec_keep_clone_end(struct mm_struct *mm)
+{
+	spin_lock(&mm->page_table_lock);
+	if (!WARN_ON_ONCE(!mm->exec_keep_clones))
+		mm->exec_keep_clones--;
+	spin_unlock(&mm->page_table_lock);
+}
+
 static int copy_mm(unsigned long clone_flags, struct task_struct *tsk)
 {
 	struct mm_struct *mm, *oldmm;
+	int ret;
 
 	tsk->min_flt = tsk->maj_flt = 0;
 	tsk->nvcsw = tsk->nivcsw = 0;
@@ -1734,6 +1759,12 @@ static int copy_mm(unsigned long clone_flags, struct task_struct *tsk)
 		return 0;
 
 	if (clone_flags & CLONE_VM) {
+		/* de_thread() takes care of our own threads. */
+		if (!(clone_flags & CLONE_THREAD)) {
+			ret = exec_keep_clone_begin(oldmm);
+			if (ret)
+				return ret;
+		}
 		mmget(oldmm);
 		mm = oldmm;
 	} else {
@@ -2638,6 +2669,10 @@ __latent_entropy struct task_struct *copy_process(
 	syscall_tracepoint_update(p);
 	write_unlock_irq(&tasklist_lock);
 
+	/* Admission can now find this shared-mm task in the task list. */
+	if ((clone_flags & (CLONE_VM | CLONE_THREAD)) == CLONE_VM && p->mm)
+		exec_keep_clone_end(p->mm);
+
 	if (pidfile)
 		fd_install(pidfd, pidfile);
 
@@ -2679,6 +2714,8 @@ __latent_entropy struct task_struct *copy_process(
 	exit_ve_namespace(p);
 bad_fork_cleanup_mm:
 	if (p->mm) {
+		if ((clone_flags & (CLONE_VM | CLONE_THREAD)) == CLONE_VM)
+			exec_keep_clone_end(p->mm);
 		mm_clear_owner(p->mm, p);
 		mmput(p->mm);
 	}

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel