Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
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


      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