Re: [RFC PATCH 01/11] prctl: Support movement of arch prctls out of common code

Re: [RFC PATCH 01/11] prctl: Support movement of arch prctls out of common code

[Dave Martin]

On Mon, May 21, 2018 at 07:28:26PM +0100, Will Deacon wrote:
> Hi Dave,
> 
> On Mon, May 14, 2018 at 06:14:17PM +0100, Dave Martin wrote:
> > The core framework for the prctl() syscall is unloved and looking
> > rather crusty these days.  It also relies on defining ancillary
> > boilerplate macros for each prctl() in order to control conditional
> > compilation of the different prctl calls.  We have better ways to
> > do this now, using Kconfig.
> > 
> > This patch defines a new arch hook arch_syscall().  Architectures
> > that implemement arch-specific syscalls can now select
> > HAVE_ARCH_SYSCALL in their Kconfig and define this function
> > appropriately.
> > 
> > The core prctl() implementation now matches option against the list
> > of common or legacy prctls, deferring to prctl_arch() if an
> > unrecognised option is encountered.
> > 
> > (arch_prctl() would have been a nicer name, but it conflicts with the
> > pre-existing syscall of the same name on x86, particularly in the um
> > code.)
> > 
> > No functional change.
> 
> [...]
> 
> > diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> > index af5f8c2..c911ff0 100644
> > --- a/include/uapi/linux/prctl.h
> > +++ b/include/uapi/linux/prctl.h
> > @@ -1,6 +1,6 @@
> >  /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > -#ifndef _LINUX_PRCTL_H
> > -#define _LINUX_PRCTL_H
> > +#ifndef _UAPI_LINUX_PRCTL_H
> > +#define _UAPI_LINUX_PRCTL_H
> 
> Is it safe to rename this #define, or is there a possibility that userspace
> could be using it for something and relying on it not changing?
> 
> Other than that:
> 
> Acked-by: Will Deacon <will.deacon@arm.com>

I think that header guards are generally viewed as being private to a
project and not API.  codesearch.debian.net yields few hits, where there
are actual #ifdefs on this symbol, they seem to be pastes of the kernel
headers:

https://codesearch.debian.net/search?q=_LINUX_PRCTL_H&perpkg=1

 -> android, linux, dietlibc ... gnome-chess (?)


There is plenty of precedent in the Howells special:

607ca46e97a1 ("UAPI: (Scripted) Disintegrate include/linux")

See for example <{,uapi/}linux/loop.h>.

Cheers
---Dave

[RFC PATCH 00/11] prctl: Modernise wiring for optional prctl() calls

[Dave Martin]

[Reviewer note: this is a cross-arch series.  To reduce spam, I have
tried not to Cc people on patches they aren't likely to care about.
The complete series can be found in the LKML or linux-arch archives.]

The core framework for the prctl() syscall is unloved and looking
rather crusty these days.  It also relies on defining ancillary
boilerplate macros for each prctl() in order to control conditional
compilation of the different prctl calls.  We have better ways to
do this now.

This series attempts to modernise the code by defining a couple of new
Kconfig variables HAVE_PRCTL_ARCH and HAVE_ARCH_PR_SET_GET_UNALIGN to
allow architectures to provide hooks for arch-dependent prctls.


For now this series has had minimal testing: some basic testing of
arch-specific prctls using PR_SVE_SET_VL on arm64; build-tested on
x86; otherwise untested.

This is not polished yet... but I'm interested to know what people
think about the approach.

Cheers
---Dave


Dave Martin (11):
  prctl: Support movement of arch prctls out of common code
  arm64: Move arch-specific prctls out of core code
  MIPS: Remove unused task argument from prctl functions
  MIPS: Move arch-specific prctls out of core code
  x86: Move arch-specific prctls out of core code
  powerpc: Remove unused task argument from prctl functions
  powerpc: Move arch-specific prctls out of core code
  ia64: Remove unused task argument from prctl functions
  ia64: Move arch-specific prctls out of core code
  prctl: Remove redundant task argument from PR_{SET,GET}_UNALIGN
    backends
  prctl: Refactor PR_{SET,GET}_UNALIGN to reduce boilerplate

 arch/Kconfig                         |   6 ++
 arch/alpha/Kconfig                   |   1 +
 arch/alpha/include/asm/thread_info.h |  23 --------
 arch/alpha/kernel/Makefile           |   2 +-
 arch/alpha/kernel/sys.c              |  36 ++++++++++++
 arch/arm64/Kconfig                   |   1 +
 arch/arm64/include/asm/processor.h   |   4 --
 arch/arm64/kernel/sys.c              |  15 +++++
 arch/ia64/Kconfig                    |   2 +
 arch/ia64/include/asm/processor.h    |  24 --------
 arch/ia64/kernel/sys_ia64.c          |  37 ++++++++++++
 arch/mips/Kconfig                    |   1 +
 arch/mips/include/asm/processor.h    |   3 -
 arch/mips/kernel/process.c           |  23 ++++----
 arch/mips/kernel/syscall.c           |  15 +++++
 arch/parisc/Kconfig                  |   1 +
 arch/parisc/include/asm/processor.h  |  14 -----
 arch/parisc/kernel/sys_parisc.c      |  15 +++++
 arch/powerpc/Kconfig                 |   2 +
 arch/powerpc/include/asm/processor.h |  20 ++-----
 arch/powerpc/kernel/process.c        |  38 ++++++------
 arch/powerpc/kernel/syscalls.c       |  17 ++++++
 arch/sh/Kconfig                      |   1 +
 arch/sh/include/asm/processor.h      |   7 ---
 arch/sh/mm/alignment.c               |  12 ++--
 arch/x86/Kconfig                     |   1 +
 arch/x86/include/asm/processor.h     |   6 --
 arch/x86/kernel/Makefile             |   1 +
 arch/x86/kernel/sys.c                |  26 +++++++++
 include/linux/prctl.h                |  27 +++++++++
 include/uapi/linux/prctl.h           |   6 +-
 kernel/sys.c                         | 110 ++++-------------------------------
 tools/include/uapi/linux/prctl.h     |   6 +-
 33 files changed, 263 insertions(+), 240 deletions(-)
 create mode 100644 arch/alpha/kernel/sys.c
 create mode 100644 arch/x86/kernel/sys.c
 create mode 100644 include/linux/prctl.h

-- 
2.1.4

[RFC PATCH 01/11] prctl: Support movement of arch prctls out of common code

[Dave Martin]

The core framework for the prctl() syscall is unloved and looking
rather crusty these days.  It also relies on defining ancillary
boilerplate macros for each prctl() in order to control conditional
compilation of the different prctl calls.  We have better ways to
do this now, using Kconfig.

This patch defines a new arch hook arch_syscall().  Architectures
that implemement arch-specific syscalls can now select
HAVE_ARCH_SYSCALL in their Kconfig and define this function
appropriately.

The core prctl() implementation now matches option against the list
of common or legacy prctls, deferring to prctl_arch() if an
unrecognised option is encountered.

(arch_prctl() would have been a nicer name, but it conflicts with the
pre-existing syscall of the same name on x86, particularly in the um
code.)

No functional change.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: James Hogan <jhogan@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Rich Felker <dalias@libc.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: x86@kernel.org
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 arch/Kconfig                     |  3 +++
 include/linux/prctl.h            | 19 +++++++++++++++++++
 include/uapi/linux/prctl.h       |  6 +++---
 kernel/sys.c                     |  2 +-
 tools/include/uapi/linux/prctl.h |  6 +++---
 5 files changed, 29 insertions(+), 7 deletions(-)
 create mode 100644 include/linux/prctl.h

diff --git a/arch/Kconfig b/arch/Kconfig
index 8e0d665..b34b3e8 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -969,4 +969,7 @@ config REFCOUNT_FULL
 	  against various use-after-free conditions that can be used in
 	  security flaw exploits.
 
+config HAVE_PRCTL_ARCH
+	bool
+
 source "kernel/gcov/Kconfig"
diff --git a/include/linux/prctl.h b/include/linux/prctl.h
new file mode 100644
index 0000000..5ce3713
--- /dev/null
+++ b/include/linux/prctl.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_PRCTL_H
+#define _LINUX_PRCTL_H
+
+#include <linux/errno.h>
+#include <uapi/linux/prctl.h>
+
+#ifdef CONFIG_HAVE_PRCTL_ARCH
+extern int prctl_arch(int option, unsigned long arg2,
+	unsigned long arg3, unsigned long arg4, unsigned long arg5);
+#else
+static inline int prctl_arch(int option, unsigned long arg2,
+	unsigned long arg3, unsigned long arg4, unsigned long arg5)
+{
+	return -EINVAL;
+}
+#endif
+
+#endif /* ! _LINUX_PRCTL_H */
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index af5f8c2..c911ff0 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _LINUX_PRCTL_H
-#define _LINUX_PRCTL_H
+#ifndef _UAPI_LINUX_PRCTL_H
+#define _UAPI_LINUX_PRCTL_H
 
 #include <linux/types.h>
 
@@ -207,4 +207,4 @@ struct prctl_mm_map {
 # define PR_SVE_VL_LEN_MASK		0xffff
 # define PR_SVE_VL_INHERIT		(1 << 17) /* inherit across exec */
 
-#endif /* _LINUX_PRCTL_H */
+#endif /* _UAPI_LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index ad69218..5077f1e 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2451,7 +2451,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 		error = SVE_GET_VL();
 		break;
 	default:
-		error = -EINVAL;
+		error = prctl_arch(option, arg2, arg3, arg4, arg5);
 		break;
 	}
 	return error;
diff --git a/tools/include/uapi/linux/prctl.h b/tools/include/uapi/linux/prctl.h
index af5f8c2..c911ff0 100644
--- a/tools/include/uapi/linux/prctl.h
+++ b/tools/include/uapi/linux/prctl.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _LINUX_PRCTL_H
-#define _LINUX_PRCTL_H
+#ifndef _UAPI_LINUX_PRCTL_H
+#define _UAPI_LINUX_PRCTL_H
 
 #include <linux/types.h>
 
@@ -207,4 +207,4 @@ struct prctl_mm_map {
 # define PR_SVE_VL_LEN_MASK		0xffff
 # define PR_SVE_VL_INHERIT		(1 << 17) /* inherit across exec */
 
-#endif /* _LINUX_PRCTL_H */
+#endif /* _UAPI_LINUX_PRCTL_H */
-- 
2.1.4

Re: [RFC PATCH 01/11] prctl: Support movement of arch prctls out of common code

[Will Deacon]

Hi Dave,

On Mon, May 14, 2018 at 06:14:17PM +0100, Dave Martin wrote:
> The core framework for the prctl() syscall is unloved and looking
> rather crusty these days.  It also relies on defining ancillary
> boilerplate macros for each prctl() in order to control conditional
> compilation of the different prctl calls.  We have better ways to
> do this now, using Kconfig.
> 
> This patch defines a new arch hook arch_syscall().  Architectures
> that implemement arch-specific syscalls can now select
> HAVE_ARCH_SYSCALL in their Kconfig and define this function
> appropriately.
> 
> The core prctl() implementation now matches option against the list
> of common or legacy prctls, deferring to prctl_arch() if an
> unrecognised option is encountered.
> 
> (arch_prctl() would have been a nicer name, but it conflicts with the
> pre-existing syscall of the same name on x86, particularly in the um
> code.)
> 
> No functional change.

[...]

> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index af5f8c2..c911ff0 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> -#ifndef _LINUX_PRCTL_H
> -#define _LINUX_PRCTL_H
> +#ifndef _UAPI_LINUX_PRCTL_H
> +#define _UAPI_LINUX_PRCTL_H

Is it safe to rename this #define, or is there a possibility that userspace
could be using it for something and relying on it not changing?

Other than that:

Acked-by: Will Deacon <will.deacon@arm.com>

Will