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

[Joseph Qi <joseph.qi@linux.alibaba.com>]

Please resend it in a new thread.

Joseph

On 4/1/26 8:55 PM, tejas bharambe wrote:
> Hi Joseph,
> 
> Sorry missed the inode declaration during rebasing. Here is v3:
> 
> From b316cc0fdfa4e6a3702b8402bd613863226e1561 Mon Sep 17 00:00:00 2001
> From: Tejas Bharambe <tejas.bharambe@outlook.com>
> Date: Tue, 31 Mar 2026 20:45:28 -0700
> Subject: [PATCH v3] ocfs2: fix use-after-free in ocfs2_fault() when
>  VM_FAULT_RETRY
> 
> 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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=a49010a0e8fcdeea075f
> Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Signed-off-by: Tejas Bharambe <tejas.bharambe@outlook.com>
> ---
>  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);
>         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)
>  );
> 
>  /* End of trace events for fs/ocfs2/mmap.c. */
> --
> 2.53.0
> 
> 
> Thanks,
> Tejas
> 
> 
> 
> 
> ________________________________________
> From: Joseph Qi <joseph.qi@linux.alibaba.com>
> Sent: Wednesday, April 1, 2026 1:29 AM
> To: tejas bharambe <tejas.bharambe@outlook.com>
> Cc: mark@fasheh.com <mark@fasheh.com>; jlbec@evilplan.org <jlbec@evilplan.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; syzbot+a49010a0e8fcdeea075f@syzkaller.appspotmail.com <syzbot+a49010a0e8fcdeea075f@syzkaller.appspotmail.com>; ocfs2-devel@lists.linux.dev <ocfs2-devel@lists.linux.dev>
> Subject: Re: [PATCH v2] ocfs2: fix use-after-free in ocfs2_fault() when VM_FAULT_RETRY
> 
> 
> 
> On 4/1/26 12:20 PM, 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@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=a49010a0e8fcdeea075f
>> Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
>> Signed-off-by: Tejas Bharambe <tejas.bharambe@outlook.com>
>> ---
>>  fs/ocfs2/mmap.c        |  4 ++--
>>  fs/ocfs2/ocfs2_trace.h | 10 ++++------
>>  2 files changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/ocfs2/mmap.c b/fs/ocfs2/mmap.c
>> index 50e2faf64c..7a4be91d6a 100644
>> --- a/fs/ocfs2/mmap.c
>> +++ b/fs/ocfs2/mmap.c
>> @@ -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,
> 
> It seems you've missed defining 'inode' at first.
> 
> Joseph
> 
>> +                       vmf->page, vmf->pgoff);
>>        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)
>>  );
>>
>>  /* End of trace events for fs/ocfs2/mmap.c. */