linux-kernel - Re: [RFC][PATCH v2 5/5] sched: User Mode Concurency Groups

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 21 Jan 2022 13:26:25 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     mingo@...hat.com, tglx@...utronix.de, juri.lelli@...hat.com,
        vincent.guittot@...aro.org, dietmar.eggemann@....com,
        rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
        bristot@...hat.com
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        linux-api@...r.kernel.org, x86@...nel.org, pjt@...gle.com,
        posk@...gle.com, avagin@...gle.com, jannh@...gle.com,
        tdelisle@...terloo.ca, mark.rutland@....com, posk@...k.io,
        david@...hat.com
Subject: Re: [RFC][PATCH v2 5/5] sched: User Mode Concurency Groups

On Thu, Jan 20, 2022 at 04:55:22PM +0100, Peter Zijlstra wrote:
> +/*
> + * Pinning a page inhibits rmap based unmap for Anon pages. Doing a load
> + * through the user mapping ensures the user mapping exists.
> + */
> +#define umcg_pin_and_load(_self, _pagep, _member)				\
> +({										\
> +	__label__ __out;							\
> +	int __ret = -EFAULT;							\
> +										\
> +	if (pin_user_pages_fast((unsigned long)(_self), 1, 0, &(_pagep)) != 1)	\
> +		goto __out;							\
> +										\
> +	if (!PageAnon(_pagep) ||						\
> +	    get_user(_member, &(_self)->_member)) {				\
> +		unpin_user_page(_pagep);					\
> +		goto __out;							\
> +	}									\
> +	__ret = 0;								\
> +__out:	__ret;									\
> +})

Per the thread with David, this wants changing like so.

---

--- a/kernel/sched/umcg.c
+++ b/kernel/sched/umcg.c
@@ -34,25 +34,26 @@ static struct task_struct *umcg_get_task
 }
 
 /*
- * Pinning a page inhibits rmap based unmap for Anon pages. Doing a load
- * through the user mapping ensures the user mapping exists.
+ * Pinning a page inhibits rmap based unmap for Anon pages. Doing a store
+ * through the user mapping ensures the user mapping exists and is writable.
  */
-#define umcg_pin_and_load(_self, _pagep, _member)				\
-({										\
-	__label__ __out;							\
-	int __ret = -EFAULT;							\
-										\
-	if (pin_user_pages_fast((unsigned long)(_self), 1, 0, &(_pagep)) != 1)	\
-		goto __out;							\
-										\
-	if (!PageAnon(_pagep) ||						\
-	    get_user(_member, &(_self)->_member)) {				\
-		unpin_user_page(_pagep);					\
-		goto __out;							\
-	}									\
-	__ret = 0;								\
-__out:	__ret;									\
-})
+static int umcg_pin_page(struct umcg_task __user *self, struct page **pagep)
+{
+	int ret = -EFAULT;
+
+	if (pin_user_pages_fast((unsigned long)self, 1, FOLL_WRITE, pagep) != 1)
+		goto out;
+
+	if (!PageAnon(*pagep) ||
+	    put_user(0ULL, &self->__zero[0])) {
+		unpin_user_page(*pagep);
+		goto out;
+	}
+
+	ret = 0;
+out:
+	return ret;
+}
 
 /**
  * umcg_pin_pages: pin pages containing struct umcg_task of
@@ -72,10 +73,13 @@ static int umcg_pin_pages(void)
 			 tsk->umcg_server))
 		return -EBUSY;
 
-	ret = umcg_pin_and_load(self, tsk->umcg_page, server_tid);
+	ret = umcg_pin_page(self, &tsk->umcg_page);
 	if (ret)
 		goto clear_self;
 
+	if (get_user(server_tid, &self->server_tid))
+		goto unpin_self;
+
 	ret = -ESRCH;
 	server = umcg_get_task(server_tid);
 	if (!server)
@@ -83,7 +87,7 @@ static int umcg_pin_pages(void)
 
 	/* must cache due to possible concurrent change */
 	tsk->umcg_server_task = READ_ONCE(server->umcg_task);
-	ret = umcg_pin_and_load(tsk->umcg_server_task, tsk->umcg_server_page, server_tid);
+	ret = umcg_pin_page(tsk->umcg_server_task, &tsk->umcg_server_page);
 	if (ret)
 		goto clear_server;
 
@@ -414,7 +418,7 @@ static int umcg_wait(u64 timo)
 				break;
 			}
 
-			ret = umcg_pin_and_load(self, page, state);
+			ret = umcg_pin_page(self, &page);
 			if (ret) {
 				page = NULL;
 				break;