Re: [PATCH 1/2] Deprecate sysctl syscall interface

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 23/04/2018 20:19, Joseph Myers wrote:
> Also, sysctl is documented, so a proper obsoletion would need to remove 
> the documentation, in addition to removing the declaration, adding a NEWS 
> entry, making existing symbol versions into compat symbols and ensuring 
> the function is not built at all for new ports or static linking.
> 

My initial idea was to follow generic logic (new ports returns EINVAL),
but indeed adding new symbols does not make sense for deprecation.  Below
it is an updated patch which addresses the issues you brought.

---

The sysctl syscall has been long deprecated [1] (since Linux 2.6.24)
with its usage issues a kernel warning stating it.  Also on GLIBC new
ports, which uses generic syscall interface, builds a stub version that
returns ENOSYS (so new ports should not rely on it anyway and some
kernel config do disable it by not setting CONFIG_SYSCTL_SYSCALL).

Every interface is directly accessed through /proc/sys and the only
real case scanarion I am aware is to have a fail-proof way to gather
entropy in cases where /dev/urandom or /proc is not available.  However,
as noted previously, this is quite fragile (new ports do not provide it)
and GLIBC now provides getentropy to this specific case (although it is
only supported on Linux 3.17).  If sysctl is stricly required, it still
can be issues through syscall().

This patch deprecate sysctl interface by issuing ENOSYS as default
and adds compat symbol for architectures which still defines
__NR_sysctl.

Checked on x86_64-linux-gnu and i686-linux-gnu.  Also checked with a
check-abi on all affected ABIs.

	* NEWS: Add sysctl.h deprecation entry.
	* include/sys/sysctl.h: Remove file.
	* sysdeps/unix/sysv/linux/bits/sysctl.h: Likewise.
	* sysdeps/unix/sysv/linux/generic/sysctl.c: Likewise.
	* sysdeps/unix/sysv/linux/sys/sysctl.h: Likewise.
	* sysdeps/unix/sysv/linux/x86/bits/sysctl.h: Likewise.
	* manual/sysinfo.texi: Update sysctl documentation.
	* sysdeps/unix/sysv/linux/Makefile (sysdep_headers): Remove
	bits/sysctl.
	* sysdeps/unix/sysv/linux/arm/ioperm.c (sysctl_call): New function.
	(init_iosys): Call syscall_call instead of sysctl.
	* sysdeps/unix/sysv/linux/sysctl.c (__sysctl): Rename to __old_sysctl
	and export only in compatibility mode.

[1] https://lwn.net/Articles/247243/
---
 ChangeLog                                 |  16 ++++
 NEWS                                      |   5 ++
 include/sys/sysctl.h                      |  13 ---
 manual/sysinfo.texi                       | 139 +-----------------------------
 sysdeps/unix/sysv/linux/Makefile          |   4 +-
 sysdeps/unix/sysv/linux/arm/ioperm.c      |  21 ++++-
 sysdeps/unix/sysv/linux/bits/sysctl.h     |   1 -
 sysdeps/unix/sysv/linux/generic/sysctl.c  |  32 -------
 sysdeps/unix/sysv/linux/sys/sysctl.h      |  73 ----------------
 sysdeps/unix/sysv/linux/sysctl.c          |  39 ++++++---
 sysdeps/unix/sysv/linux/x86/bits/sysctl.h |  20 -----
 11 files changed, 72 insertions(+), 291 deletions(-)
 delete mode 100644 include/sys/sysctl.h
 delete mode 100644 sysdeps/unix/sysv/linux/bits/sysctl.h
 delete mode 100644 sysdeps/unix/sysv/linux/generic/sysctl.c
 delete mode 100644 sysdeps/unix/sysv/linux/sys/sysctl.h
 delete mode 100644 sysdeps/unix/sysv/linux/x86/bits/sysctl.h

diff --git a/NEWS b/NEWS
index 92c9b14..ffe3bb4 100644
--- a/NEWS
+++ b/NEWS
@@ -57,6 +57,11 @@ Deprecated and removed features, and other changes affecting compatibility:
    these macros should first include <sys/types.h>, and then include
    <sys/sysmacros.h> if __GNU_LIBRARY__ is defined.
 
+ * The Linux header file <sys/sysctl.h> is no longer installed.  The
+   sysctl syscall has been long deprecated since Linux 2.6.24 in favor
+   of /proc/sys.  A compatibility symbol is still provided, however it is
+   not being defined for static linking or for new ports.
+
 Changes to build and runtime requirements:
 
   [Add changes to build and runtime requirements here]
diff --git a/include/sys/sysctl.h b/include/sys/sysctl.h
deleted file mode 100644
index 2a15e91..0000000
--- a/include/sys/sysctl.h
+++ /dev/null
@@ -1,13 +0,0 @@
-#ifndef _SYS_SYSCTL_H
-#include_next <sys/sysctl.h>
-
-# ifndef _ISOMAC
-
-/* Read or write system parameters (Linux, FreeBSD specific).  */
-extern int __sysctl (int *__name, int __nlen, void *__oldval,
-		     size_t *__oldlenp, void *__newval, size_t __newlen);
-libc_hidden_proto (__sysctl)
-
-
-# endif /* !_ISOMAC */
-#endif  /* _SYS_SYSCTL_H */
diff --git a/manual/sysinfo.texi b/manual/sysinfo.texi
index 4beee01..1ebc0b9 100644
--- a/manual/sysinfo.texi
+++ b/manual/sysinfo.texi
@@ -1113,140 +1113,5 @@ when @code{umount2} is also available.
 @node System Parameters
 @section System Parameters
 
-This section describes the @code{sysctl} function, which gets and sets
-a variety of system parameters.
-
-The symbols used in this section are declared in the file @file{sys/sysctl.h}.
-
-@deftypefun int sysctl (int *@var{names}, int @var{nlen}, void *@var{oldval}, size_t *@var{oldlenp}, void *@var{newval}, size_t @var{newlen})
-@standards{BSD, sys/sysctl.h}
-@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
-@c Direct syscall, Linux only.
-
-@code{sysctl} gets or sets a specified system parameter.  There are so
-many of these parameters that it is not practical to list them all here,
-but here are some examples:
-
-@itemize @bullet
-@item network domain name
-@item paging parameters
-@item network Address Resolution Protocol timeout time
-@item maximum number of files that may be open
-@item root filesystem device
-@item when kernel was built
-@end itemize
-
-The set of available parameters depends on the kernel configuration and
-can change while the system is running, particularly when you load and
-unload loadable kernel modules.
-
-The system parameters with which @code{sysctl} is concerned are arranged
-in a hierarchical structure like a hierarchical filesystem.  To identify
-a particular parameter, you specify a path through the structure in a
-way analogous to specifying the pathname of a file.  Each component of
-the path is specified by an integer and each of these integers has a
-macro defined for it by @file{sys/sysctl.h}.  @var{names} is the path, in
-the form of an array of integers.  Each component of the path is one
-element of the array, in order.  @var{nlen} is the number of components
-in the path.
-
-For example, the first component of the path for all the paging
-parameters is the value @code{CTL_VM}.  For the free page thresholds, the
-second component of the path is @code{VM_FREEPG}.  So to get the free
-page threshold values, make @var{names} an array containing the two
-elements @code{CTL_VM} and @code{VM_FREEPG} and make @var{nlen} = 2.
-
-
-The format of the value of a parameter depends on the parameter.
-Sometimes it is an integer; sometimes it is an ASCII string; sometimes
-it is an elaborate structure.  In the case of the free page thresholds
-used in the example above, the parameter value is a structure containing
-several integers.
-
-In any case, you identify a place to return the parameter's value with
-@var{oldval} and specify the amount of storage available at that
-location as *@var{oldlenp}.  *@var{oldlenp} does double duty because it
-is also the output location that contains the actual length of the
-returned value.
-
-If you don't want the parameter value returned, specify a null pointer
-for @var{oldval}.
-
-To set the parameter, specify the address and length of the new value
-as @var{newval} and @var{newlen}.  If you don't want to set the parameter,
-specify a null pointer as @var{newval}.
-
-If you get and set a parameter in the same @code{sysctl} call, the value
-returned is the value of the parameter before it was set.
-
-Each system parameter has a set of permissions similar to the
-permissions for a file (including the permissions on directories in its
-path) that determine whether you may get or set it.  For the purposes of
-these permissions, every parameter is considered to be owned by the
-superuser and Group 0 so processes with that effective uid or gid may
-have more access to system parameters.  Unlike with files, the superuser
-does not invariably have full permission to all system parameters, because
-some of them are designed not to be changed ever.
-
-
-@code{sysctl} returns a zero return value if it succeeds.  Otherwise, it
-returns @code{-1} and sets @code{errno} appropriately.  Besides the
-failures that apply to all system calls, the following are the
-@code{errno} codes for all possible failures:
-
-@table @code
-@item EPERM
-The process is not permitted to access one of the components of the
-path of the system parameter or is not permitted to access the system parameter
-itself in the way (read or write) that it requested.
-@c There is some indication in the Linux 2.2 code that the code is trying to
-@c return EACCES here, but the EACCES value never actually makes it to the
-@c user.
-@item ENOTDIR
-There is no system parameter corresponding to @var{name}.
-@item EFAULT
-@var{oldval} is not null, which means the process wanted to read the parameter,
-but *@var{oldlenp} is zero, so there is no place to return it.
-@item EINVAL
-@itemize @bullet
-@item
-The process attempted to set a system parameter to a value that is not valid
-for that parameter.
-@item
-The space provided for the return of the system parameter is not the right
-size for that parameter.
-@end itemize
-@item ENOMEM
-This value may be returned instead of the more correct @code{EINVAL} in some
-cases where the space provided for the return of the system parameter is too
-small.
-
-@end table
-
-@end deftypefun
-
-If you have a Linux kernel with the @code{proc} filesystem, you can get
-and set most of the same parameters by reading and writing to files in
-the @code{sys} directory of the @code{proc} filesystem.  In the @code{sys}
-directory, the directory structure represents the hierarchical structure
-of the parameters.  E.g. you can display the free page thresholds with
-@smallexample
-cat /proc/sys/vm/freepages
-@end smallexample
-@c In Linux, the sysctl() and /proc instances of the parameter are created
-@c together.  The proc filesystem accesses the same data structure as
-@c sysctl(), which has special fields in it for /proc.  But it is still
-@c possible to create a sysctl-only parameter.
-
-Some more traditional and more widely available, though less general,
-@glibcadj{} functions for getting and setting some of the same system
-parameters are:
-
-@itemize @bullet
-@item
-@code{getdomainname}, @code{setdomainname}
-@item
-@code{gethostname}, @code{sethostname} (@xref{Host Identification}.)
-@item
-@code{uname} (@xref{Platform Type}.)
-@end itemize
+The @code{sysctl} function is deprecated in favor of @code{proc} filesystem
+and @theglibc{} provides only symbol for backward compatibility.
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 8f19e0e..6737900 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -28,7 +28,7 @@ CFLAGS-open_by_handle_at.c = -fexceptions -fasynchronous-unwind-tables
 CFLAGS-sync_file_range.c = -fexceptions -fasynchronous-unwind-tables
 CFLAGS-tst-writev.c += "-DARTIFICIAL_LIMIT=(0x80000000-sysconf(_SC_PAGESIZE))"
 
-sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
+sysdep_headers += sys/mount.h sys/acct.h \
 		  sys/klog.h \
 		  sys/user.h sys/prctl.h \
 		  sys/kd.h sys/soundcard.h sys/vt.h \
@@ -38,7 +38,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
 		  bits/a.out.h sys/inotify.h sys/signalfd.h sys/eventfd.h \
 		  sys/timerfd.h sys/fanotify.h bits/eventfd.h bits/inotify.h \
 		  bits/signalfd.h bits/timerfd.h bits/epoll.h \
-		  bits/socket_type.h bits/syscall.h bits/sysctl.h \
+		  bits/socket_type.h bits/syscall.h \
 		  bits/mman-linux.h bits/mman-shared.h bits/ptrace-shared.h \
 		  bits/siginfo-arch.h bits/siginfo-consts-arch.h
 
diff --git a/sysdeps/unix/sysv/linux/arm/ioperm.c b/sysdeps/unix/sysv/linux/arm/ioperm.c
index 4c0bb63..0762475 100644
--- a/sysdeps/unix/sysv/linux/arm/ioperm.c
+++ b/sysdeps/unix/sysv/linux/arm/ioperm.c
@@ -61,14 +61,31 @@ static struct {
  */
 
 static int
+sysctl_call (int *name, int nlen, void *oldval, size_t *oldlenp,
+	     void *newval, size_t newlen)
+{
+  struct kernel_sysctl_args
+  {
+    int *name;
+    int nlen;
+    void *oldval;
+    size_t *oldlenp;
+    void *newval;
+    size_t newlen;
+    unsigned long __unused[4];
+  } args = { name, nlen, oldval, oldlenp, newval, newlen };
+  return INLINE_SYSCALL_CALL (_sysctl, &args);
+}
+
+static int
 init_iosys (void)
 {
   static int iobase_name[] = { CTL_BUS, CTL_BUS_ISA, BUS_ISA_PORT_BASE };
   static int ioshift_name[] = { CTL_BUS, CTL_BUS_ISA, BUS_ISA_PORT_SHIFT };
   size_t len = sizeof(io.base);
 
-  if (! __sysctl (iobase_name, 3, &io.io_base, &len, NULL, 0)
-      && ! __sysctl (ioshift_name, 3, &io.shift, &len, NULL, 0))
+  if (! sysctl_call (iobase_name, 3, &io.io_base, &len, NULL, 0)
+      && ! sysctl_call (ioshift_name, 3, &io.shift, &len, NULL, 0))
     {
       io.initdone = 1;
       return 0;
diff --git a/sysdeps/unix/sysv/linux/bits/sysctl.h b/sysdeps/unix/sysv/linux/bits/sysctl.h
deleted file mode 100644
index 81447b2..0000000
--- a/sysdeps/unix/sysv/linux/bits/sysctl.h
+++ /dev/null
@@ -1 +0,0 @@
-/* Empty file.  */
diff --git a/sysdeps/unix/sysv/linux/generic/sysctl.c b/sysdeps/unix/sysv/linux/generic/sysctl.c
deleted file mode 100644
index 61e7fa8..0000000
--- a/sysdeps/unix/sysv/linux/generic/sysctl.c
+++ /dev/null
@@ -1,32 +0,0 @@
-/* Copyright (C) 2011-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Chris Metcalf <cmetcalf@tilera.com>, 2011.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library.  If not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#include <errno.h>
-
-#include <sysdep.h>
-#include <sys/syscall.h>
-
-/* This deprecated syscall is no longer used (replaced with /proc/sys).  */
-int
-sysctl (int *name, int nlen, void *oldval, size_t *oldlenp,
-        void *newval, size_t newlen)
-{
-  __set_errno (ENOSYS);
-  return -1;
-}
-stub_warning (sysctl)
diff --git a/sysdeps/unix/sysv/linux/sys/sysctl.h b/sysdeps/unix/sysv/linux/sys/sysctl.h
deleted file mode 100644
index d125a83..0000000
--- a/sysdeps/unix/sysv/linux/sys/sysctl.h
+++ /dev/null
@@ -1,73 +0,0 @@
-/* Copyright (C) 1996-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#ifndef	_SYS_SYSCTL_H
-#define	_SYS_SYSCTL_H	1
-
-#include <features.h>
-#define __need_size_t
-#include <stddef.h>
-/* Prevent more kernel headers than necessary to be included.  */
-#ifndef _LINUX_KERNEL_H
-# define _LINUX_KERNEL_H	1
-# define __undef_LINUX_KERNEL_H
-#endif
-#ifndef _LINUX_TYPES_H
-# define _LINUX_TYPES_H		1
-# define __undef_LINUX_TYPES_H
-#endif
-#ifndef _LINUX_LIST_H
-# define _LINUX_LIST_H		1
-# define __undef_LINUX_LIST_H
-#endif
-#ifndef __LINUX_COMPILER_H
-# define __LINUX_COMPILER_H	1
-# define __user
-# define __undef__LINUX_COMPILER_H
-#endif
-
-#include <linux/sysctl.h>
-
-#ifdef __undef_LINUX_KERNEL_H
-# undef _LINUX_KERNEL_H
-# undef __undef_LINUX_KERNEL_H
-#endif
-#ifdef __undef_LINUX_TYPES_H
-# undef _LINUX_TYPES_H
-# undef __undef_LINUX_TYPES_H
-#endif
-#ifdef __undef_LINUX_LIST_H
-# undef _LINUX_LIST_H
-# undef __undef_LINUX_LIST_H
-#endif
-#ifdef __undef__LINUX_COMPILER_H
-# undef __LINUX_COMPILER_H
-# undef __user
-# undef __undef__LINUX_COMPILER_H
-#endif
-
-#include <bits/sysctl.h>
-
-__BEGIN_DECLS
-
-/* Read or write system parameters.  */
-extern int sysctl (int *__name, int __nlen, void *__oldval,
-		   size_t *__oldlenp, void *__newval, size_t __newlen) __THROW;
-
-__END_DECLS
-
-#endif	/* _SYS_SYSCTL_H */
diff --git a/sysdeps/unix/sysv/linux/sysctl.c b/sysdeps/unix/sysv/linux/sysctl.c
index bb5e31a..38a0081 100644
--- a/sysdeps/unix/sysv/linux/sysctl.c
+++ b/sysdeps/unix/sysv/linux/sysctl.c
@@ -16,18 +16,29 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
-#include <string.h>	/* For the real memset prototype.  */
-#include <sys/sysctl.h>
+#include <shlib-compat.h>
 
-#include <sysdep.h>
-#include <sys/syscall.h>
+/* This deprecated syscall is no longer used (replaced with /proc/sys).  */
+#if SHLIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_28)
+
+# include <unistd.h>
+# include <errno.h>
 
 int
-__sysctl (int *name, int nlen, void *oldval, size_t *oldlenp,
-	  void *newval, size_t newlen)
+__old_sysctl (int *name, int nlen, void *oldval, size_t *oldlenp,
+	      void *newval, size_t newlen)
 {
-  struct __sysctl_args args =
+# ifdef __NR__sysctl
+  struct kernel_sysctl_args
+  {
+    int *name;
+    int nlen;
+    void *oldval;
+    size_t *oldlenp;
+    void *newval;
+    size_t newlen;
+    unsigned long __unused[4];
+  } args =
   {
     .name = name,
     .nlen = nlen,
@@ -37,7 +48,13 @@ __sysctl (int *name, int nlen, void *oldval, size_t *oldlenp,
     .newlen = newlen
   };
 
-  return INLINE_SYSCALL (_sysctl, 1, &args);
+  return INLINE_SYSCALL_CALL (_sysctl, &args);
+# else
+  return INLINE_SYSCALL_ERROR_RETURN_VALUE (ENOSYS);
+# endif
 }
-libc_hidden_def (__sysctl)
-weak_alias (__sysctl, sysctl)
+compat_symbol (libc, __old_sysctl, sysctl, GLIBC_2_0);
+# ifdef __NR__sysctl
+strong_alias (__old_sysctl, __sysctl)
+# endif
+#endif
diff --git a/sysdeps/unix/sysv/linux/x86/bits/sysctl.h b/sysdeps/unix/sysv/linux/x86/bits/sysctl.h
deleted file mode 100644
index 8d76ed8..0000000
--- a/sysdeps/unix/sysv/linux/x86/bits/sysctl.h
+++ /dev/null
@@ -1,20 +0,0 @@
-/* Copyright (C) 2012-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#if defined __x86_64__ && defined __ILP32__
-# error "sysctl system call is unsupported in x32 kernel"
-#endif
-- 
2.7.4