275 lines
9.6 KiB
Diff
275 lines
9.6 KiB
Diff
From aac205646b22c60a35eeb810537df406a0ce2c65 Mon Sep 17 00:00:00 2001
|
|
From: Rafael Aquini <raquini@redhat.com>
|
|
Date: Wed, 1 Jul 2026 16:38:58 -0400
|
|
Subject: [PATCH] procfs: avoid fetching build ID while holding VMA lock
|
|
|
|
JIRA: https://redhat.atlassian.net/browse/RHEL-189666
|
|
CVE: CVE-2026-23199
|
|
Conflicts:
|
|
* fs/proc/task_mmu.c: minor differences in the hunks due to RHEL-10 yet not
|
|
utilizing granular per-VMA lock for PROCMAP_QUERY and instead relying on
|
|
the full-fledged mmap_lock semaphore for sinchronization. The convertion
|
|
of PROCMAP_QUERY to per-VMA locks is v6.18 material and its imposed churn
|
|
is not required nor it is a necessary condition for the correction of this
|
|
backport.
|
|
|
|
commit b5cbacd7f86f4f62b8813688c8e73be94e8e1951
|
|
Author: Andrii Nakryiko <andrii@kernel.org>
|
|
Date: Thu Jan 29 13:53:40 2026 -0800
|
|
|
|
procfs: avoid fetching build ID while holding VMA lock
|
|
|
|
Fix PROCMAP_QUERY to fetch optional build ID only after dropping mmap_lock
|
|
or per-VMA lock, whichever was used to lock VMA under question, to avoid
|
|
deadlock reported by syzbot:
|
|
|
|
-> #1 (&mm->mmap_lock){++++}-{4:4}:
|
|
__might_fault+0xed/0x170
|
|
_copy_to_iter+0x118/0x1720
|
|
copy_page_to_iter+0x12d/0x1e0
|
|
filemap_read+0x720/0x10a0
|
|
blkdev_read_iter+0x2b5/0x4e0
|
|
vfs_read+0x7f4/0xae0
|
|
ksys_read+0x12a/0x250
|
|
do_syscall_64+0xcb/0xf80
|
|
entry_SYSCALL_64_after_hwframe+0x77/0x7f
|
|
|
|
-> #0 (&sb->s_type->i_mutex_key#8){++++}-{4:4}:
|
|
__lock_acquire+0x1509/0x26d0
|
|
lock_acquire+0x185/0x340
|
|
down_read+0x98/0x490
|
|
blkdev_read_iter+0x2a7/0x4e0
|
|
__kernel_read+0x39a/0xa90
|
|
freader_fetch+0x1d5/0xa80
|
|
__build_id_parse.isra.0+0xea/0x6a0
|
|
do_procmap_query+0xd75/0x1050
|
|
procfs_procmap_ioctl+0x7a/0xb0
|
|
__x64_sys_ioctl+0x18e/0x210
|
|
do_syscall_64+0xcb/0xf80
|
|
entry_SYSCALL_64_after_hwframe+0x77/0x7f
|
|
|
|
other info that might help us debug this:
|
|
|
|
Possible unsafe locking scenario:
|
|
|
|
CPU0 CPU1
|
|
---- ----
|
|
rlock(&mm->mmap_lock);
|
|
lock(&sb->s_type->i_mutex_key#8);
|
|
lock(&mm->mmap_lock);
|
|
rlock(&sb->s_type->i_mutex_key#8);
|
|
|
|
*** DEADLOCK ***
|
|
|
|
This seems to be exacerbated (as we haven't seen these syzbot reports
|
|
before that) by the recent:
|
|
|
|
777a8560fd29 ("lib/buildid: use __kernel_read() for sleepable context")
|
|
|
|
To make this safe, we need to grab file refcount while VMA is still locked, but
|
|
other than that everything is pretty straightforward. Internal build_id_parse()
|
|
API assumes VMA is passed, but it only needs the underlying file reference, so
|
|
just add another variant build_id_parse_file() that expects file passed
|
|
directly.
|
|
|
|
[akpm@linux-foundation.org: fix up kerneldoc]
|
|
Link: https://lkml.kernel.org/r/20260129215340.3742283-1-andrii@kernel.org
|
|
Fixes: ed5d583a88a9 ("fs/procfs: implement efficient VMA querying API for /proc/<pid>/maps")
|
|
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
|
|
Reported-by: <syzbot+4e70c8e0a2017b432f7a@syzkaller.appspotmail.com>
|
|
Reviewed-by: Suren Baghdasaryan <surenb@google.com>
|
|
Tested-by: Suren Baghdasaryan <surenb@google.com>
|
|
Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>
|
|
Cc: Alexei Starovoitov <ast@kernel.org>
|
|
Cc: Daniel Borkmann <daniel@iogearbox.net>
|
|
Cc: Eduard Zingerman <eddyz87@gmail.com>
|
|
Cc: Hao Luo <haoluo@google.com>
|
|
Cc: Jiri Olsa <jolsa@kernel.org>
|
|
Cc: John Fastabend <john.fastabend@gmail.com>
|
|
Cc: KP Singh <kpsingh@kernel.org>
|
|
Cc: Martin KaFai Lau <martin.lau@linux.dev>
|
|
Cc: Song Liu <song@kernel.org>
|
|
Cc: Stanislav Fomichev <sdf@fomichev.me>
|
|
Cc: Yonghong Song <yonghong.song@linux.dev>
|
|
Cc: <stable@vger.kernel.org>
|
|
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
|
|
Signed-off-by: Rafael Aquini <raquini@redhat.com>
|
|
|
|
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
|
|
index 4cb8583..27cc491 100644
|
|
--- a/fs/proc/task_mmu.c
|
|
+++ b/fs/proc/task_mmu.c
|
|
@@ -456,6 +456,7 @@ static int do_procmap_query(struct proc_maps_private *priv, void __user *uarg)
|
|
struct procmap_query karg;
|
|
struct vm_area_struct *vma;
|
|
struct mm_struct *mm;
|
|
+ struct file *vm_file = NULL;
|
|
const char *name = NULL;
|
|
char build_id_buf[BUILD_ID_SIZE_MAX], *name_buf = NULL;
|
|
__u64 usize;
|
|
@@ -528,21 +529,6 @@ static int do_procmap_query(struct proc_maps_private *priv, void __user *uarg)
|
|
karg.inode = 0;
|
|
}
|
|
|
|
- if (karg.build_id_size) {
|
|
- __u32 build_id_sz;
|
|
-
|
|
- err = build_id_parse(vma, build_id_buf, &build_id_sz);
|
|
- if (err) {
|
|
- karg.build_id_size = 0;
|
|
- } else {
|
|
- if (karg.build_id_size < build_id_sz) {
|
|
- err = -ENAMETOOLONG;
|
|
- goto out;
|
|
- }
|
|
- karg.build_id_size = build_id_sz;
|
|
- }
|
|
- }
|
|
-
|
|
if (karg.vma_name_size) {
|
|
size_t name_buf_sz = min_t(size_t, PATH_MAX, karg.vma_name_size);
|
|
const struct path *path;
|
|
@@ -576,10 +562,34 @@ static int do_procmap_query(struct proc_maps_private *priv, void __user *uarg)
|
|
karg.vma_name_size = name_sz;
|
|
}
|
|
|
|
+ if (karg.build_id_size && vma->vm_file)
|
|
+ vm_file = get_file(vma->vm_file);
|
|
+
|
|
/* unlock vma or mmap_lock, and put mm_struct before copying data to user */
|
|
query_vma_teardown(mm, vma);
|
|
mmput(mm);
|
|
|
|
+ if (karg.build_id_size) {
|
|
+ __u32 build_id_sz;
|
|
+
|
|
+ if (vm_file)
|
|
+ err = build_id_parse_file(vm_file, build_id_buf, &build_id_sz);
|
|
+ else
|
|
+ err = -ENOENT;
|
|
+ if (err) {
|
|
+ karg.build_id_size = 0;
|
|
+ } else {
|
|
+ if (karg.build_id_size < build_id_sz) {
|
|
+ err = -ENAMETOOLONG;
|
|
+ goto out;
|
|
+ }
|
|
+ karg.build_id_size = build_id_sz;
|
|
+ }
|
|
+ }
|
|
+
|
|
+ if (vm_file)
|
|
+ fput(vm_file);
|
|
+
|
|
if (karg.vma_name_size && copy_to_user(u64_to_user_ptr(karg.vma_name_addr),
|
|
name, karg.vma_name_size)) {
|
|
kfree(name_buf);
|
|
@@ -599,6 +609,8 @@ static int do_procmap_query(struct proc_maps_private *priv, void __user *uarg)
|
|
out:
|
|
query_vma_teardown(mm, vma);
|
|
mmput(mm);
|
|
+ if (vm_file)
|
|
+ fput(vm_file);
|
|
kfree(name_buf);
|
|
return err;
|
|
}
|
|
diff --git a/include/linux/buildid.h b/include/linux/buildid.h
|
|
index 831c1b4..7acc06b 100644
|
|
--- a/include/linux/buildid.h
|
|
+++ b/include/linux/buildid.h
|
|
@@ -7,7 +7,10 @@
|
|
#define BUILD_ID_SIZE_MAX 20
|
|
|
|
struct vm_area_struct;
|
|
+struct file;
|
|
+
|
|
int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id, __u32 *size);
|
|
+int build_id_parse_file(struct file *file, unsigned char *build_id, __u32 *size);
|
|
int build_id_parse_nofault(struct vm_area_struct *vma, unsigned char *build_id, __u32 *size);
|
|
int build_id_parse_buf(const void *buf, unsigned char *build_id, u32 buf_size);
|
|
|
|
diff --git a/lib/buildid.c b/lib/buildid.c
|
|
index 8183310..c4b7376 100644
|
|
--- a/lib/buildid.c
|
|
+++ b/lib/buildid.c
|
|
@@ -279,7 +279,7 @@ static int get_build_id_64(struct freader *r, unsigned char *build_id, __u32 *si
|
|
/* enough for Elf64_Ehdr, Elf64_Phdr, and all the smaller requests */
|
|
#define MAX_FREADER_BUF_SZ 64
|
|
|
|
-static int __build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
|
|
+static int __build_id_parse(struct file *file, unsigned char *build_id,
|
|
__u32 *size, bool may_fault)
|
|
{
|
|
const Elf32_Ehdr *ehdr;
|
|
@@ -287,11 +287,7 @@ static int __build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
|
|
char buf[MAX_FREADER_BUF_SZ];
|
|
int ret;
|
|
|
|
- /* only works for page backed storage */
|
|
- if (!vma->vm_file)
|
|
- return -EINVAL;
|
|
-
|
|
- freader_init_from_file(&r, buf, sizeof(buf), vma->vm_file, may_fault);
|
|
+ freader_init_from_file(&r, buf, sizeof(buf), file, may_fault);
|
|
|
|
/* fetch first 18 bytes of ELF header for checks */
|
|
ehdr = freader_fetch(&r, 0, offsetofend(Elf32_Ehdr, e_type));
|
|
@@ -319,8 +315,8 @@ static int __build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
|
|
return ret;
|
|
}
|
|
|
|
-/*
|
|
- * Parse build ID of ELF file mapped to vma
|
|
+/**
|
|
+ * build_id_parse_nofault() - Parse build ID of ELF file mapped to vma
|
|
* @vma: vma object
|
|
* @build_id: buffer to store build id, at least BUILD_ID_SIZE long
|
|
* @size: returns actual build id size in case of success
|
|
@@ -332,11 +328,14 @@ static int __build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
|
|
*/
|
|
int build_id_parse_nofault(struct vm_area_struct *vma, unsigned char *build_id, __u32 *size)
|
|
{
|
|
- return __build_id_parse(vma, build_id, size, false /* !may_fault */);
|
|
+ if (!vma->vm_file)
|
|
+ return -EINVAL;
|
|
+
|
|
+ return __build_id_parse(vma->vm_file, build_id, size, false /* !may_fault */);
|
|
}
|
|
|
|
-/*
|
|
- * Parse build ID of ELF file mapped to VMA
|
|
+/**
|
|
+ * build_id_parse() - Parse build ID of ELF file mapped to VMA
|
|
* @vma: vma object
|
|
* @build_id: buffer to store build id, at least BUILD_ID_SIZE long
|
|
* @size: returns actual build id size in case of success
|
|
@@ -348,7 +347,26 @@ int build_id_parse_nofault(struct vm_area_struct *vma, unsigned char *build_id,
|
|
*/
|
|
int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id, __u32 *size)
|
|
{
|
|
- return __build_id_parse(vma, build_id, size, true /* may_fault */);
|
|
+ if (!vma->vm_file)
|
|
+ return -EINVAL;
|
|
+
|
|
+ return __build_id_parse(vma->vm_file, build_id, size, true /* may_fault */);
|
|
+}
|
|
+
|
|
+/**
|
|
+ * build_id_parse_file() - Parse build ID of ELF file
|
|
+ * @file: file object
|
|
+ * @build_id: buffer to store build id, at least BUILD_ID_SIZE long
|
|
+ * @size: returns actual build id size in case of success
|
|
+ *
|
|
+ * Assumes faultable context and can cause page faults to bring in file data
|
|
+ * into page cache.
|
|
+ *
|
|
+ * Return: 0 on success; negative error, otherwise
|
|
+ */
|
|
+int build_id_parse_file(struct file *file, unsigned char *build_id, __u32 *size)
|
|
+{
|
|
+ return __build_id_parse(file, build_id, size, true /* may_fault */);
|
|
}
|
|
|
|
/**
|