[PATCH 0/1] Fix an imbalanced file ref count in the mmap syscall

[PATCH 0/1] Fix an imbalanced file ref count in the mmap syscall

[Liav Albani]

Hi!

This is my first patch that I contribute myself to the Linux kernel.
My apologize if there are issues, I tried to solve this issue in one
evening and understand the call graph and what could go wrong.

I work recently on a project of mine called nfiop, and specifically on a
kernel module called ufedm - https://github.com/nfiop/ufedm .

The project has a driver that allows a userspace implementation to
intervene in the write and read callbacks of an MTD device by spawning
an upper MTD device that sends the data back and forth to userspace, so
userspace can implement its custom ECC engine and send back the results.

So far so good. I started working with kernel 6.18.8 on an Olimex A20
board and then moved to my x86 linux machine with a nandsim simulation.
When I started my project, I implemented a memory backing based on a
shmem file for the shared memory buffer, that is expected to be mmap'ed
from my custom character device.

When I updated my host machine, the new kernel version was 7.x so I
decided to update my module as well.
Many changes were comsetic, but the major change was to implement a
mmap_prepare hook.

I wrote something simple, taking notes on the mmap_prepare rules and
the guide from kernel docs.

Then I started to test that my character device is at least working
fine, so I have my unit tests for ioctls and mmap, and the mmap
unit-test failed after the second-run.

A kernel stack looked like this:
```
imbalanced put on file reference count
WARNING: fs/file.c:36 at __file_ref_put_badval.isra.0+0x3f/0x60, CPU#1: test_proxy_mmap/116
Modules linked in: ufedm(OE) virtio_net net_failover failover nandsim nand nandcore bch mtd
CPU: 1 UID: 0 PID: 116 Comm: test_proxy_mmap Tainted: G           OE       7.2.0-rc3-gaf5e34a41cd6 #1 PREEMPT(full)  bdadc55b51524b311c430c1536a97e7071150fd3
Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.17.0-2-2 04/01/2014
RIP: 0010:__file_ref_put_badval.isra.0+0x3f/0x60
Code: 85 f6 78 05 c3 cc cc cc cc 48 b8 00 00 00 00 00 00 00 a0 48 89 07 c3 cc cc cc cc 48 83 ec 08 48 89 3c 24 48 8d 3d 61 bc 30 02 <67> 48 0f b9 3a 48 ba 00 00 00 00 00 00 00 e0 48 8b 04 24 48 89 10
RSP: 0018:ffffc900006bbae8 EFLAGS: 00010296
RAX: bfffffffffffffff RBX: ffff88810409a180 RCX: 0000000038748000
RDX: ffff888100a44480 RSI: dfffffffffffffff RDI: ffffffff83b12730
RBP: ffffc900006bbbf8 R08: 0000000000000000 R09: 00007f8b49f8f000
R10: ffffc900006bbba0 R11: ffff88810409a208 R12: ffffc900006bbba0
R13: 00007f8b4a179000 R14: ffff888102222440 R15: 0000000000015040
FS:  0000000038726400(0000) GS:ffff8881b77d1000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000038728618 CR3: 0000000104a1c006 CR4: 0000000000370ef0
Call Trace:
 <TASK>
 __file_ref_put+0x30/0x40
 fput+0x4f/0x80
 remove_vma+0x42/0x60
 vms_complete_munmap_vmas+0x179/0x230
 do_vmi_align_munmap+0x225/0x2a0
 do_vmi_munmap+0xe6/0x1c0
 __vm_munmap+0x113/0x200
 __x64_sys_munmap+0x1b/0x30
 do_syscall_64+0xaa/0x660
 ? ksys_mmap_pgoff+0x168/0x200
 ? do_syscall_64+0xaa/0x660
 ? vfs_write+0x281/0x560
 ? ksys_write+0x7b/0x110
 ? do_syscall_64+0xaa/0x660
 ? do_syscall_64+0x5f/0x660
 ? clear_bhb_loop+0x30/0x80
 ? clear_bhb_loop+0x30/0x80
 entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x440bcb
```

I started investigating this, and it became apparent that there's an
issue with ref-counting of the vm_file - I changed it in my mmap_prepare
hook, so it must be the cause, right?

I also tried to artificially increment the ref-count of my shmem file
when I create it (taking another ref), so the second attempt didn't fail
but the third did...

I looked at the stack trace, and the culprit is the fact that we do fput
on a vm_file that we didn't pin - to fix this, I added a flag if we
actually did get_file() on the vm_file or not, on the vma struct.

When testing this patch, I've observed that the problem was completely
gone.

The mmap_prepare hook is still quite new, so presumably fixing corner
cases like this one is a good candidate for upstream, for future drivers
that might do the same as I do in my driver.

Liav Albani (1):
  mm/vma: fix imbalanced file reference count, properly abort
    mmap_prepare

 include/linux/mm_types.h |  1 +
 mm/vma.c                 | 24 +++++++++++++-----------
 2 files changed, 14 insertions(+), 11 deletions(-)

-- 
2.55.0

[PATCH] mm/vma: fix imbalanced file reference count, properly abort mmap_prepare

[Liav Albani]

Under the new `vm_area_desc` struct, a driver may change the vm_file
to provide a different backing VM file.

In such case, it became apparent during testing of this capability, that
a remove_vma() may drop a refcount on a VM file that we didn't call
get_file() upon.

In addition to that, calling vms_abort_munmap_vmas when we didn't do
actual work is wrong, because mmap_prepare hook is about preparing the
mmap state and not performing any actual mapping (yet).

A kernel stack (before of this patch) could look like this:

imbalanced put on file reference count
WARNING: fs/file.c:36 at __file_ref_put_badval.isra.0+0x3f/0x60, CPU#1: test_proxy_mmap/116
Modules linked in: ufedm(OE) virtio_net net_failover failover nandsim nand nandcore bch mtd
CPU: 1 UID: 0 PID: 116 Comm: test_proxy_mmap Tainted: G           OE       7.2.0-rc3-gaf5e34a41cd6 #1 PREEMPT(full)  bdadc55b51524b311c430c1536a97e7071150fd3
Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.17.0-2-2 04/01/2014
RIP: 0010:__file_ref_put_badval.isra.0+0x3f/0x60
Code: 85 f6 78 05 c3 cc cc cc cc 48 b8 00 00 00 00 00 00 00 a0 48 89 07 c3 cc cc cc cc 48 83 ec 08 48 89 3c 24 48 8d 3d 61 bc 30 02 <67> 48 0f b9 3a 48 ba 00 00 00 00 00 00 00 e0 48 8b 04 24 48 89 10
RSP: 0018:ffffc900006bbae8 EFLAGS: 00010296
RAX: bfffffffffffffff RBX: ffff88810409a180 RCX: 0000000038748000
RDX: ffff888100a44480 RSI: dfffffffffffffff RDI: ffffffff83b12730
RBP: ffffc900006bbbf8 R08: 0000000000000000 R09: 00007f8b49f8f000
R10: ffffc900006bbba0 R11: ffff88810409a208 R12: ffffc900006bbba0
R13: 00007f8b4a179000 R14: ffff888102222440 R15: 0000000000015040
FS:  0000000038726400(0000) GS:ffff8881b77d1000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000038728618 CR3: 0000000104a1c006 CR4: 0000000000370ef0
Call Trace:
 <TASK>
 __file_ref_put+0x30/0x40
 fput+0x4f/0x80
 remove_vma+0x42/0x60
 vms_complete_munmap_vmas+0x179/0x230
 do_vmi_align_munmap+0x225/0x2a0
 do_vmi_munmap+0xe6/0x1c0
 __vm_munmap+0x113/0x200
 __x64_sys_munmap+0x1b/0x30
 do_syscall_64+0xaa/0x660
 ? ksys_mmap_pgoff+0x168/0x200
 ? do_syscall_64+0xaa/0x660
 ? vfs_write+0x281/0x560
 ? ksys_write+0x7b/0x110
 ? do_syscall_64+0xaa/0x660
 ? do_syscall_64+0x5f/0x660
 ? clear_bhb_loop+0x30/0x80
 ? clear_bhb_loop+0x30/0x80
 entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x440bcb

Signed-off-by: Liav Albani <liavalb@gmail.com>
---
 include/linux/mm_types.h |  1 +
 mm/vma.c                 | 24 +++++++++++++-----------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index b18c2b2e7d2c..5db0894e6064 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -981,6 +981,7 @@ struct vm_area_struct {
 	unsigned long vm_pgoff;		/* Offset (within vm_file) in PAGE_SIZE
 					   units */
 	struct file * vm_file;		/* File we map to (can be NULL). */
+	bool vm_file_doesnt_need_put; /* Do fput only if we did get_file... */
 	void * vm_private_data;		/* was vm_pte (shared mem) */
 
 #ifdef CONFIG_SWAP
diff --git a/mm/vma.c b/mm/vma.c
index 9eea2850818a..0c2e71fc09be 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -467,7 +467,7 @@ void remove_vma(struct vm_area_struct *vma)
 {
 	might_sleep();
 	vma_close(vma);
-	if (vma->vm_file)
+	if (vma->vm_file && !vma->vm_file_doesnt_need_put)
 		fput(vma->vm_file);
 	mpol_put(vma_policy(vma));
 	vm_area_free(vma);
@@ -2487,6 +2487,7 @@ static int __mmap_new_file_vma(struct mmap_state *map,
 	int error;
 
 	vma->vm_file = map->file;
+	vma->vm_file_doesnt_need_put = map->file_doesnt_need_get;
 	if (!map->file_doesnt_need_get)
 		get_file(map->file);
 
@@ -2497,7 +2498,14 @@ static int __mmap_new_file_vma(struct mmap_state *map,
 	if (error) {
 		UNMAP_STATE(unmap, vmi, vma, vma->vm_start, vma->vm_end,
 			    map->prev, map->next);
-		fput(vma->vm_file);
+
+		/*
+		 * We set vma->vm_file to map->file, so if we didn't increase
+		 * the refcount, don't decrease it in this path.
+		 */
+		if (!map->file_doesnt_need_get)
+			fput(vma->vm_file);
+
 		vma->vm_file = NULL;
 
 		vma_iter_set(vmi, vma->vm_end);
@@ -2757,7 +2765,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
 	if (!error && have_mmap_prepare)
 		error = call_mmap_prepare(&map, &desc);
 	if (error)
-		goto abort_munmap;
+		goto abort_mmap_prepare;
 
 	if (map.check_ksm_early)
 		update_ksm_flags(&map);
@@ -2795,15 +2803,9 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
 unacct_error:
 	if (map.charged)
 		vm_unacct_memory(map.charged);
-abort_munmap:
-	/*
-	 * This indicates that .mmap_prepare has set a new file, differing from
-	 * desc->vm_file. But since we're aborting the operation, only the
-	 * original file will be cleaned up. Ensure we clean up both.
-	 */
-	if (map.file_doesnt_need_get)
-		fput(map.file);
+
 	vms_abort_munmap_vmas(&map.vms, &map.mas_detach);
+abort_mmap_prepare:
 	return error;
 }
 
-- 
2.55.0

Re: [PATCH 0/1] Fix an imbalanced file ref count in the mmap syscall

[Pedro Falcato]

On Sat, Jul 18, 2026 at 03:34:16PM +0300, Liav Albani wrote:
> Hi!
> 
> This is my first patch that I contribute myself to the Linux kernel.
> My apologize if there are issues, I tried to solve this issue in one
> evening and understand the call graph and what could go wrong.
> 
> I work recently on a project of mine called nfiop, and specifically on a
> kernel module called ufedm - https://github.com/nfiop/ufedm .
> 
> The project has a driver that allows a userspace implementation to
> intervene in the write and read callbacks of an MTD device by spawning
> an upper MTD device that sends the data back and forth to userspace, so
> userspace can implement its custom ECC engine and send back the results.
> 
> So far so good. I started working with kernel 6.18.8 on an Olimex A20
> board and then moved to my x86 linux machine with a nandsim simulation.
> When I started my project, I implemented a memory backing based on a
> shmem file for the shared memory buffer, that is expected to be mmap'ed
> from my custom character device.
> 
> When I updated my host machine, the new kernel version was 7.x so I
> decided to update my module as well.
> Many changes were comsetic, but the major change was to implement a
> mmap_prepare hook.
> 
> I wrote something simple, taking notes on the mmap_prepare rules and
> the guide from kernel docs.
> 
> Then I started to test that my character device is at least working
> fine, so I have my unit tests for ioctls and mmap, and the mmap
> unit-test failed after the second-run.
> 
> A kernel stack looked like this:
> ```
> imbalanced put on file reference count
> WARNING: fs/file.c:36 at __file_ref_put_badval.isra.0+0x3f/0x60, CPU#1: test_proxy_mmap/116
> Modules linked in: ufedm(OE) virtio_net net_failover failover nandsim nand nandcore bch mtd
> CPU: 1 UID: 0 PID: 116 Comm: test_proxy_mmap Tainted: G           OE       7.2.0-rc3-gaf5e34a41cd6 #1 PREEMPT(full)  bdadc55b51524b311c430c1536a97e7071150fd3
> Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.17.0-2-2 04/01/2014
> RIP: 0010:__file_ref_put_badval.isra.0+0x3f/0x60
> Code: 85 f6 78 05 c3 cc cc cc cc 48 b8 00 00 00 00 00 00 00 a0 48 89 07 c3 cc cc cc cc 48 83 ec 08 48 89 3c 24 48 8d 3d 61 bc 30 02 <67> 48 0f b9 3a 48 ba 00 00 00 00 00 00 00 e0 48 8b 04 24 48 89 10
> RSP: 0018:ffffc900006bbae8 EFLAGS: 00010296
> RAX: bfffffffffffffff RBX: ffff88810409a180 RCX: 0000000038748000
> RDX: ffff888100a44480 RSI: dfffffffffffffff RDI: ffffffff83b12730
> RBP: ffffc900006bbbf8 R08: 0000000000000000 R09: 00007f8b49f8f000
> R10: ffffc900006bbba0 R11: ffff88810409a208 R12: ffffc900006bbba0
> R13: 00007f8b4a179000 R14: ffff888102222440 R15: 0000000000015040
> FS:  0000000038726400(0000) GS:ffff8881b77d1000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000038728618 CR3: 0000000104a1c006 CR4: 0000000000370ef0
> Call Trace:
>  <TASK>
>  __file_ref_put+0x30/0x40
>  fput+0x4f/0x80
>  remove_vma+0x42/0x60
>  vms_complete_munmap_vmas+0x179/0x230
>  do_vmi_align_munmap+0x225/0x2a0
>  do_vmi_munmap+0xe6/0x1c0
>  __vm_munmap+0x113/0x200
>  __x64_sys_munmap+0x1b/0x30
>  do_syscall_64+0xaa/0x660
>  ? ksys_mmap_pgoff+0x168/0x200
>  ? do_syscall_64+0xaa/0x660
>  ? vfs_write+0x281/0x560
>  ? ksys_write+0x7b/0x110
>  ? do_syscall_64+0xaa/0x660
>  ? do_syscall_64+0x5f/0x660
>  ? clear_bhb_loop+0x30/0x80
>  ? clear_bhb_loop+0x30/0x80
>  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> RIP: 0033:0x440bcb
> ```
> 
> I started investigating this, and it became apparent that there's an
> issue with ref-counting of the vm_file - I changed it in my mmap_prepare
> hook, so it must be the cause, right?
> 
> I also tried to artificially increment the ref-count of my shmem file

You need to take a ref when doing the mmap_prepare file setting. mmap (and VMAs)
consumes one reference, thus need to pin the file. Thus why the ref count
manipulation around this whole code...

So, in your case, something like:

	get_file(dev->shm_mapping.filp);

in your proxy_chrdev_mmap_prepare looks correct. Otherwise, something else
could sweep the file under the VMA. Note that this is true for every mmapped
file, even something like /dev/zero (which does what you're doing, creates
a shmem file and changes the VMA desc) implicitly donates the reference to
mmap, which will later be unref'd (and the shmem file torn down) on munmap.

Let me know if I hugely misunderstood your problem :)

-- 
Pedro

Re: [PATCH 0/1] Fix an imbalanced file ref count in the mmap syscall

[Lorenzo Stoakes (ARM)]

On Sat, Jul 18, 2026 at 02:25:15PM +0100, Pedro Falcato wrote:
> On Sat, Jul 18, 2026 at 03:34:16PM +0300, Liav Albani wrote:
> > Hi!
> >
> > This is my first patch that I contribute myself to the Linux kernel.
> > My apologize if there are issues, I tried to solve this issue in one
> > evening and understand the call graph and what could go wrong.
> >
> > I work recently on a project of mine called nfiop, and specifically on a
> > kernel module called ufedm - https://github.com/nfiop/ufedm .
> >
> > The project has a driver that allows a userspace implementation to
> > intervene in the write and read callbacks of an MTD device by spawning
> > an upper MTD device that sends the data back and forth to userspace, so
> > userspace can implement its custom ECC engine and send back the results.
> >
> > So far so good. I started working with kernel 6.18.8 on an Olimex A20
> > board and then moved to my x86 linux machine with a nandsim simulation.
> > When I started my project, I implemented a memory backing based on a
> > shmem file for the shared memory buffer, that is expected to be mmap'ed
> > from my custom character device.
> >
> > When I updated my host machine, the new kernel version was 7.x so I
> > decided to update my module as well.
> > Many changes were comsetic, but the major change was to implement a
> > mmap_prepare hook.
> >
> > I wrote something simple, taking notes on the mmap_prepare rules and
> > the guide from kernel docs.
> >
> > Then I started to test that my character device is at least working
> > fine, so I have my unit tests for ioctls and mmap, and the mmap
> > unit-test failed after the second-run.
> >
> > A kernel stack looked like this:
> > ```
> > imbalanced put on file reference count
> > WARNING: fs/file.c:36 at __file_ref_put_badval.isra.0+0x3f/0x60, CPU#1: test_proxy_mmap/116
> > Modules linked in: ufedm(OE) virtio_net net_failover failover nandsim nand nandcore bch mtd
> > CPU: 1 UID: 0 PID: 116 Comm: test_proxy_mmap Tainted: G           OE       7.2.0-rc3-gaf5e34a41cd6 #1 PREEMPT(full)  bdadc55b51524b311c430c1536a97e7071150fd3
> > Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.17.0-2-2 04/01/2014
> > RIP: 0010:__file_ref_put_badval.isra.0+0x3f/0x60
> > Code: 85 f6 78 05 c3 cc cc cc cc 48 b8 00 00 00 00 00 00 00 a0 48 89 07 c3 cc cc cc cc 48 83 ec 08 48 89 3c 24 48 8d 3d 61 bc 30 02 <67> 48 0f b9 3a 48 ba 00 00 00 00 00 00 00 e0 48 8b 04 24 48 89 10
> > RSP: 0018:ffffc900006bbae8 EFLAGS: 00010296
> > RAX: bfffffffffffffff RBX: ffff88810409a180 RCX: 0000000038748000
> > RDX: ffff888100a44480 RSI: dfffffffffffffff RDI: ffffffff83b12730
> > RBP: ffffc900006bbbf8 R08: 0000000000000000 R09: 00007f8b49f8f000
> > R10: ffffc900006bbba0 R11: ffff88810409a208 R12: ffffc900006bbba0
> > R13: 00007f8b4a179000 R14: ffff888102222440 R15: 0000000000015040
> > FS:  0000000038726400(0000) GS:ffff8881b77d1000(0000) knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000038728618 CR3: 0000000104a1c006 CR4: 0000000000370ef0
> > Call Trace:
> >  <TASK>
> >  __file_ref_put+0x30/0x40
> >  fput+0x4f/0x80
> >  remove_vma+0x42/0x60
> >  vms_complete_munmap_vmas+0x179/0x230
> >  do_vmi_align_munmap+0x225/0x2a0
> >  do_vmi_munmap+0xe6/0x1c0
> >  __vm_munmap+0x113/0x200
> >  __x64_sys_munmap+0x1b/0x30
> >  do_syscall_64+0xaa/0x660
> >  ? ksys_mmap_pgoff+0x168/0x200
> >  ? do_syscall_64+0xaa/0x660
> >  ? vfs_write+0x281/0x560
> >  ? ksys_write+0x7b/0x110
> >  ? do_syscall_64+0xaa/0x660
> >  ? do_syscall_64+0x5f/0x660
> >  ? clear_bhb_loop+0x30/0x80
> >  ? clear_bhb_loop+0x30/0x80
> >  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> > RIP: 0033:0x440bcb
> > ```
> >
> > I started investigating this, and it became apparent that there's an
> > issue with ref-counting of the vm_file - I changed it in my mmap_prepare
> > hook, so it must be the cause, right?
> >
> > I also tried to artificially increment the ref-count of my shmem file
>
> You need to take a ref when doing the mmap_prepare file setting. mmap (and VMAs)
> consumes one reference, thus need to pin the file. Thus why the ref count
> manipulation around this whole code...
>
> So, in your case, something like:
>
> 	get_file(dev->shm_mapping.filp);
>
> in your proxy_chrdev_mmap_prepare looks correct. Otherwise, something else
> could sweep the file under the VMA. Note that this is true for every mmapped
> file, even something like /dev/zero (which does what you're doing, creates
> a shmem file and changes the VMA desc) implicitly donates the reference to
> mmap, which will later be unref'd (and the shmem file torn down) on munmap.
>
> Let me know if I hugely misunderstood your problem :)
>
> --
> Pedro

Yeah I agree. Thanks for the contribution and your interest in this part of
the kernel :) but any file replacing an existing one must be pinned as it
is then used by the mmap and the pin signifies that.

Perhaps a patch to the documentation could be useful here? In
Documentation/filesystems/mmap_prepare.rst

If you found it confusing that that implies it's not clear, so something
there saying 'if you replace the file, you MUST pin it via get_file()
before assigning desc->vm_file'.

Thanks, Lorenzo

Re: [PATCH] mm/vma: fix imbalanced file reference count, properly abort mmap_prepare

[Lorenzo Stoakes (ARM)]

Thanks for the patch but as per the 0/1 this is not correct - the issue is that
your mmap_prepare hook doesn't get the file.

(Also in general, we don't add cover letters for single patches, only for series
with >1 patch :)

On Sat, Jul 18, 2026 at 03:34:17PM +0300, Liav Albani wrote:
> Under the new `vm_area_desc` struct, a driver may change the vm_file
> to provide a different backing VM file.
>
> In such case, it became apparent during testing of this capability, that
> a remove_vma() may drop a refcount on a VM file that we didn't call
> get_file() upon.

As above.

>
> In addition to that, calling vms_abort_munmap_vmas when we didn't do
> actual work is wrong, because mmap_prepare hook is about preparing the
> mmap state and not performing any actual mapping (yet).

Not the case - __mmap_setup() unmaps any existing mappings in the area
being mapped into (they are taken out of the maple tree and put into a
detached state then added to the temporary map->mt_detach tree), and
vms_abort_munmap_vmas() will put them back.

The vms data structure is set up in int_vma_munmap(), then
vms_gather_munmap_vmas() populates both map->vms and map->mas_detach.

If nothing was unmapped map->vms->nr_pages == 0, and
vms_abort_munmap_vmas() is a noop.

>
> A kernel stack (before of this patch) could look like this:
>
> imbalanced put on file reference count
> WARNING: fs/file.c:36 at __file_ref_put_badval.isra.0+0x3f/0x60, CPU#1: test_proxy_mmap/116
> Modules linked in: ufedm(OE) virtio_net net_failover failover nandsim nand nandcore bch mtd
> CPU: 1 UID: 0 PID: 116 Comm: test_proxy_mmap Tainted: G           OE       7.2.0-rc3-gaf5e34a41cd6 #1 PREEMPT(full)  bdadc55b51524b311c430c1536a97e7071150fd3
> Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.17.0-2-2 04/01/2014
> RIP: 0010:__file_ref_put_badval.isra.0+0x3f/0x60
> Code: 85 f6 78 05 c3 cc cc cc cc 48 b8 00 00 00 00 00 00 00 a0 48 89 07 c3 cc cc cc cc 48 83 ec 08 48 89 3c 24 48 8d 3d 61 bc 30 02 <67> 48 0f b9 3a 48 ba 00 00 00 00 00 00 00 e0 48 8b 04 24 48 89 10
> RSP: 0018:ffffc900006bbae8 EFLAGS: 00010296
> RAX: bfffffffffffffff RBX: ffff88810409a180 RCX: 0000000038748000
> RDX: ffff888100a44480 RSI: dfffffffffffffff RDI: ffffffff83b12730
> RBP: ffffc900006bbbf8 R08: 0000000000000000 R09: 00007f8b49f8f000
> R10: ffffc900006bbba0 R11: ffff88810409a208 R12: ffffc900006bbba0
> R13: 00007f8b4a179000 R14: ffff888102222440 R15: 0000000000015040
> FS:  0000000038726400(0000) GS:ffff8881b77d1000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000038728618 CR3: 0000000104a1c006 CR4: 0000000000370ef0
> Call Trace:
>  <TASK>
>  __file_ref_put+0x30/0x40
>  fput+0x4f/0x80
>  remove_vma+0x42/0x60
>  vms_complete_munmap_vmas+0x179/0x230
>  do_vmi_align_munmap+0x225/0x2a0
>  do_vmi_munmap+0xe6/0x1c0
>  __vm_munmap+0x113/0x200
>  __x64_sys_munmap+0x1b/0x30
>  do_syscall_64+0xaa/0x660
>  ? ksys_mmap_pgoff+0x168/0x200
>  ? do_syscall_64+0xaa/0x660
>  ? vfs_write+0x281/0x560
>  ? ksys_write+0x7b/0x110
>  ? do_syscall_64+0xaa/0x660
>  ? do_syscall_64+0x5f/0x660
>  ? clear_bhb_loop+0x30/0x80
>  ? clear_bhb_loop+0x30/0x80
>  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> RIP: 0033:0x440bcb
>
> Signed-off-by: Liav Albani <liavalb@gmail.com>
> ---
>  include/linux/mm_types.h |  1 +
>  mm/vma.c                 | 24 +++++++++++++-----------
>  2 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index b18c2b2e7d2c..5db0894e6064 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -981,6 +981,7 @@ struct vm_area_struct {
>  	unsigned long vm_pgoff;		/* Offset (within vm_file) in PAGE_SIZE
>  					   units */
>  	struct file * vm_file;		/* File we map to (can be NULL). */
> +	bool vm_file_doesnt_need_put; /* Do fput only if we did get_file... */

We definitely wouldn't add a field like this in the VMA type :) VMA fields
are in very short supply and the data structure is very sensitive on size
(it scales to a lot on many systems with many threads).

In general it feels like this is to pass things down to remove_vma(), but
that's an indicator really that you need to put it somehwere else.

>  	void * vm_private_data;		/* was vm_pte (shared mem) */
>
>  #ifdef CONFIG_SWAP
> diff --git a/mm/vma.c b/mm/vma.c
> index 9eea2850818a..0c2e71fc09be 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -467,7 +467,7 @@ void remove_vma(struct vm_area_struct *vma)
>  {
>  	might_sleep();
>  	vma_close(vma);
> -	if (vma->vm_file)
> +	if (vma->vm_file && !vma->vm_file_doesnt_need_put)
>  		fput(vma->vm_file);
>  	mpol_put(vma_policy(vma));
>  	vm_area_free(vma);
> @@ -2487,6 +2487,7 @@ static int __mmap_new_file_vma(struct mmap_state *map,
>  	int error;
>
>  	vma->vm_file = map->file;
> +	vma->vm_file_doesnt_need_put = map->file_doesnt_need_get;
>  	if (!map->file_doesnt_need_get)
>  		get_file(map->file);
>
> @@ -2497,7 +2498,14 @@ static int __mmap_new_file_vma(struct mmap_state *map,
>  	if (error) {
>  		UNMAP_STATE(unmap, vmi, vma, vma->vm_start, vma->vm_end,
>  			    map->prev, map->next);
> -		fput(vma->vm_file);
> +
> +		/*
> +		 * We set vma->vm_file to map->file, so if we didn't increase
> +		 * the refcount, don't decrease it in this path.
> +		 */
> +		if (!map->file_doesnt_need_get)
> +			fput(vma->vm_file);
> +

But this just means the file is unpinned in general and you really want its
lifetime to be at least that of the mapping :>)

>  		vma->vm_file = NULL;
>
>  		vma_iter_set(vmi, vma->vm_end);
> @@ -2757,7 +2765,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
>  	if (!error && have_mmap_prepare)
>  		error = call_mmap_prepare(&map, &desc);
>  	if (error)
> -		goto abort_munmap;
> +		goto abort_mmap_prepare;

Yeah, as above, this is incorrect.

>
>  	if (map.check_ksm_early)
>  		update_ksm_flags(&map);
> @@ -2795,15 +2803,9 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
>  unacct_error:
>  	if (map.charged)
>  		vm_unacct_memory(map.charged);
> -abort_munmap:
> -	/*
> -	 * This indicates that .mmap_prepare has set a new file, differing from
> -	 * desc->vm_file. But since we're aborting the operation, only the
> -	 * original file will be cleaned up. Ensure we clean up both.
> -	 */
> -	if (map.file_doesnt_need_get)
> -		fput(map.file);
> +
>  	vms_abort_munmap_vmas(&map.vms, &map.mas_detach);
> +abort_mmap_prepare:
>  	return error;
>  }
>
> --
> 2.55.0
>

In general I can see you're a genuine kernel enthusiast so I don't want to
dissuade you :)

This code is fiddly in general so it's _entirely_ understandable you had
some misapprehensions about it.

Hopefully the feedback helps and you are able to continue implementing
your feature, do let me know if you're able to do what you need with your
driver (mmap_prepare is in live development and I'm gradually adding more
actions to suit actual driver usage).

It's actually nice to see something human rather than LLM for some early
contributions :)

Cheers, Lorenzo

Re: [PATCH] mm/vma: fix imbalanced file reference count, properly abort mmap_prepare

[Liav Albani]

[-- Attachment #1: Type: text/plain, Size: 8793 bytes --]

Hi and thanks for your reply (both of you)!

I see that mmap_prepare essentially creates a shmem_file on each mmap call
for /dev/zero, so it's like donating a ref, right?
I will try that because it looks good to me :)

There is the second part to this patch and I was about to ask if we
actually want to do the abort_munmap path when aborting because of failed
mmap_prepare
and it seems like Lorenzo already gave the answer to this :)

I will let you all know about the results and see if it works fine.

Again thanks for your help and time!
Liav


‫בתאריך שבת, 18 ביולי 2026 ב-17:18 מאת ‪Lorenzo Stoakes (ARM)‬‏ <‪
ljs@kernel.org‬‏>:‬

> Thanks for the patch but as per the 0/1 this is not correct - the issue is
> that
> your mmap_prepare hook doesn't get the file.
>
> (Also in general, we don't add cover letters for single patches, only for
> series
> with >1 patch :)
>
> On Sat, Jul 18, 2026 at 03:34:17PM +0300, Liav Albani wrote:
> > Under the new `vm_area_desc` struct, a driver may change the vm_file
> > to provide a different backing VM file.
> >
> > In such case, it became apparent during testing of this capability, that
> > a remove_vma() may drop a refcount on a VM file that we didn't call
> > get_file() upon.
>
> As above.
>
> >
> > In addition to that, calling vms_abort_munmap_vmas when we didn't do
> > actual work is wrong, because mmap_prepare hook is about preparing the
> > mmap state and not performing any actual mapping (yet).
>
> Not the case - __mmap_setup() unmaps any existing mappings in the area
> being mapped into (they are taken out of the maple tree and put into a
> detached state then added to the temporary map->mt_detach tree), and
> vms_abort_munmap_vmas() will put them back.
>
> The vms data structure is set up in int_vma_munmap(), then
> vms_gather_munmap_vmas() populates both map->vms and map->mas_detach.
>
> If nothing was unmapped map->vms->nr_pages == 0, and
> vms_abort_munmap_vmas() is a noop.
>
> >
> > A kernel stack (before of this patch) could look like this:
> >
> > imbalanced put on file reference count
> > WARNING: fs/file.c:36 at __file_ref_put_badval.isra.0+0x3f/0x60, CPU#1:
> test_proxy_mmap/116
> > Modules linked in: ufedm(OE) virtio_net net_failover failover nandsim
> nand nandcore bch mtd
> > CPU: 1 UID: 0 PID: 116 Comm: test_proxy_mmap Tainted: G           OE
>    7.2.0-rc3-gaf5e34a41cd6 #1 PREEMPT(full)
> bdadc55b51524b311c430c1536a97e7071150fd3
> > Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux
> 1.17.0-2-2 04/01/2014
> > RIP: 0010:__file_ref_put_badval.isra.0+0x3f/0x60
> > Code: 85 f6 78 05 c3 cc cc cc cc 48 b8 00 00 00 00 00 00 00 a0 48 89 07
> c3 cc cc cc cc 48 83 ec 08 48 89 3c 24 48 8d 3d 61 bc 30 02 <67> 48 0f b9
> 3a 48 ba 00 00 00 00 00 00 00 e0 48 8b 04 24 48 89 10
> > RSP: 0018:ffffc900006bbae8 EFLAGS: 00010296
> > RAX: bfffffffffffffff RBX: ffff88810409a180 RCX: 0000000038748000
> > RDX: ffff888100a44480 RSI: dfffffffffffffff RDI: ffffffff83b12730
> > RBP: ffffc900006bbbf8 R08: 0000000000000000 R09: 00007f8b49f8f000
> > R10: ffffc900006bbba0 R11: ffff88810409a208 R12: ffffc900006bbba0
> > R13: 00007f8b4a179000 R14: ffff888102222440 R15: 0000000000015040
> > FS:  0000000038726400(0000) GS:ffff8881b77d1000(0000)
> knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000038728618 CR3: 0000000104a1c006 CR4: 0000000000370ef0
> > Call Trace:
> >  <TASK>
> >  __file_ref_put+0x30/0x40
> >  fput+0x4f/0x80
> >  remove_vma+0x42/0x60
> >  vms_complete_munmap_vmas+0x179/0x230
> >  do_vmi_align_munmap+0x225/0x2a0
> >  do_vmi_munmap+0xe6/0x1c0
> >  __vm_munmap+0x113/0x200
> >  __x64_sys_munmap+0x1b/0x30
> >  do_syscall_64+0xaa/0x660
> >  ? ksys_mmap_pgoff+0x168/0x200
> >  ? do_syscall_64+0xaa/0x660
> >  ? vfs_write+0x281/0x560
> >  ? ksys_write+0x7b/0x110
> >  ? do_syscall_64+0xaa/0x660
> >  ? do_syscall_64+0x5f/0x660
> >  ? clear_bhb_loop+0x30/0x80
> >  ? clear_bhb_loop+0x30/0x80
> >  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> > RIP: 0033:0x440bcb
> >
> > Signed-off-by: Liav Albani <liavalb@gmail.com>
> > ---
> >  include/linux/mm_types.h |  1 +
> >  mm/vma.c                 | 24 +++++++++++++-----------
> >  2 files changed, 14 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index b18c2b2e7d2c..5db0894e6064 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -981,6 +981,7 @@ struct vm_area_struct {
> >       unsigned long vm_pgoff;         /* Offset (within vm_file) in
> PAGE_SIZE
> >                                          units */
> >       struct file * vm_file;          /* File we map to (can be NULL). */
> > +     bool vm_file_doesnt_need_put; /* Do fput only if we did
> get_file... */
>
> We definitely wouldn't add a field like this in the VMA type :) VMA fields
> are in very short supply and the data structure is very sensitive on size
> (it scales to a lot on many systems with many threads).
>
> In general it feels like this is to pass things down to remove_vma(), but
> that's an indicator really that you need to put it somehwere else.
>
> >       void * vm_private_data;         /* was vm_pte (shared mem) */
> >
> >  #ifdef CONFIG_SWAP
> > diff --git a/mm/vma.c b/mm/vma.c
> > index 9eea2850818a..0c2e71fc09be 100644
> > --- a/mm/vma.c
> > +++ b/mm/vma.c
> > @@ -467,7 +467,7 @@ void remove_vma(struct vm_area_struct *vma)
> >  {
> >       might_sleep();
> >       vma_close(vma);
> > -     if (vma->vm_file)
> > +     if (vma->vm_file && !vma->vm_file_doesnt_need_put)
> >               fput(vma->vm_file);
> >       mpol_put(vma_policy(vma));
> >       vm_area_free(vma);
> > @@ -2487,6 +2487,7 @@ static int __mmap_new_file_vma(struct mmap_state
> *map,
> >       int error;
> >
> >       vma->vm_file = map->file;
> > +     vma->vm_file_doesnt_need_put = map->file_doesnt_need_get;
> >       if (!map->file_doesnt_need_get)
> >               get_file(map->file);
> >
> > @@ -2497,7 +2498,14 @@ static int __mmap_new_file_vma(struct mmap_state
> *map,
> >       if (error) {
> >               UNMAP_STATE(unmap, vmi, vma, vma->vm_start, vma->vm_end,
> >                           map->prev, map->next);
> > -             fput(vma->vm_file);
> > +
> > +             /*
> > +              * We set vma->vm_file to map->file, so if we didn't
> increase
> > +              * the refcount, don't decrease it in this path.
> > +              */
> > +             if (!map->file_doesnt_need_get)
> > +                     fput(vma->vm_file);
> > +
>
> But this just means the file is unpinned in general and you really want its
> lifetime to be at least that of the mapping :>)
>
> >               vma->vm_file = NULL;
> >
> >               vma_iter_set(vmi, vma->vm_end);
> > @@ -2757,7 +2765,7 @@ static unsigned long __mmap_region(struct file
> *file, unsigned long addr,
> >       if (!error && have_mmap_prepare)
> >               error = call_mmap_prepare(&map, &desc);
> >       if (error)
> > -             goto abort_munmap;
> > +             goto abort_mmap_prepare;
>
> Yeah, as above, this is incorrect.
>
> >
> >       if (map.check_ksm_early)
> >               update_ksm_flags(&map);
> > @@ -2795,15 +2803,9 @@ static unsigned long __mmap_region(struct file
> *file, unsigned long addr,
> >  unacct_error:
> >       if (map.charged)
> >               vm_unacct_memory(map.charged);
> > -abort_munmap:
> > -     /*
> > -      * This indicates that .mmap_prepare has set a new file, differing
> from
> > -      * desc->vm_file. But since we're aborting the operation, only the
> > -      * original file will be cleaned up. Ensure we clean up both.
> > -      */
> > -     if (map.file_doesnt_need_get)
> > -             fput(map.file);
> > +
> >       vms_abort_munmap_vmas(&map.vms, &map.mas_detach);
> > +abort_mmap_prepare:
> >       return error;
> >  }
> >
> > --
> > 2.55.0
> >
>
> In general I can see you're a genuine kernel enthusiast so I don't want to
> dissuade you :)
>
> This code is fiddly in general so it's _entirely_ understandable you had
> some misapprehensions about it.
>
> Hopefully the feedback helps and you are able to continue implementing
> your feature, do let me know if you're able to do what you need with your
> driver (mmap_prepare is in live development and I'm gradually adding more
> actions to suit actual driver usage).
>
> It's actually nice to see something human rather than LLM for some early
> contributions :)
>
> Cheers, Lorenzo
>

[-- Attachment #2: Type: text/html, Size: 10605 bytes --]