Re: [PATCH v7] ocfs2: fix use-after-free in ocfs2_fault() when VM_FAULT_RETRY
Joseph Qi <[email protected]>
| Newsgroups | dev.linux.lists.ocfs2-devel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
This patch has already been added into mm-tree. You don't have to resend it. BTW, don't send to [email protected] by default. You can use '--suppress-cc' when git send-email. Thanks, Joseph On 4/16/26 10:55 AM, 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 ip_blkno as a plain integer before calling > filemap_fault(), and removing vma from the trace event. Since > ip_blkno is copied by value before the lock can be dropped, it > remains valid regardless of what happens to the vma or inode > afterward. > > Reported-by: [email protected] > Closes: https://syzkaller.appspot.com/bug?extid=a49010a0e8fcdeea075f > Fixes: 614a9e849ca6 ("ocfs2: Remove FILE_IO from masklog.") > 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. */