All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Christian Brauner <brauner@kernel.org>
Cc: Kees Cook <keescook@chromium.org>, Jan Kara <jack@suse.cz>,
	Andreas Gruenbacher <agruenba@redhat.com>,
	Daniel Verkamp <dverkamp@chromium.org>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Jeff Xu <jeffxu@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Frederic Weisbecker <frederic@kernel.org>,
	syzbot+ac3b41786a2d0565b6d5@syzkaller.appspotmail.com,
	linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
Subject: [PATCH] pid: Replace struct pid 1-element array with flex-array
Date: Wed, 17 May 2023 15:58:42 -0700	[thread overview]
Message-ID: <20230517225838.never.965-kees@kernel.org> (raw)

For pid namespaces, struct pid uses a dynamically sized array member,
"numbers". This was implemented using the ancient 1-element fake flexible
array, which has been deprecated for decades. Replace it with a C99
flexible array, refactor the array size calculations to use struct_size(),
and address elements via indexes. Note that the static initializer (which
defines a single element) works as-is, and requires no special handling.

Without this, CONFIG_UBSAN_BOUNDS (and potentially CONFIG_FORTIFY_SOURCE)
will trigger bounds checks:
https://lore.kernel.org/lkml/20230517-bushaltestelle-super-e223978c1ba6@brauner

Cc: Christian Brauner <brauner@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Andreas Gruenbacher <agruenba@redhat.com>
Cc: Daniel Verkamp <dverkamp@chromium.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Jeff Xu <jeffxu@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Frederic Weisbecker <frederic@kernel.org>
Reported-by: syzbot+ac3b41786a2d0565b6d5@syzkaller.appspotmail.com
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/pid.h    |  2 +-
 kernel/pid.c           | 13 ++++++++-----
 kernel/pid_namespace.c |  2 +-
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/linux/pid.h b/include/linux/pid.h
index b75de288a8c2..653a527574c4 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -67,7 +67,7 @@ struct pid
 	/* wait queue for pidfd notifications */
 	wait_queue_head_t wait_pidfd;
 	struct rcu_head rcu;
-	struct upid numbers[1];
+	struct upid numbers[];
 };
 
 extern struct pid init_struct_pid;
diff --git a/kernel/pid.c b/kernel/pid.c
index f93954a0384d..f5008146e2e4 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -131,7 +131,7 @@ void free_pid(struct pid *pid)
 
 	spin_lock_irqsave(&pidmap_lock, flags);
 	for (i = 0; i <= pid->level; i++) {
-		struct upid *upid = pid->numbers + i;
+		struct upid *upid = &pid->numbers[i];
 		struct pid_namespace *ns = upid->ns;
 		switch (--ns->pid_allocated) {
 		case 2:
@@ -265,7 +265,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid,
 	init_waitqueue_head(&pid->wait_pidfd);
 	INIT_HLIST_HEAD(&pid->inodes);
 
-	upid = pid->numbers + ns->level;
+	upid = &pid->numbers[ns->level];
 	spin_lock_irq(&pidmap_lock);
 	if (!(ns->pid_allocated & PIDNS_ADDING))
 		goto out_unlock;
@@ -285,7 +285,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid,
 out_free:
 	spin_lock_irq(&pidmap_lock);
 	while (++i <= ns->level) {
-		upid = pid->numbers + i;
+		upid = &pid->numbers[i];
 		idr_remove(&upid->ns->idr, upid->nr);
 	}
 
@@ -656,8 +656,11 @@ void __init pid_idr_init(void)
 
 	idr_init(&init_pid_ns.idr);
 
-	init_pid_ns.pid_cachep = KMEM_CACHE(pid,
-			SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT);
+	init_pid_ns.pid_cachep = kmem_cache_create("pid",
+			struct_size((struct pid *)0, numbers, 1),
+			__alignof__(struct pid),
+			SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT,
+			NULL);
 }
 
 static struct file *__pidfd_fget(struct task_struct *task, int fd)
diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index b43eee07b00c..ebb20091a9da 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -48,7 +48,7 @@ static struct kmem_cache *create_pid_cachep(unsigned int level)
 		return kc;
 
 	snprintf(name, sizeof(name), "pid_%u", level + 1);
-	len = sizeof(struct pid) + level * sizeof(struct upid);
+	len = struct_size((struct pid *)0, numbers, level + 1);
 	mutex_lock(&pid_caches_mutex);
 	/* Name collision forces to do allocation under mutex. */
 	if (!*pkc)
-- 
2.34.1


             reply	other threads:[~2023-05-17 22:58 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-17 22:58 Kees Cook [this message]
2023-05-18 14:48 ` [PATCH] pid: Replace struct pid 1-element array with flex-array Christian Brauner
2023-05-18 15:07 ` Christian Brauner
2023-05-30 14:44   ` Jeff Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230517225838.never.965-kees@kernel.org \
    --to=keescook@chromium.org \
    --cc=agruenba@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=boqun.feng@gmail.com \
    --cc=brauner@kernel.org \
    --cc=dverkamp@chromium.org \
    --cc=frederic@kernel.org \
    --cc=jack@suse.cz \
    --cc=jeffxu@google.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=syzbot+ac3b41786a2d0565b6d5@syzkaller.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.