[v6] ocfs2: fix use-after-free in ocfs2_fault() when VM_FAULT_RETRY

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

Posted by Tejas Bharambe 2 months ago

From: Tejas Bharambe <tejas.bharambe@outlook.com>

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: 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        |  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. */
-- 
2.53.0

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

Posted by Joseph Qi 2 months ago


On 4/10/26 4:38 PM, Tejas Bharambe wrote:
> From: Tejas Bharambe <tejas.bharambe@outlook.com>
> 
> 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: 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>

Fixes: 614a9e849ca6 ("ocfs2: Remove FILE_IO from masklog.")
Cc: stable@vger.kernel.org
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> ---
>  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. */

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

Posted by Tejas Bharambe 2 months ago

Thank you Joseph,

Andrew, does it look good?

On Fri, Apr 10, 2026 at 2:36 AM Joseph Qi <joseph.qi@linux.alibaba.com> wrote:
>
>
>
> On 4/10/26 4:38 PM, Tejas Bharambe wrote:
> > From: Tejas Bharambe <tejas.bharambe@outlook.com>
> >
> > 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: 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>
>
> Fixes: 614a9e849ca6 ("ocfs2: Remove FILE_IO from masklog.")
> Cc: stable@vger.kernel.org
> Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
>
> > ---
> >  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. */
>