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 v2] pid: Replace struct pid 1-element array with flex-array
Date: Fri, 30 Jun 2023 11:04:22 -0700 [thread overview]
Message-ID: <20230630180418.gonna.286-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 when entering a pid namespace:
https://lore.kernel.org/lkml/20230517-bushaltestelle-super-e223978c1ba6@brauner
For example: unshare --fork --pid --mount-proc readlink /proc/self
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
Closes: https://lore.kernel.org/lkml/000000000000c6de2a05fbdecbbb@google.com/
Acked-by: Jeff Xu <jeffxu@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
v2:
- drop pointer math to array index conversions (torvalds)
- use struct_size_t now that it exists (torvalds)
- updated commit log with reproducer example
v1: https://lore.kernel.org/lkml/20230517225838.never.965-kees@kernel.org/
---
include/linux/pid.h | 2 +-
kernel/pid.c | 7 +++++--
kernel/pid_namespace.c | 2 +-
3 files changed, 7 insertions(+), 4 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..6a1d23a11026 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -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_t(struct pid, 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..0bf44afe04dd 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_t(struct pid, numbers, level + 1);
mutex_lock(&pid_caches_mutex);
/* Name collision forces to do allocation under mutex. */
if (!*pkc)
--
2.34.1
next reply other threads:[~2023-06-30 18:04 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-30 18:04 Kees Cook [this message]
2023-06-30 19:00 ` [PATCH v2] pid: Replace struct pid 1-element array with flex-array Gustavo A. R. Silva
2023-07-01 6:24 ` Christian Brauner
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=20230630180418.gonna.286-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.