From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Pedro Falcato <pfalcato@suse.de>
Cc: Liav Albani <liavalb@gmail.com>,
akpm@linux-foundation.org, david@kernel.org, liam@infradead.org,
vbabka@kernel.org, rppt@kernel.org, surenb@google.com,
mhocko@suse.com, jannh@google.com, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/1] Fix an imbalanced file ref count in the mmap syscall
Date: Sat, 18 Jul 2026 15:05:25 +0100 [thread overview]
Message-ID: <aluHg-QYRAPoFPof@lucifer> (raw)
In-Reply-To: <alt9VpLcRFHAiy8q@pedro-suse.lan>
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
prev parent reply other threads:[~2026-07-18 14:05 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-18 12:34 [PATCH 0/1] Fix an imbalanced file ref count in the mmap syscall Liav Albani
2026-07-18 12:34 ` [PATCH] mm/vma: fix imbalanced file reference count, properly abort mmap_prepare Liav Albani
2026-07-18 14:17 ` Lorenzo Stoakes (ARM)
2026-07-18 14:25 ` Liav Albani
2026-07-18 13:25 ` [PATCH 0/1] Fix an imbalanced file ref count in the mmap syscall Pedro Falcato
2026-07-18 14:05 ` Lorenzo Stoakes (ARM) [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aluHg-QYRAPoFPof@lucifer \
--to=ljs@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=david@kernel.org \
--cc=jannh@google.com \
--cc=liam@infradead.org \
--cc=liavalb@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=pfalcato@suse.de \
--cc=rppt@kernel.org \
--cc=surenb@google.com \
--cc=vbabka@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox