Re: [PATCH v5] ocfs2: fix use-after-free in ocfs2_fault() when VM_FAULT_RETRY
Tejas Bharambe <[email protected]>
| Newsgroups | dev.linux.lists.ocfs2-devel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAJL2up5epdHZfH-UCTt+QyHR7LpDke1ddwurjepQU16gnYv-iA@mail.gmail.com> |
Noted sir. Sending in v6 On Fri, Apr 10, 2026 at 1:30 AM Joseph Qi <[email protected]> wrote: > > > > On 4/10/26 4:04 PM, Tejas Bharambe wrote: > > From: Tejas Bharambe <[email protected]> > > > > filemap_fault() may drop the mmap_lock before returning VM_FAULT_RETRY, > > as documented in mm/filemap.c: > > > > "If our return value has VM_FAULT_RETRY set, it's because the mmap_lock > > may be dropped before doing I/O or by lock_folio_maybe_drop_mmap()." > > > > When this happens, a concurrent munmap() can call remove_vma() and free > > the vm_area_struct via RCU. The saved 'vma' pointer in ocfs2_fault() then > > becomes a dangling pointer, and the subsequent trace_ocfs2_fault() call > > dereferences it -- a use-after-free. > > > > Fix this by saving the inode reference before calling filemap_fault(), > > and removing vma from the trace event. The inode remains valid across > > the lock drop since the file is still open, so the trace can fire in > > all cases without dereferencing the potentially freed vma. > > > > The commit log should also be updated. > Also do not send to [email protected] by default. > > Thanks, > Joseph > > > Reported-by: [email protected] > > Closes: https://syzkaller.appspot.com/bug?extid=a49010a0e8fcdeea075f > > Cc: [email protected] > > Suggested-by: Joseph Qi <[email protected]> > > Signed-off-by: Tejas Bharambe <[email protected]> > > --- > > fs/ocfs2/mmap.c | 7 +++---- > > fs/ocfs2/ocfs2_trace.h | 10 ++++------ > > 2 files changed, 7 insertions(+), 10 deletions(-) > > > > diff --git a/fs/ocfs2/mmap.c b/fs/ocfs2/mmap.c > > index 50e2faf64c..6c570157ca 100644 > > --- a/fs/ocfs2/mmap.c > > +++ b/fs/ocfs2/mmap.c > > @@ -30,7 +30,8 @@ > > > > static vm_fault_t ocfs2_fault(struct vm_fault *vmf) > > { > > - struct vm_area_struct *vma = vmf->vma; > > + unsigned long long ip_blkno = > > + OCFS2_I(file_inode(vmf->vma->vm_file))->ip_blkno; > > sigset_t oldset; > > vm_fault_t ret; > > > > @@ -38,11 +39,9 @@ static vm_fault_t ocfs2_fault(struct vm_fault *vmf) > > ret = filemap_fault(vmf); > > ocfs2_unblock_signals(&oldset); > > > > - trace_ocfs2_fault(OCFS2_I(vma->vm_file->f_mapping->host)->ip_blkno, > > - vma, vmf->page, vmf->pgoff); > > + trace_ocfs2_fault(ip_blkno, vmf->page, vmf->pgoff); > > return ret; > > } > > - > > static vm_fault_t __ocfs2_page_mkwrite(struct file *file, > > struct buffer_head *di_bh, struct folio *folio) > > { > > diff --git a/fs/ocfs2/ocfs2_trace.h b/fs/ocfs2/ocfs2_trace.h > > index 4b32fb5658..6c2c97a980 100644 > > --- a/fs/ocfs2/ocfs2_trace.h > > +++ b/fs/ocfs2/ocfs2_trace.h > > @@ -1246,22 +1246,20 @@ TRACE_EVENT(ocfs2_write_end_inline, > > > > TRACE_EVENT(ocfs2_fault, > > TP_PROTO(unsigned long long ino, > > - void *area, void *page, unsigned long pgoff), > > - TP_ARGS(ino, area, page, pgoff), > > + void *page, unsigned long pgoff), > > + TP_ARGS(ino, page, pgoff), > > TP_STRUCT__entry( > > __field(unsigned long long, ino) > > - __field(void *, area) > > __field(void *, page) > > __field(unsigned long, pgoff) > > ), > > TP_fast_assign( > > __entry->ino = ino; > > - __entry->area = area; > > __entry->page = page; > > __entry->pgoff = pgoff; > > ), > > - TP_printk("%llu %p %p %lu", > > - __entry->ino, __entry->area, __entry->page, __entry->pgoff) > > + TP_printk("%llu %p %lu", > > + __entry->ino, __entry->page, __entry->pgoff) > > ); > > > > /* End of trace events for fs/ocfs2/mmap.c. */ >