linux-kernel - Re: [syzbot] [integrity?] [lsm?] possible deadlock in process

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHQche-Gsy4=UT6+znKyPRDEHQm9y-MQ+zacoqfywKaz7VA2kg@mail.gmail.com>
Date: Sun, 29 Sep 2024 02:07:47 +0800
From: Shu Han <ebpqwerty472123@...il.com>
To: syzbot <syzbot+1cd571a672400ef3a930@...kaller.appspotmail.com>
Cc: akpm@...ux-foundation.org, dmitry.kasatkin@...il.com, 
	eric.snowberg@...cle.com, hughd@...gle.com, jmorris@...ei.org, 
	linux-integrity@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-mm@...ck.org, linux-security-module@...r.kernel.org, 
	paul@...l-moore.com, roberto.sassu@...wei.com, serge@...lyn.com, 
	stephen.smalley.work@...il.com, syzkaller-bugs@...glegroups.com, 
	zohar@...ux.ibm.com
Subject: Re: [syzbot] [integrity?] [lsm?] possible deadlock in
 process_measurement (4)

> ======================================================
> WARNING: possible circular locking dependency detected
> 6.11.0-syzkaller-10045-g97d8894b6f4c #0 Not tainted
> ------------------------------------------------------
> syz-executor369/5231 is trying to acquire lock:
> ffff888072852370 (&sb->s_type->i_mutex_key#12){+.+.}-{3:3}, at: inode_lock include/linux/fs.h:815 [inline]
> ffff888072852370 (&sb->s_type->i_mutex_key#12){+.+.}-{3:3}, at: process_measurement+0x439/0x1fb0 security/integrity/ima/ima_main.c:250
>
> but task is already holding lock:
> ffff88807ac9a798 (&mm->mmap_lock){++++}-{3:3}, at: mmap_write_lock_killable include/linux/mmap_lock.h:122 [inline]
> ffff88807ac9a798 (&mm->mmap_lock){++++}-{3:3}, at: __do_sys_remap_file_pages mm/mmap.c:1649 [inline]
> ffff88807ac9a798 (&mm->mmap_lock){++++}-{3:3}, at: __se_sys_remap_file_pages+0x22d/0xa50 mm/mmap.c:1624
>
> which lock already depends on the new lock.

This issue (if not a false positive?) is due to the possible `prot`
change caused by the processing logic for READ_IMPLIES_EXEC in do_mmap(),
so the remap_file_pages() must perform LSM check before calling do_mmap(),
this is what the previous commit want to do.

The LSM check is required to know what the `prot` is, but `prot` must be
obtained after holding the `mmap_write_lock`.

If the `mmap_write_lock` is released after getting the `prot` and before
the LSM call in remap_file_pages(), it may cause TOCTOU.

So, possible solutions may include:
1. Remove the security check by removing the the possibility of the `prot`
   change:
1.1. move the the processing logic for READ_IMPLIES_EXEC out of the
     do_mmap(). This also ensures that such missing checks which the
     previous commit fixes will not occur again(suggested).
     See the RFC PATCH
     https://lore.kernel.org/all/20240928180044.50-1-ebpqwerty472123@gmail.com/
1.2. Replace do_mmap() in remap_file_pages() to mmap_region(), which do
     the actually memory mapping without the respect to READ_IMPLIES_EXEC.
     But this requires other checks in do_mmap() is performed in
     remap_file_pages(), such as the `file_mmap_ok`(may complex).
2. Perform operations similar to updating a value by CAS(may slow):
for (;;) {
    mmap_write_lock();
    prot = get_prot();
    mmap_write_unlock();
    if (!call_lsm(prot)) return;
    mmap_write_lock();
    if (prot != get_prot()) continue;
    do_mmap();
    mmap_write_unlock();
}