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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.