| 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
| ||
|
Message-ID: <itlh63rhko3m7fqbq4l5hy7nmaiqywoizg4gr66bb3ylmzntjc@ezuwwti3roln>
Date: Wed, 15 Jan 2025 10:07:38 +0100
From: Joel Granados <joel.granados@...nel.org>
To: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
Cc: brauner@...nel.org, stgraber@...raber.org, tycho@...ho.pizza,
cyphar@...har.com, yun.zhou@...driver.com, rostedt@...dmis.org,
linux-kernel@...r.kernel.org, Christian Brauner <christian.brauner@...ntu.com>
Subject: Re: [PATCH v2 1/2] pid: allow pid_max to be set per pid namespace
On Fri, Nov 22, 2024 at 02:24:58PM +0100, Alexander Mikhalitsyn wrote:
> From: Christian Brauner <christian.brauner@...ntu.com>
>
> The pid_max sysctl is a global value. For a long time the default value
> has been 65535 and during the pidfd dicussions Linus proposed to bump
> pid_max by default (cf. [1]). Based on this discussion systemd started
> bumping pid_max to 2^22. So all new systems now run with a very high
> pid_max limit with some distros having also backported that change.
> The decision to bump pid_max is obviously correct. It just doesn't make
> a lot of sense nowadays to enforce such a low pid number. There's
> sufficient tooling to make selecting specific processes without typing
> really large pid numbers available.
...
> diff --git a/kernel/pid.c b/kernel/pid.c
> index 115448e89c3e..ce3e94e26a0f 100644
> --- a/kernel/pid.c
> +++ b/kernel/pid.c
> @@ -60,10 +60,8 @@ struct pid init_struct_pid = {
> }, }
> };
>
> -int pid_max = PID_MAX_DEFAULT;
> -
> -int pid_max_min = RESERVED_PIDS + 1;
> -int pid_max_max = PID_MAX_LIMIT;
> +static int pid_max_min = RESERVED_PIDS + 1;
> +static int pid_max_max = PID_MAX_LIMIT;
> /*
> * Pseudo filesystems start inode numbering after one. We use Reserved
> * PIDs as a natural offset.
> @@ -87,6 +85,7 @@ struct pid_namespace init_pid_ns = {
> #ifdef CONFIG_PID_NS
> .ns.ops = &pidns_operations,
> #endif
> + .pid_max = PID_MAX_DEFAULT,
> #if defined(CONFIG_SYSCTL) && defined(CONFIG_MEMFD_CREATE)
> .memfd_noexec_scope = MEMFD_NOEXEC_SCOPE_EXEC,
> #endif
> @@ -193,6 +192,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid,
>
> for (i = ns->level; i >= 0; i--) {
> int tid = 0;
> + int pid_max = READ_ONCE(tmp->pid_max);
>
> if (set_tid_size) {
> tid = set_tid[ns->level - i];
> @@ -644,17 +644,118 @@ SYSCALL_DEFINE2(pidfd_open, pid_t, pid, unsigned int, flags)
> return fd;
> }
>
> +#ifdef CONFIG_SYSCTL
> +static struct ctl_table_set *pid_table_root_lookup(struct ctl_table_root *root)
> +{
> + return &task_active_pid_ns(current)->set;
> +}
> +
> +static int set_is_seen(struct ctl_table_set *set)
> +{
> + return &task_active_pid_ns(current)->set == set;
> +}
> +
> +static int pid_table_root_permissions(struct ctl_table_header *head,
> + const struct ctl_table *table)
> +{
> + struct pid_namespace *pidns =
> + container_of(head->set, struct pid_namespace, set);
> + int mode = table->mode;
> +
> + if (ns_capable(pidns->user_ns, CAP_SYS_ADMIN) ||
> + uid_eq(current_euid(), make_kuid(pidns->user_ns, 0)))
> + mode = (mode & S_IRWXU) >> 6;
> + else if (in_egroup_p(make_kgid(pidns->user_ns, 0)))
> + mode = (mode & S_IRWXG) >> 3;
> + else
> + mode = mode & S_IROTH;
> + return (mode << 6) | (mode << 3) | mode;
> +}
> +
> +static void pid_table_root_set_ownership(struct ctl_table_header *head,
> + kuid_t *uid, kgid_t *gid)
> +{
> + struct pid_namespace *pidns =
> + container_of(head->set, struct pid_namespace, set);
> + kuid_t ns_root_uid;
> + kgid_t ns_root_gid;
> +
> + ns_root_uid = make_kuid(pidns->user_ns, 0);
> + if (uid_valid(ns_root_uid))
> + *uid = ns_root_uid;
> +
> + ns_root_gid = make_kgid(pidns->user_ns, 0);
> + if (gid_valid(ns_root_gid))
> + *gid = ns_root_gid;
> +}
> +
> +static struct ctl_table_root pid_table_root = {
> + .lookup = pid_table_root_lookup,
> + .permissions = pid_table_root_permissions,
> + .set_ownership = pid_table_root_set_ownership,
> +};
> +
> +static struct ctl_table pid_table[] = {
If possible, please make this const. Sorry I missed this in the original
review.
Best
> + {
> + .procname = "pid_max",
> + .data = &init_pid_ns.pid_max,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = &pid_max_min,
> + .extra2 = &pid_max_max,
> + },
> +};
> +#endif
> +
> +int register_pidns_sysctls(struct pid_namespace *pidns)
> +{
> +#ifdef CONFIG_SYSCTL
> + struct ctl_table *tbl;
> +
> + setup_sysctl_set(&pidns->set, &pid_table_root, set_is_seen);
> +
> + tbl = kmemdup(pid_table, sizeof(pid_table), GFP_KERNEL);
> + if (!tbl)
> + return -ENOMEM;
> + tbl->data = &pidns->pid_max;
> + pidns->pid_max = min(pid_max_max, max_t(int, pidns->pid_max,
> + PIDS_PER_CPU_DEFAULT * num_possible_cpus()));
> +
> + pidns->sysctls = __register_sysctl_table(&pidns->set, "kernel", tbl,
> + ARRAY_SIZE(pid_table));
> + if (!pidns->sysctls) {
> + kfree(tbl);
> + retire_sysctl_set(&pidns->set);
> + return -ENOMEM;
> + }
> +#endif
> + return 0;
> +}
> +
> +void unregister_pidns_sysctls(struct pid_namespace *pidns)
> +{
> +#ifdef CONFIG_SYSCTL
> + const struct ctl_table *tbl;
> +
> + tbl = pidns->sysctls->ctl_table_arg;
> + unregister_sysctl_table(pidns->sysctls);
> + retire_sysctl_set(&pidns->set);
> + kfree(tbl);
> +#endif
> +}
> +
> void __init pid_idr_init(void)
> {
> /* Verify no one has done anything silly: */
> BUILD_BUG_ON(PID_MAX_LIMIT >= PIDNS_ADDING);
>
> /* bump default and minimum pid_max based on number of cpus */
> - pid_max = min(pid_max_max, max_t(int, pid_max,
> - PIDS_PER_CPU_DEFAULT * num_possible_cpus()));
> + init_pid_ns.pid_max = min(pid_max_max, max_t(int, init_pid_ns.pid_max,
> + PIDS_PER_CPU_DEFAULT * num_possible_cpus()));
> pid_max_min = max_t(int, pid_max_min,
> PIDS_PER_CPU_MIN * num_possible_cpus());
> - pr_info("pid_max: default: %u minimum: %u\n", pid_max, pid_max_min);
> + pr_info("pid_max: default: %u minimum: %u\n", init_pid_ns.pid_max, pid_max_min);
>
> idr_init(&init_pid_ns.idr);
>
> @@ -665,6 +766,16 @@ void __init pid_idr_init(void)
> NULL);
> }
>
> +static __init int pid_namespace_sysctl_init(void)
> +{
> +#ifdef CONFIG_SYSCTL
> + /* "kernel" directory will have already been initialized. */
> + BUG_ON(register_pidns_sysctls(&init_pid_ns));
> +#endif
> + return 0;
> +}
> +subsys_initcall(pid_namespace_sysctl_init);
> +
> static struct file *__pidfd_fget(struct task_struct *task, int fd)
> {
> struct file *file;
...
--
Joel Granados
Powered by blists - more mailing lists