[PATCH 0/3] two small fixes out of kernel/sys.c

[PATCH 0/3] two small fixes out of kernel/sys.c

[Bradley Morgan]

Two unrelated things that came out of reading kernel/sys.c line by line.
They are in one series only because they came from the same read, patch 3
stands on its own and can be taken without the other two.

Patches 1 and 2 have to go in that order.  PR_FUTEX_HASH is the only case
in the prctl switch that does not check the args it does not use, arg5
does not even reach futex_hash_prctl().  Patch 2 adds the checks.  Patch 1
goes first because prctl() is variadic, so the two callers under tools/
leave the args they do not pass as whatever those registers hold, and
patch 2 would fail them at random without it.

Patch 3 is compat_sysinfo().  It scales the 64 bit memory values down so
they fit the 32 bit fields, and stops when the unit reaches PAGE_SIZE.
But do_sysinfo() hands it bytes with mem_unit at 1, so on a box with
enough ram it stops far too early and drops the high bits.  16 TiB reads
back as 0 and 17 TiB reads back as 1 TiB.  The loop only runs longer on
the path that was already wrong, nothing that works today changes.

Tested on ppc64.

Bradley Morgan (3):
  tools: pass the unused prctl() args explicitly for PR_FUTEX_HASH
  futex: reject the unused PR_FUTEX_HASH arguments
  compat: make compat_sysinfo() scale far enough to actually fit

 include/linux/futex.h                         |  6 ++--
 kernel/futex/core.c                           |  7 +++--
 kernel/sys.c                                  | 28 +++++++++----------
 tools/perf/bench/futex.c                      |  4 +--
 .../futex/functional/futex_priv_hash.c        |  4 +--
 5 files changed, 26 insertions(+), 23 deletions(-)

-- 
2.53.0

[PATCH 1/3] tools: pass the unused prctl() args explicitly for PR_FUTEX_HASH

[Bradley Morgan]

prctl() is declared variadic in sys/prctl.h, so an argument the caller
leaves out of the call is whatever that register happens to hold.
Neither file passes all five.  The two SET_SLOTS calls leave arg5 out
and the two GET_SLOTS calls leave arg3, arg4 and arg5 out, so those are
whatever the registers hold.  The kernel does not look at them today so
it works, but it works by luck.  I checked that against a running
kernel, garbage in arg5 on SET_SLOTS and in all three on GET_SLOTS are
accepted.

Pass the zeros.  No functional change.

Signed-off-by: Bradley Morgan <brads@mainlining.org>
---
 tools/perf/bench/futex.c                                   | 4 ++--
 tools/testing/selftests/futex/functional/futex_priv_hash.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/bench/futex.c b/tools/perf/bench/futex.c
index 1968c9d00b5bb..f4c1c30efe0bd 100644
--- a/tools/perf/bench/futex.c
+++ b/tools/perf/bench/futex.c
@@ -20,7 +20,7 @@ void futex_set_nbuckets_param(struct bench_futex_parameters *params)
 	if (params->nbuckets < 0)
 		return;
 
-	ret = prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_SET_SLOTS, params->nbuckets, 0);
+	ret = prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_SET_SLOTS, params->nbuckets, 0, 0);
 	if (ret) {
 		printf("Requesting %d hash buckets failed: %d/%m\n",
 		       params->nbuckets, ret);
@@ -33,7 +33,7 @@ void futex_print_nbuckets(struct bench_futex_parameters *params)
 	char *futex_hash_mode;
 	int ret;
 
-	ret = prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_GET_SLOTS);
+	ret = prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_GET_SLOTS, 0, 0, 0);
 	if (params->nbuckets >= 0) {
 		if (ret != params->nbuckets) {
 			if (ret < 0) {
diff --git a/tools/testing/selftests/futex/functional/futex_priv_hash.c b/tools/testing/selftests/futex/functional/futex_priv_hash.c
index a8742e2045402..96bfb72362221 100644
--- a/tools/testing/selftests/futex/functional/futex_priv_hash.c
+++ b/tools/testing/selftests/futex/functional/futex_priv_hash.c
@@ -32,12 +32,12 @@ static int counter;
 
 static int futex_hash_slots_set(unsigned int slots)
 {
-	return prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_SET_SLOTS, slots, 0);
+	return prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_SET_SLOTS, slots, 0, 0);
 }
 
 static int futex_hash_slots_get(void)
 {
-	return prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_GET_SLOTS);
+	return prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_GET_SLOTS, 0, 0, 0);
 }
 
 static void futex_hash_slots_set_verify(struct __test_metadata *_metadata, int slots)
-- 
2.53.0

[PATCH 2/3] futex: reject the unused PR_FUTEX_HASH arguments

[Bradley Morgan]

PR_SET_MM, PR_SET_MDWE and the shadow stack cases all check that the
args they do not use are zero before doing anything.  PR_FUTEX_HASH is
the one that does not.  arg5 never even reaches futex_hash_prctl(), and
the GET_SLOTS branch never looks at arg3 or arg4, so
prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_GET_SLOTS, 1, 2, 3) hands back the
slot count.

Once an arg is ignored it is burned, whatever userspace sends there
today becomes part of the ABI and we can never use it for anything
without breaking whoever already passes garbage.  Take arg5 too and
reject the unused args per sub command, same as the neighbours do.

prctl() is variadic, so the two callers under tools/ leave those args as
whatever the register holds, they only work today because nothing was
checked.  The preceding patch in this series writes the zeros out first,
so this does not break them.

Fixes: 80367ad01d93 ("futex: Add basic infrastructure for local task local hash")
Signed-off-by: Bradley Morgan <brads@mainlining.org>
---
 include/linux/futex.h | 6 ++++--
 kernel/futex/core.c   | 7 +++++--
 kernel/sys.c          | 2 +-
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/include/linux/futex.h b/include/linux/futex.h
index a895d8c69d663..70adc9fce6e63 100644
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -80,7 +80,8 @@ void futex_exec_done(struct task_struct *tsk);
 
 long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout,
 	      u32 __user *uaddr2, u32 val2, u32 val3);
-int futex_hash_prctl(unsigned long arg2, unsigned long arg3, unsigned long arg4);
+int futex_hash_prctl(unsigned long arg2, unsigned long arg3, unsigned long arg4,
+		     unsigned long arg5);
 
 #ifdef CONFIG_FUTEX_PRIVATE_HASH
 int futex_hash_allocate_default(void);
@@ -100,7 +101,8 @@ static inline long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout
 {
 	return -EINVAL;
 }
-static inline int futex_hash_prctl(unsigned long arg2, unsigned long arg3, unsigned long arg4)
+static inline int futex_hash_prctl(unsigned long arg2, unsigned long arg3,
+				   unsigned long arg4, unsigned long arg5)
 {
 	return -EINVAL;
 }
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index a240638f69e5c..eabf8abc2624b 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -2068,19 +2068,22 @@ void futex_mm_init(struct mm_struct *mm)
 	futex_robust_unlock_init_mm(&mm->futex);
 }
 
-int futex_hash_prctl(unsigned long arg2, unsigned long arg3, unsigned long arg4)
+int futex_hash_prctl(unsigned long arg2, unsigned long arg3, unsigned long arg4,
+		     unsigned long arg5)
 {
 	unsigned int flags = FH_CUSTOM;
 	int ret;
 
 	switch (arg2) {
 	case PR_FUTEX_HASH_SET_SLOTS:
-		if (arg4)
+		if (arg4 || arg5)
 			return -EINVAL;
 		ret = futex_hash_allocate(arg3, flags);
 		break;
 
 	case PR_FUTEX_HASH_GET_SLOTS:
+		if (arg3 || arg4 || arg5)
+			return -EINVAL;
 		ret = futex_hash_get_slots();
 		break;
 
diff --git a/kernel/sys.c b/kernel/sys.c
index 755bfa5ccd92c..c4839c986f5e0 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2885,7 +2885,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 		error = posixtimer_create_prctl(arg2);
 		break;
 	case PR_FUTEX_HASH:
-		error = futex_hash_prctl(arg2, arg3, arg4);
+		error = futex_hash_prctl(arg2, arg3, arg4, arg5);
 		break;
 	case PR_RSEQ_SLICE_EXTENSION:
 		if (arg4 || arg5)
-- 
2.53.0

[PATCH 3/3] compat: make compat_sysinfo() scale far enough to actually fit

[Bradley Morgan]

compat_sysinfo() checks whether totalram or totalswap is too big for the
compat struct and scales the values down to fit, but the loop stops as
soon as s.mem_unit reaches PAGE_SIZE.  That caps the compression at one
page, 12 bits with 4K pages.  On a 64 bit kernel do_sysinfo() always
hands it values in bytes with mem_unit == 1, its own overflow math runs
on unsigned long so it does not bail until 16 EiB, so the loop runs its
12 turns, the values come back out as a page count, and anything past
2^32 pages still does not fit.  Storing that into the u32 field then
just drops the top bits.

So a 32 bit process on a box past 2^32 pages gets a wrapped number
instead of a coarse one, with 4K pages that starts at 16 TiB:

  ram      old                  new
  8 TiB    2147483648 x 4096    unchanged
  16 TiB   0 x 4096             2147483648 x 8192
  17 TiB   268435456 x 4096     2281701376 x 8192
  32 TiB   0 x 4096             2147483648 x 16384

The 17 TiB row is the nasty one, a 32 bit process is told the box has
1 TiB of ram.  Keep scaling until the values fit instead of until the
unit reaches a page, the unit then says how coarse the number is, which
is what mem_unit is for.

Nothing that works today changes.  I ran both loops side by side over
ram from 1 MiB to 2^56 with 4K and 64K pages, they disagree on exactly
the inputs where the old one dropped bits and agree on every one it got
right.

Signed-off-by: Bradley Morgan <brads@mainlining.org>
---
 kernel/sys.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/kernel/sys.c b/kernel/sys.c
index c4839c986f5e0..8f683b6bb87b8 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -3035,24 +3035,22 @@ COMPAT_SYSCALL_DEFINE1(sysinfo, struct compat_sysinfo __user *, info)
 	do_sysinfo(&s);
 
 	/* Check to see if any memory value is too large for 32-bit and scale
-	 *  down if needed
+	 * down if needed.  Keep going until the values fit, not just until the
+	 * unit reaches a page.  The values halve each turn, so this ends.
 	 */
 	if (upper_32_bits(s.totalram) || upper_32_bits(s.totalswap)) {
-		int bitcount = 0;
-
-		while (s.mem_unit < PAGE_SIZE) {
+		while (s.mem_unit < PAGE_SIZE || upper_32_bits(s.totalram) ||
+		       upper_32_bits(s.totalswap)) {
 			s.mem_unit <<= 1;
-			bitcount++;
+			s.totalram >>= 1;
+			s.freeram >>= 1;
+			s.sharedram >>= 1;
+			s.bufferram >>= 1;
+			s.totalswap >>= 1;
+			s.freeswap >>= 1;
+			s.totalhigh >>= 1;
+			s.freehigh >>= 1;
 		}
-
-		s.totalram >>= bitcount;
-		s.freeram >>= bitcount;
-		s.sharedram >>= bitcount;
-		s.bufferram >>= bitcount;
-		s.totalswap >>= bitcount;
-		s.freeswap >>= bitcount;
-		s.totalhigh >>= bitcount;
-		s.freehigh >>= bitcount;
 	}
 
 	memset(&s_32, 0, sizeof(s_32));
-- 
2.53.0