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

Hi Andrew,

You're right, I missed that scenario.

The inode can be freed if the file descriptor is closed after mmap() and munmap() races with the fault handler.

I can do one of the following:
1. I can skip the trace firing when VM_FAULT_RETRY is set as I did in v1. It was changed to v4 after Joseph's suggestion to keep traces.
2. If we want to keep traces, we can use ihold()/iput() as shown below:

ihold(inode);   //pin inode
ret = filemap_fault(vmf);
trace_ocfs2_fault(OCFS2_I(inode)->ip_blkno, ...);  // safe, refcount held
iput(inode);  //release inode


Which approach do you prefer?

Thanks,
Tejas
________________________________________
From: Andrew Morton <[email protected]>
Sent: Saturday, April 4, 2026 5:50 PM
To: tejas bharambe <[email protected]>
Cc: Tejas Bharambe <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>
Subject: Re: [PATCH v4] ocfs2: fix use-after-free in ocfs2_fault() when VM_FAULT_RETRY

On Sun, 5 Apr 2026 00:30:14 +0000 tejas bharambe <[email protected]> wrote:

> Following is my response for question posted on https://sashiko.dev/#/patchset/20260403035333.136824-1-tejas.bharambe%40outlook.com
>
>
> No. For ocfs2_fault() to be executing, the file must be open and
> the process holds an active file descriptor. The inode's lifetime
> is tied to the file's reference count, which remains held by the
> file descriptor for the duration of the fault handler. munmap()
> can free the VMA (decrementing vm_file's refcount) but cannot
> free the inode as long as the file descriptor is open. The faulting
> thread cannot call close() while it is inside the fault handler,
> so the inode is guaranteed to outlive the trace call.

I don't think that's the scenario which Sashiko is suggesting.

Suppose userspace does

        fd = open(...);
        p = mmap(fd, ...);
        close(fd);

Now, that mmap is the only ref against fd.

Now, suppose that userspace does munmap() while another thread is in
the fault handler.