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

[Liav Albani <liavalb@gmail.com>]

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