Commit 7ac592aa35a684ff ("sched: prctl() core-scheduling interface")
made use of enum pid_type in prctl's arg4; this type and the associated
enumeration definitions are not exposed to userspace. Try to fix that
by providing enum _kernel_pid_type and tying in-kernel enum pid_type
definitions to it. Note that enum pid_type cannot be exposed as is,
since "enum pid_type" is already exists in various projects [1] (notably
gcc and strace), and "enum __pid_type" is defined by glibc and uclibc
for fcntl(F_SETOWN_EX) owner ID type.
[1] https://codesearch.debian.net/search?q=enum+pid_type
Complements: 7ac592aa35a684ff ("sched: prctl() core-scheduling interface")
Signed-off-by: Eugene Syromiatnikov <[email protected]>
---
.../admin-guide/hw-vuln/core-scheduling.rst | 7 ++++---
include/linux/pid.h | 12 +++++++-----
include/uapi/linux/pid.h | 20 ++++++++++++++++++++
include/uapi/linux/prctl.h | 1 +
4 files changed, 32 insertions(+), 8 deletions(-)
create mode 100644 include/uapi/linux/pid.h
diff --git a/Documentation/admin-guide/hw-vuln/core-scheduling.rst b/Documentation/admin-guide/hw-vuln/core-scheduling.rst
index 7b410ae..3eb2b7c 100644
--- a/Documentation/admin-guide/hw-vuln/core-scheduling.rst
+++ b/Documentation/admin-guide/hw-vuln/core-scheduling.rst
@@ -61,9 +61,10 @@ arg3:
``pid`` of the task for which the operation applies.
arg4:
- ``pid_type`` for which the operation applies. It is of type ``enum pid_type``.
- For example, if arg4 is ``PIDTYPE_TGID``, then the operation of this command
- will be performed for all tasks in the task group of ``pid``.
+ ``pid_type`` for which the operation applies. It is of type
+ ``enum __kernel_pid_type``. For example, if arg4 is ``__PIDTYPE_TGID``,
+ then the operation of this command will be performed for all tasks
+ in the task group of ``pid``.
arg5:
userspace pointer to an unsigned long for storing the cookie returned by
diff --git a/include/linux/pid.h b/include/linux/pid.h
index fa10acb..f8ca4c9 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -5,14 +5,16 @@
#include <linux/rculist.h>
#include <linux/wait.h>
#include <linux/refcount.h>
+#include <uapi/linux/pid.h>
enum pid_type
{
- PIDTYPE_PID,
- PIDTYPE_TGID,
- PIDTYPE_PGID,
- PIDTYPE_SID,
- PIDTYPE_MAX,
+ PIDTYPE_PID = __PIDTYPE_PID,
+ PIDTYPE_TGID = __PIDTYPE_TGID,
+ PIDTYPE_PGID = __PIDTYPE_PGID,
+ PIDTYPE_SID = __PIDTYPE_SID,
+
+ PIDTYPE_MAX = __PIDTYPE_MAX
};
/*
diff --git a/include/uapi/linux/pid.h b/include/uapi/linux/pid.h
new file mode 100644
index 0000000..91d08e4
--- /dev/null
+++ b/include/uapi/linux/pid.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_LINUX_PID_H
+#define _UAPI_LINUX_PID_H
+
+/*
+ * Type of process-related ID. So far, it is used only for prctl(PR_SCHED_CORE);
+ * not to be confused with type field of f_owner_ex structure argument
+ * of fcntl(F_SETOWN_EX).
+ */
+enum __kernel_pid_type
+{
+ __PIDTYPE_PID,
+ __PIDTYPE_TGID,
+ __PIDTYPE_PGID,
+ __PIDTYPE_SID,
+
+ __PIDTYPE_MAX /* Non-UAPI */
+};
+
+#endif /* _UAPI_LINUX_PID_H */
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 967d9c5..4e794aa 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -3,6 +3,7 @@
#define _LINUX_PRCTL_H
#include <linux/types.h>
+#include <linux/pid.h> /* enum __kernel_pid_type */
/* Values to pass as first argument to prctl() */
--
2.1.4
Hi Eugene,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.14-rc4 next-20210806]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Eugene-Syromiatnikov/uapi-expose-enum-pid_type/20210807-090303
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git c9194f32bfd932e976a158d1af97a63be68a2aab
config: arm64-randconfig-r025-20210804 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/2bbca57c66637caced712672da1cce4441107353
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Eugene-Syromiatnikov/uapi-expose-enum-pid_type/20210807-090303
git checkout 2bbca57c66637caced712672da1cce4441107353
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
In file included from include/linux/pid.h:5,
from include/uapi/linux/prctl.h:6,
from arch/arm64/include/asm/pointer_auth.h:6,
from arch/arm64/include/asm/processor.h:36,
from include/linux/rcupdate.h:30,
from include/linux/percpu-refcount.h:56,
from include/linux/memremap.h:6,
from drivers/dax/bus.c:3:
include/linux/rculist.h: In function '__list_add_rcu':
>> include/linux/rculist.h:93:2: error: implicit declaration of function 'rcu_assign_pointer' [-Werror=implicit-function-declaration]
93 | rcu_assign_pointer(list_next_rcu(prev), new);
| ^~~~~~~~~~~~~~~~~~
In file included from include/linux/string.h:262,
from include/linux/bitmap.h:10,
from include/linux/cpumask.h:12,
from include/linux/smp.h:13,
from include/linux/percpu.h:7,
from include/linux/percpu-refcount.h:55,
from include/linux/memremap.h:6,
from drivers/dax/bus.c:3:
In function 'strncpy',
inlined from 'do_id_store' at drivers/dax/bus.c:89:5:
include/linux/fortify-string.h:27:30: warning: '__builtin_strncpy' specified bound 30 equals destination size [-Wstringop-truncation]
27 | #define __underlying_strncpy __builtin_strncpy
| ^
include/linux/fortify-string.h:38:9: note: in expansion of macro '__underlying_strncpy'
38 | return __underlying_strncpy(p, q, size);
| ^~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from include/uapi/linux/prctl.h:6,
from arch/arm64/include/asm/pointer_auth.h:6,
from arch/arm64/include/asm/processor.h:36,
from include/asm-generic/qrwlock.h:14,
from ./arch/arm64/include/generated/asm/qrwlock.h:1,
from arch/arm64/include/asm/spinlock.h:9,
from include/linux/spinlock.h:90,
from include/linux/wait.h:9,
from include/linux/wait_bit.h:8,
from include/linux/fs.h:6,
from include/linux/debugfs.h:15,
from kernel/dma/pool.c:7:
>> include/linux/pid.h:70:2: error: unknown type name 'wait_queue_head_t'
70 | wait_queue_head_t wait_pidfd;
| ^~~~~~~~~~~~~~~~~
vim +/rcu_assign_pointer +93 include/linux/rculist.h
28875945ba98d1 Joel Fernandes (Google 2019-07-16 78)
82524746c27fa4 Franck Bui-Huu 2008-05-12 79 /*
82524746c27fa4 Franck Bui-Huu 2008-05-12 80 * Insert a new entry between two known consecutive entries.
82524746c27fa4 Franck Bui-Huu 2008-05-12 81 *
82524746c27fa4 Franck Bui-Huu 2008-05-12 82 * This is only for internal list manipulation where we know
82524746c27fa4 Franck Bui-Huu 2008-05-12 83 * the prev/next entries already!
82524746c27fa4 Franck Bui-Huu 2008-05-12 84 */
82524746c27fa4 Franck Bui-Huu 2008-05-12 85 static inline void __list_add_rcu(struct list_head *new,
82524746c27fa4 Franck Bui-Huu 2008-05-12 86 struct list_head *prev, struct list_head *next)
82524746c27fa4 Franck Bui-Huu 2008-05-12 87 {
54acd4397d7e7a Kees Cook 2016-08-17 88 if (!__list_add_valid(new, prev, next))
54acd4397d7e7a Kees Cook 2016-08-17 89 return;
54acd4397d7e7a Kees Cook 2016-08-17 90
82524746c27fa4 Franck Bui-Huu 2008-05-12 91 new->next = next;
82524746c27fa4 Franck Bui-Huu 2008-05-12 92 new->prev = prev;
67bdbffd696f29 Arnd Bergmann 2010-02-25 @93 rcu_assign_pointer(list_next_rcu(prev), new);
82524746c27fa4 Franck Bui-Huu 2008-05-12 94 next->prev = new;
82524746c27fa4 Franck Bui-Huu 2008-05-12 95 }
82524746c27fa4 Franck Bui-Huu 2008-05-12 96
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Sat, Aug 07, 2021 at 03:01:23AM +0200, Eugene Syromiatnikov wrote:
> Commit 7ac592aa35a684ff ("sched: prctl() core-scheduling interface")
> made use of enum pid_type in prctl's arg4; this type and the associated
(Hm, I thought we were trying to stop exposing the thread-group leader
concept directly to userspace. But that ship probably has sailed now.)
> + __PIDTYPE_PID,
> + __PIDTYPE_TGID,
> + __PIDTYPE_PGID,
What's rather odd is that the interface seems to not be using
PIDTYPE_PID, PIDTYPE_TGID, or PIDTYPE_PGID to indicate the type of the
pid but rather the scope of the operation which conflicts with the
names in the enum.
So I think exposing these values as PIDTYPE_* is misleading as it
doesn't really apply to the passed in pid. For example, if I pass in a
non-thread group leader and specify PIDTYPE_TGID then this interface
goes and digs out the thread-group leader from signal->pids when
arguably it should've failed because I passed in a non-thread-group
leader pid. So I think we shouldn't expose the enum with __PIDTYPE_*
prefix.
I think there's two ways. First, the prctl() would need to change a bit
and then we could use the P_PID, P_PGID definitions from wait.h that we
already expose to userspace (adding a P_TGID). But that would mean a
uapi change for the prctl() and I've never really liked those defines
anyway as they are equally strangely named.
So I would suggest we expose a custom enum or a set of defines in
prctl.h. Like (sorry, probably terrible names):
#define PR_SCHED_CORE_SCOPE_THREAD
#define PR_SCHED_CORE_SCOPE_THREADGROUP
#define PR_SCHED_CORE_SCOPE_PROCESSGROUP
that have the same values as PIDTYPE_* but express clearly that this
indicates the scope of the operation independent of whether or not the
passed in pid is a thread-group leader, process-group leader or not.
Thoughts?
Christian
> enumeration definitions are not exposed to userspace. Try to fix that
> by providing enum _kernel_pid_type and tying in-kernel enum pid_type
> definitions to it. Note that enum pid_type cannot be exposed as is,
> since "enum pid_type" is already exists in various projects [1] (notably
> gcc and strace), and "enum __pid_type" is defined by glibc and uclibc
> for fcntl(F_SETOWN_EX) owner ID type.
>
> [1] https://codesearch.debian.net/search?q=enum+pid_type
>
> Complements: 7ac592aa35a684ff ("sched: prctl() core-scheduling interface")
> Signed-off-by: Eugene Syromiatnikov <[email protected]>
> ---
> .../admin-guide/hw-vuln/core-scheduling.rst | 7 ++++---
> include/linux/pid.h | 12 +++++++-----
> include/uapi/linux/pid.h | 20 ++++++++++++++++++++
> include/uapi/linux/prctl.h | 1 +
> 4 files changed, 32 insertions(+), 8 deletions(-)
> create mode 100644 include/uapi/linux/pid.h
>
> diff --git a/Documentation/admin-guide/hw-vuln/core-scheduling.rst b/Documentation/admin-guide/hw-vuln/core-scheduling.rst
> index 7b410ae..3eb2b7c 100644
> --- a/Documentation/admin-guide/hw-vuln/core-scheduling.rst
> +++ b/Documentation/admin-guide/hw-vuln/core-scheduling.rst
> @@ -61,9 +61,10 @@ arg3:
> ``pid`` of the task for which the operation applies.
>
> arg4:
> - ``pid_type`` for which the operation applies. It is of type ``enum pid_type``.
> - For example, if arg4 is ``PIDTYPE_TGID``, then the operation of this command
> - will be performed for all tasks in the task group of ``pid``.
> + ``pid_type`` for which the operation applies. It is of type
> + ``enum __kernel_pid_type``. For example, if arg4 is ``__PIDTYPE_TGID``,
> + then the operation of this command will be performed for all tasks
> + in the task group of ``pid``.
>
> arg5:
> userspace pointer to an unsigned long for storing the cookie returned by
> diff --git a/include/linux/pid.h b/include/linux/pid.h
> index fa10acb..f8ca4c9 100644
> --- a/include/linux/pid.h
> +++ b/include/linux/pid.h
> @@ -5,14 +5,16 @@
> #include <linux/rculist.h>
> #include <linux/wait.h>
> #include <linux/refcount.h>
> +#include <uapi/linux/pid.h>
>
> enum pid_type
> {
> - PIDTYPE_PID,
> - PIDTYPE_TGID,
> - PIDTYPE_PGID,
> - PIDTYPE_SID,
> - PIDTYPE_MAX,
> + PIDTYPE_PID = __PIDTYPE_PID,
> + PIDTYPE_TGID = __PIDTYPE_TGID,
> + PIDTYPE_PGID = __PIDTYPE_PGID,
> + PIDTYPE_SID = __PIDTYPE_SID,
> +
> + PIDTYPE_MAX = __PIDTYPE_MAX
> };
>
> /*
> diff --git a/include/uapi/linux/pid.h b/include/uapi/linux/pid.h
> new file mode 100644
> index 0000000..91d08e4
> --- /dev/null
> +++ b/include/uapi/linux/pid.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _UAPI_LINUX_PID_H
> +#define _UAPI_LINUX_PID_H
> +
> +/*
> + * Type of process-related ID. So far, it is used only for prctl(PR_SCHED_CORE);
> + * not to be confused with type field of f_owner_ex structure argument
> + * of fcntl(F_SETOWN_EX).
> + */
> +enum __kernel_pid_type
> +{
> + __PIDTYPE_PID,
> + __PIDTYPE_TGID,
> + __PIDTYPE_PGID,
> + __PIDTYPE_SID,
> +
> + __PIDTYPE_MAX /* Non-UAPI */
> +};
> +
> +#endif /* _UAPI_LINUX_PID_H */
> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index 967d9c5..4e794aa 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -3,6 +3,7 @@
> #define _LINUX_PRCTL_H
>
> #include <linux/types.h>
> +#include <linux/pid.h> /* enum __kernel_pid_type */
>
> /* Values to pass as first argument to prctl() */
>
> --
> 2.1.4
>