Re: [PATCH v3] ocfs2: fix use-after-free in ocfs2_fault() when VM_FAULT_RETRY

[tejas bharambe]

Well!!! outlook issues. I am resending it from gmail account using git send-email.
________________________________________
From: Joseph Qi <joseph.qi@xxxxxxxxxxxxxxxxx>
Sent: Wednesday, April 1, 2026 8:32 PM
To: tejas bharambe <tejas.bharambe@xxxxxxxxxxx>
Cc: mark@xxxxxxxxxx <mark@xxxxxxxxxx>; jlbec@xxxxxxxxxxxx <jlbec@xxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>; syzbot+a49010a0e8fcdeea075f@xxxxxxxxxxxxxxxxxxxxxxxxx <syzbot+a49010a0e8fcdeea075f@xxxxxxxxxxxxxxxxxxxxxxxxx>; ocfs2-devel@xxxxxxxxxxxxxxx <ocfs2-devel@xxxxxxxxxxxxxxx>
Subject: Re: [PATCH v3] ocfs2: fix use-after-free in ocfs2_fault() when VM_FAULT_RETRY



On 4/2/26 11:10 AM, tejas bharambe wrote:
> 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.
>
> Reported-by: syzbot+a49010a0e8fcdeea075f@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=a49010a0e8fcdeea075f
> Suggested-by: Joseph Qi <joseph.qi@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Tejas Bharambe <tejas.bharambe@xxxxxxxxxxx>
> ---
>  fs/ocfs2/mmap.c        |  6 +++---
>  fs/ocfs2/ocfs2_trace.h | 10 ++++------
>  2 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/fs/ocfs2/mmap.c b/fs/ocfs2/mmap.c
> index 50e2faf64c..41c08c5a3d 100644
> --- a/fs/ocfs2/mmap.c
> +++ b/fs/ocfs2/mmap.c
> @@ -30,7 +30,7 @@
>
>  static vm_fault_t ocfs2_fault(struct vm_fault *vmf)
>  {
> -     struct vm_area_struct *vma = vmf->vma;
> +     struct inode *inode = file_inode(vmf->vma->vm_file);
>       sigset_t oldset;
>       vm_fault_t ret;
>
> @@ -38,8 +38,8 @@ 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(OCFS2_I(inode)->ip_blkno,
> +     vmf->page, vmf->pgoff);

Seems malformed?

>       return ret;
>  }
>
> 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)

Ditto.

>  );
>
>  /* End of trace events for fs/ocfs2/mmap.c. */
> --
> 2.53.0
>