On Fri, Oct 18, 2024 at 12:15 PM Roberto Sassu <roberto.sassu@xxxxxxxxxxxxxxx> wrote: > From: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx> > > 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@xxxxxxxxxxxxxxx # v6.12-rcx > Fixes: ea7e2d5e49c0 ("mm: call the security_mmap_file() LSM hook in remap_file_pages()") > Reported-by: syzbot+1cd571a672400ef3a930@xxxxxxxxxxxxxxxxxxxxxxxxx > Closes: https://lore.kernel.org/linux-security-module/66f7b10e.050a0220.46d20.0036.GAE@xxxxxxxxxx/ > Reviewed-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx> > Reviewed-by: Jann Horn <jannh@xxxxxxxxxx> > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> > Tested-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx> > Tested-by: syzbot+1cd571a672400ef3a930@xxxxxxxxxxxxxxxxxxxxxxxxx > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxx> 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 :) -- paul-moore.com