* [PATCH mm-stable] procfs: avoid fetching build ID while holding VMA lock
@ 2026-01-28 18:32 Andrii Nakryiko
2026-01-28 18:50 ` Andrew Morton
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2026-01-28 18:32 UTC (permalink / raw)
To: akpm, linux-mm
Cc: linux-fsdevel, bpf, surenb, Andrii Nakryiko,
syzbot+4e70c8e0a2017b432f7a
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 ***
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.
Fixes: ed5d583a88a9 ("fs/procfs: implement efficient VMA querying API for /proc/<pid>/maps")
Reported-by: syzbot+4e70c8e0a2017b432f7a@syzkaller.appspotmail.com
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
fs/proc/task_mmu.c | 42 ++++++++++++++++++++++++++---------------
include/linux/buildid.h | 3 +++
lib/buildid.c | 34 +++++++++++++++++++++++++--------
3 files changed, 56 insertions(+), 23 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 480db575553e..dd3b5cf9f0b7 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -656,6 +656,7 @@ static int do_procmap_query(struct mm_struct *mm, void __user *uarg)
struct proc_maps_locking_ctx lock_ctx = { .mm = mm };
struct procmap_query karg;
struct vm_area_struct *vma;
+ struct file *vm_file = NULL;
const char *name = NULL;
char build_id_buf[BUILD_ID_SIZE_MAX], *name_buf = NULL;
__u64 usize;
@@ -727,21 +728,6 @@ static int do_procmap_query(struct mm_struct *mm, 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;
@@ -775,10 +761,34 @@ static int do_procmap_query(struct mm_struct *mm, 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(&lock_ctx);
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);
@@ -798,6 +808,8 @@ static int do_procmap_query(struct mm_struct *mm, void __user *uarg)
out:
query_vma_teardown(&lock_ctx);
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 831c1b4b626c..7acc06b22fb7 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 818331051afe..dc643a6293c1 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));
@@ -332,7 +328,10 @@ 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 */);
}
/*
@@ -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 */);
+}
+
+/*
+ * Parse build ID of ELF file
+ * @vma: 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 */);
}
/**
--
2.47.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH mm-stable] procfs: avoid fetching build ID while holding VMA lock
2026-01-28 18:32 [PATCH mm-stable] procfs: avoid fetching build ID while holding VMA lock Andrii Nakryiko
@ 2026-01-28 18:50 ` Andrew Morton
2026-01-28 21:30 ` Andrii Nakryiko
2026-01-29 5:50 ` Suren Baghdasaryan
2026-01-29 15:52 ` Yonghong Song
2 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2026-01-28 18:50 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: linux-mm, linux-fsdevel, bpf, surenb, syzbot+4e70c8e0a2017b432f7a
On Wed, 28 Jan 2026 10:32:32 -0800 Andrii Nakryiko <andrii@kernel.org> wrote:
> 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:
>
> ...
>
> 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.
>
> Fixes: ed5d583a88a9 ("fs/procfs: implement efficient VMA querying API for /proc/<pid>/maps")
> Reported-by: syzbot+4e70c8e0a2017b432f7a@syzkaller.appspotmail.com
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Thanks. ed5d583a88a9 was 6 months ago so I assume this isn't super
urgent, so it needn't be rushed into mainline via mm.git's hotfixes
queue.
To provide for additional review and test time I'll queue the fix for
the upcoming merge window (Feb 18 upstream merge) with a cc:stable.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH mm-stable] procfs: avoid fetching build ID while holding VMA lock
2026-01-28 18:50 ` Andrew Morton
@ 2026-01-28 21:30 ` Andrii Nakryiko
2026-01-29 23:28 ` Andrew Morton
0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2026-01-28 21:30 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrii Nakryiko, linux-mm, linux-fsdevel, bpf, surenb,
syzbot+4e70c8e0a2017b432f7a
On Wed, Jan 28, 2026 at 10:50 AM Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> On Wed, 28 Jan 2026 10:32:32 -0800 Andrii Nakryiko <andrii@kernel.org> wrote:
>
> > 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:
> >
> > ...
> >
> > 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.
> >
> > Fixes: ed5d583a88a9 ("fs/procfs: implement efficient VMA querying API for /proc/<pid>/maps")
> > Reported-by: syzbot+4e70c8e0a2017b432f7a@syzkaller.appspotmail.com
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>
> Thanks. ed5d583a88a9 was 6 months ago so I assume this isn't super
> urgent, so it needn't be rushed into mainline via mm.git's hotfixes
> queue.
>
> To provide for additional review and test time I'll queue the fix for
> the upcoming merge window (Feb 18 upstream merge) with a cc:stable.
This seems to be exacerbated (as we haven't seen these syzbot reports
before that) by more recent:
777a8560fd29 ("lib/buildid: use __kernel_read() for sleepable context")
which is in mm-stable. So I'm not sure, probably would be best to have
both of them together.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH mm-stable] procfs: avoid fetching build ID while holding VMA lock
2026-01-28 18:32 [PATCH mm-stable] procfs: avoid fetching build ID while holding VMA lock Andrii Nakryiko
2026-01-28 18:50 ` Andrew Morton
@ 2026-01-29 5:50 ` Suren Baghdasaryan
2026-01-29 21:43 ` Andrii Nakryiko
2026-01-29 15:52 ` Yonghong Song
2 siblings, 1 reply; 10+ messages in thread
From: Suren Baghdasaryan @ 2026-01-29 5:50 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: akpm, linux-mm, linux-fsdevel, bpf, syzbot+4e70c8e0a2017b432f7a
On Wed, Jan 28, 2026 at 10:32 AM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> 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 ***
>
> 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.
>
> Fixes: ed5d583a88a9 ("fs/procfs: implement efficient VMA querying API for /proc/<pid>/maps")
> Reported-by: syzbot+4e70c8e0a2017b432f7a@syzkaller.appspotmail.com
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
One nit below, otherwise LGTM.
Reviewed-by: Suren Baghdasaryan <surenb@google.com>
Tested-by: Suren Baghdasaryan <surenb@google.com>
> ---
> fs/proc/task_mmu.c | 42 ++++++++++++++++++++++++++---------------
> include/linux/buildid.h | 3 +++
> lib/buildid.c | 34 +++++++++++++++++++++++++--------
> 3 files changed, 56 insertions(+), 23 deletions(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 480db575553e..dd3b5cf9f0b7 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -656,6 +656,7 @@ static int do_procmap_query(struct mm_struct *mm, void __user *uarg)
> struct proc_maps_locking_ctx lock_ctx = { .mm = mm };
> struct procmap_query karg;
> struct vm_area_struct *vma;
> + struct file *vm_file = NULL;
> const char *name = NULL;
> char build_id_buf[BUILD_ID_SIZE_MAX], *name_buf = NULL;
> __u64 usize;
> @@ -727,21 +728,6 @@ static int do_procmap_query(struct mm_struct *mm, 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;
> @@ -775,10 +761,34 @@ static int do_procmap_query(struct mm_struct *mm, 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(&lock_ctx);
> 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;
Before this change we returned EINVAL when vma->vm_file==NULL, now we
return ENOENT. Probably not critical but is there a reason for this
change?
> + 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);
> @@ -798,6 +808,8 @@ static int do_procmap_query(struct mm_struct *mm, void __user *uarg)
> out:
> query_vma_teardown(&lock_ctx);
> 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 831c1b4b626c..7acc06b22fb7 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 818331051afe..dc643a6293c1 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));
> @@ -332,7 +328,10 @@ 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 */);
> }
>
> /*
> @@ -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 */);
> +}
> +
> +/*
> + * Parse build ID of ELF file
> + * @vma: 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 */);
> }
>
> /**
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH mm-stable] procfs: avoid fetching build ID while holding VMA lock
2026-01-28 18:32 [PATCH mm-stable] procfs: avoid fetching build ID while holding VMA lock Andrii Nakryiko
2026-01-28 18:50 ` Andrew Morton
2026-01-29 5:50 ` Suren Baghdasaryan
@ 2026-01-29 15:52 ` Yonghong Song
2026-01-29 17:14 ` Mykyta Yatsenko
2026-01-29 21:48 ` Andrii Nakryiko
2 siblings, 2 replies; 10+ messages in thread
From: Yonghong Song @ 2026-01-29 15:52 UTC (permalink / raw)
To: Andrii Nakryiko, akpm, linux-mm
Cc: linux-fsdevel, bpf, surenb, syzbot+4e70c8e0a2017b432f7a
On 1/28/26 10:32 AM, Andrii Nakryiko wrote:
> 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 ***
>
> 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.
>
> Fixes: ed5d583a88a9 ("fs/procfs: implement efficient VMA querying API for /proc/<pid>/maps")
> Reported-by: syzbot+4e70c8e0a2017b432f7a@syzkaller.appspotmail.com
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> fs/proc/task_mmu.c | 42 ++++++++++++++++++++++++++---------------
> include/linux/buildid.h | 3 +++
> lib/buildid.c | 34 +++++++++++++++++++++++++--------
> 3 files changed, 56 insertions(+), 23 deletions(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 480db575553e..dd3b5cf9f0b7 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -656,6 +656,7 @@ static int do_procmap_query(struct mm_struct *mm, void __user *uarg)
> struct proc_maps_locking_ctx lock_ctx = { .mm = mm };
> struct procmap_query karg;
> struct vm_area_struct *vma;
> + struct file *vm_file = NULL;
> const char *name = NULL;
> char build_id_buf[BUILD_ID_SIZE_MAX], *name_buf = NULL;
> __u64 usize;
> @@ -727,21 +728,6 @@ static int do_procmap_query(struct mm_struct *mm, 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;
> @@ -775,10 +761,34 @@ static int do_procmap_query(struct mm_struct *mm, 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(&lock_ctx);
> 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);
> @@ -798,6 +808,8 @@ static int do_procmap_query(struct mm_struct *mm, void __user *uarg)
> out:
> query_vma_teardown(&lock_ctx);
> 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 831c1b4b626c..7acc06b22fb7 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 818331051afe..dc643a6293c1 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));
> @@ -332,7 +328,10 @@ 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 */);
> }
>
> /*
> @@ -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 */);
> +}
> +
> +/*
> + * Parse build ID of ELF file
> + * @vma: file object
Should this be
@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 */);
> }
>
> /**
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH mm-stable] procfs: avoid fetching build ID while holding VMA lock
2026-01-29 15:52 ` Yonghong Song
@ 2026-01-29 17:14 ` Mykyta Yatsenko
2026-01-29 21:46 ` Andrii Nakryiko
2026-01-29 21:48 ` Andrii Nakryiko
1 sibling, 1 reply; 10+ messages in thread
From: Mykyta Yatsenko @ 2026-01-29 17:14 UTC (permalink / raw)
To: Yonghong Song, Andrii Nakryiko, akpm, linux-mm
Cc: linux-fsdevel, bpf, surenb, syzbot+4e70c8e0a2017b432f7a
On 1/29/26 15:52, Yonghong Song wrote:
>
>
> On 1/28/26 10:32 AM, Andrii Nakryiko wrote:
>> 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 ***
>>
>> 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.
>>
>> Fixes: ed5d583a88a9 ("fs/procfs: implement efficient VMA querying API
>> for /proc/<pid>/maps")
>> Reported-by: syzbot+4e70c8e0a2017b432f7a@syzkaller.appspotmail.com
>> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>> ---
>> fs/proc/task_mmu.c | 42 ++++++++++++++++++++++++++---------------
>> include/linux/buildid.h | 3 +++
>> lib/buildid.c | 34 +++++++++++++++++++++++++--------
>> 3 files changed, 56 insertions(+), 23 deletions(-)
>>
>> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
>> index 480db575553e..dd3b5cf9f0b7 100644
>> --- a/fs/proc/task_mmu.c
>> +++ b/fs/proc/task_mmu.c
>> @@ -656,6 +656,7 @@ static int do_procmap_query(struct mm_struct *mm,
>> void __user *uarg)
>> struct proc_maps_locking_ctx lock_ctx = { .mm = mm };
>> struct procmap_query karg;
>> struct vm_area_struct *vma;
>> + struct file *vm_file = NULL;
>> const char *name = NULL;
>> char build_id_buf[BUILD_ID_SIZE_MAX], *name_buf = NULL;
>> __u64 usize;
>> @@ -727,21 +728,6 @@ static int do_procmap_query(struct mm_struct
>> *mm, 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;
>> @@ -775,10 +761,34 @@ static int do_procmap_query(struct mm_struct
>> *mm, 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(&lock_ctx);
>> 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);
>> @@ -798,6 +808,8 @@ static int do_procmap_query(struct mm_struct *mm,
>> void __user *uarg)
>> out:
>> query_vma_teardown(&lock_ctx);
>> 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 831c1b4b626c..7acc06b22fb7 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 818331051afe..dc643a6293c1 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));
>> @@ -332,7 +328,10 @@ 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 */);
>> }
>> /*
>> @@ -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 */);
>> +}
>> +
>> +/*
>> + * Parse build ID of ELF file
>> + * @vma: file object
>
> Should this be
> @file: file object
> ?
kernel-doc comment should start with
/**
instead of
/*
(additional asterisk). Not sure if that is important,
but it gets different highlight color in my editor.
>
>> + * @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
>> */);
>> }
>> /**
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH mm-stable] procfs: avoid fetching build ID while holding VMA lock
2026-01-29 5:50 ` Suren Baghdasaryan
@ 2026-01-29 21:43 ` Andrii Nakryiko
0 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2026-01-29 21:43 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: Andrii Nakryiko, akpm, linux-mm, linux-fsdevel, bpf,
syzbot+4e70c8e0a2017b432f7a
On Wed, Jan 28, 2026 at 9:51 PM Suren Baghdasaryan <surenb@google.com> wrote:
>
> On Wed, Jan 28, 2026 at 10:32 AM Andrii Nakryiko <andrii@kernel.org> wrote:
> >
> > 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 ***
> >
> > 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.
> >
> > Fixes: ed5d583a88a9 ("fs/procfs: implement efficient VMA querying API for /proc/<pid>/maps")
> > Reported-by: syzbot+4e70c8e0a2017b432f7a@syzkaller.appspotmail.com
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>
> One nit below, otherwise LGTM.
>
> Reviewed-by: Suren Baghdasaryan <surenb@google.com>
> Tested-by: Suren Baghdasaryan <surenb@google.com>
>
> > ---
> > fs/proc/task_mmu.c | 42 ++++++++++++++++++++++++++---------------
> > include/linux/buildid.h | 3 +++
> > lib/buildid.c | 34 +++++++++++++++++++++++++--------
> > 3 files changed, 56 insertions(+), 23 deletions(-)
> >
> > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> > index 480db575553e..dd3b5cf9f0b7 100644
> > --- a/fs/proc/task_mmu.c
> > +++ b/fs/proc/task_mmu.c
> > @@ -656,6 +656,7 @@ static int do_procmap_query(struct mm_struct *mm, void __user *uarg)
> > struct proc_maps_locking_ctx lock_ctx = { .mm = mm };
> > struct procmap_query karg;
> > struct vm_area_struct *vma;
> > + struct file *vm_file = NULL;
> > const char *name = NULL;
> > char build_id_buf[BUILD_ID_SIZE_MAX], *name_buf = NULL;
> > __u64 usize;
> > @@ -727,21 +728,6 @@ static int do_procmap_query(struct mm_struct *mm, 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;
> > @@ -775,10 +761,34 @@ static int do_procmap_query(struct mm_struct *mm, 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(&lock_ctx);
> > 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;
>
> Before this change we returned EINVAL when vma->vm_file==NULL, now we
> return ENOENT. Probably not critical but is there a reason for this
> change?
>
This error code is not returned to the caller. Even if build ID
parsing returns an error, all we do is `karg.build_id_size = 0;` and
proceed (successfully). So -ENOENT doesn't matter, but it felt like
"there is no file -> ENOENT" as the internal signal made more sense.
I'll keep it as is, I still think it makes sense.
> > + 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);
[...]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH mm-stable] procfs: avoid fetching build ID while holding VMA lock
2026-01-29 17:14 ` Mykyta Yatsenko
@ 2026-01-29 21:46 ` Andrii Nakryiko
0 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2026-01-29 21:46 UTC (permalink / raw)
To: Mykyta Yatsenko
Cc: Yonghong Song, Andrii Nakryiko, akpm, linux-mm, linux-fsdevel,
bpf, surenb
On Thu, Jan 29, 2026 at 9:14 AM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> On 1/29/26 15:52, Yonghong Song wrote:
> >
> >
> > On 1/28/26 10:32 AM, Andrii Nakryiko wrote:
> >> 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 ***
> >>
> >> 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.
> >>
> >> Fixes: ed5d583a88a9 ("fs/procfs: implement efficient VMA querying API
> >> for /proc/<pid>/maps")
> >> Reported-by: syzbot+4e70c8e0a2017b432f7a@syzkaller.appspotmail.com
> >> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> >> ---
> >> fs/proc/task_mmu.c | 42 ++++++++++++++++++++++++++---------------
> >> include/linux/buildid.h | 3 +++
> >> lib/buildid.c | 34 +++++++++++++++++++++++++--------
> >> 3 files changed, 56 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> >> index 480db575553e..dd3b5cf9f0b7 100644
> >> --- a/fs/proc/task_mmu.c
> >> +++ b/fs/proc/task_mmu.c
> >> @@ -656,6 +656,7 @@ static int do_procmap_query(struct mm_struct *mm,
> >> void __user *uarg)
> >> struct proc_maps_locking_ctx lock_ctx = { .mm = mm };
> >> struct procmap_query karg;
> >> struct vm_area_struct *vma;
> >> + struct file *vm_file = NULL;
> >> const char *name = NULL;
> >> char build_id_buf[BUILD_ID_SIZE_MAX], *name_buf = NULL;
> >> __u64 usize;
> >> @@ -727,21 +728,6 @@ static int do_procmap_query(struct mm_struct
> >> *mm, 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;
> >> @@ -775,10 +761,34 @@ static int do_procmap_query(struct mm_struct
> >> *mm, 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(&lock_ctx);
> >> 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);
> >> @@ -798,6 +808,8 @@ static int do_procmap_query(struct mm_struct *mm,
> >> void __user *uarg)
> >> out:
> >> query_vma_teardown(&lock_ctx);
> >> 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 831c1b4b626c..7acc06b22fb7 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 818331051afe..dc643a6293c1 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));
> >> @@ -332,7 +328,10 @@ 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 */);
> >> }
> >> /*
> >> @@ -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 */);
> >> +}
> >> +
> >> +/*
> >> + * Parse build ID of ELF file
> >> + * @vma: file object
> >
> > Should this be
> > @file: file object
> > ?
> kernel-doc comment should start with
>
> /**
> instead of
> /*
>
> (additional asterisk). Not sure if that is important,
> but it gets different highlight color in my editor.
Ah, I just copied a similar comment from another variant of that
helper. I'll fix this one up (but we'll leave with the others intact
for now, to minimize any conflicts across trees)
>
> >
> >> + * @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
> >> */);
> >> }
> >> /**
> >
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH mm-stable] procfs: avoid fetching build ID while holding VMA lock
2026-01-29 15:52 ` Yonghong Song
2026-01-29 17:14 ` Mykyta Yatsenko
@ 2026-01-29 21:48 ` Andrii Nakryiko
1 sibling, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2026-01-29 21:48 UTC (permalink / raw)
To: Yonghong Song; +Cc: Andrii Nakryiko, akpm, linux-mm, linux-fsdevel, bpf, surenb
On Thu, Jan 29, 2026 at 7:52 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
>
> On 1/28/26 10:32 AM, Andrii Nakryiko wrote:
> > 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 ***
> >
> > 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.
> >
> > Fixes: ed5d583a88a9 ("fs/procfs: implement efficient VMA querying API for /proc/<pid>/maps")
> > Reported-by: syzbot+4e70c8e0a2017b432f7a@syzkaller.appspotmail.com
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> > fs/proc/task_mmu.c | 42 ++++++++++++++++++++++++++---------------
> > include/linux/buildid.h | 3 +++
> > lib/buildid.c | 34 +++++++++++++++++++++++++--------
> > 3 files changed, 56 insertions(+), 23 deletions(-)
> >
[...]
> > /*
> > @@ -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 */);
> > +}
> > +
> > +/*
> > + * Parse build ID of ELF file
> > + * @vma: file object
>
> Should this be
> @file: file object
> ?
>
yep, thanks, fixed
> > + * @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 */);
> > }
> >
> > /**
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH mm-stable] procfs: avoid fetching build ID while holding VMA lock
2026-01-28 21:30 ` Andrii Nakryiko
@ 2026-01-29 23:28 ` Andrew Morton
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Morton @ 2026-01-29 23:28 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, linux-mm, linux-fsdevel, bpf, surenb,
syzbot+4e70c8e0a2017b432f7a
On Wed, 28 Jan 2026 13:30:37 -0800 Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > To provide for additional review and test time I'll queue the fix for
> > the upcoming merge window (Feb 18 upstream merge) with a cc:stable.
>
> This seems to be exacerbated (as we haven't seen these syzbot reports
> before that) by more recent:
>
> 777a8560fd29 ("lib/buildid: use __kernel_read() for sleepable context")
>
> which is in mm-stable. So I'm not sure, probably would be best to have
> both of them together.
777a8560fd29 is actually in mainline. I'll move this fix into mm.git's
mm-hotfixes queue so it should be present in 6.19.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-01-29 23:28 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-28 18:32 [PATCH mm-stable] procfs: avoid fetching build ID while holding VMA lock Andrii Nakryiko
2026-01-28 18:50 ` Andrew Morton
2026-01-28 21:30 ` Andrii Nakryiko
2026-01-29 23:28 ` Andrew Morton
2026-01-29 5:50 ` Suren Baghdasaryan
2026-01-29 21:43 ` Andrii Nakryiko
2026-01-29 15:52 ` Yonghong Song
2026-01-29 17:14 ` Mykyta Yatsenko
2026-01-29 21:46 ` Andrii Nakryiko
2026-01-29 21:48 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox