On Fri, Oct 18, 2024 at 12:15 PM Roberto Sassu roberto.sassu@huaweicloud.com wrote:
From: "Kirill A. Shutemov" kirill.shutemov@linux.intel.com
Commit ea7e2d5e49c0 ("mm: call the security_mmap_file() LSM hook in remap_file_pages()") fixed a security issue, it added an LSM check when trying to remap file pages, so that LSMs have the opportunity to evaluate such action like for other memory operations such as mmap() and mprotect().
However, that commit called security_mmap_file() inside the mmap_lock lock, while the other calls do it before taking the lock, after commit 8b3ec6814c83 ("take security_mmap_file() outside of ->mmap_sem").
This caused lock inversion issue with IMA which was taking the mmap_lock and i_mutex lock in the opposite way when the remap_file_pages() system call was called.
Solve the issue by splitting the critical region in remap_file_pages() in two regions: the first takes a read lock of mmap_lock, retrieves the VMA and the file descriptor associated, and calculates the 'prot' and 'flags' variables; the second takes a write lock on mmap_lock, checks that the VMA flags and the VMA file descriptor are the same as the ones obtained in the first critical region (otherwise the system call fails), and calls do_mmap().
In between, after releasing the read lock and before taking the write lock, call security_mmap_file(), and solve the lock inversion issue.
Cc: stable@vger.kernel.org # v6.12-rcx Fixes: ea7e2d5e49c0 ("mm: call the security_mmap_file() LSM hook in remap_file_pages()") Reported-by: syzbot+1cd571a672400ef3a930@syzkaller.appspotmail.com Closes: https://lore.kernel.org/linux-security-module/66f7b10e.050a0220.46d20.0036.G... Reviewed-by: Roberto Sassu roberto.sassu@huawei.com Reviewed-by: Jann Horn jannh@google.com Reviewed-by: Lorenzo Stoakes lorenzo.stoakes@oracle.com Tested-by: Roberto Sassu roberto.sassu@huawei.com Tested-by: syzbot+1cd571a672400ef3a930@syzkaller.appspotmail.com Signed-off-by: Kirill A. Shutemov kirill.shutemov@linux.intel.com
mm/mmap.c | 69 +++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 52 insertions(+), 17 deletions(-)
Thanks for working on this Roberto, Kirill, and everyone else who had a hand in reviewing and testing.
Reviewed-by: Paul Moore paul@paul-moore.com
Andrew, I see you're pulling this into the MM/hotfixes-unstable branch, do you also plan to send this up to Linus soon/next-week? If so, great, if not let me know and I can send it up via the LSM tree.
We need to get clarity around Roberto's sign-off, but I think that is more of an administrative mistake rather than an intentional omission :)