From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8AAFD14A60F for ; Sat, 18 Jul 2026 14:05:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784383543; cv=none; b=nfXsaoRhuCdilz5pDA8M9XO00G8zwZ1X3r3WQNFXzh50EMUTj4ORnz+wnlvGLa5D9HC2In2CUhXQaeZlrNfVbDLJksACRJZgjjrCBOGpwHtWOUpWq1VEg3YhrZS/1Ad+dnDcJvXC2g2VWghbUwpx3XUePw/IBCs/ZCxp/7eTxAk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784383543; c=relaxed/simple; bh=xgHdy6Dm8mXGwtxeilpiE+MISBJlx/X39xb+iqIO9C4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=q5EwXQ7ViZHG0kK0aLsIiOB+vofK8jNrd+xKxoI2xqX9IfH0BrwfYDh2rpF6j30zFwhOiecJM8Cgu0B6lPagRowhhNAWR/8knSVH7zWrjROhSAkca8q4cwkK7ritH8M8tKx+8fxyQYV0643iKsXNf2rEOSHXBGI4b02P7SSWCnE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=XiZHHtgv; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="XiZHHtgv" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7DA6C1F000E9; Sat, 18 Jul 2026 14:05:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1784383542; bh=5YboIhEy8Gf7Sbuxc0hEVi+sD8/vS/YgZJ3wCELGbNY=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=XiZHHtgv5ObrEyZ0pjsFqmHrk+8+cTiY6Yr+xwQBj2UAz5NslBYWQk3Y1HbsymMp/ 2ScgrKZwjxdrJik5V+jhEH2pa6xa2Sj60UrVOnyAx8nI2eC4c90ssxIFINxQMjHu0a y8cRk2G3J0HzfidzQC0P1/FTyGsN9QFpQQEZRE2jcOYUbl/Ndy9e77oGW9Ig3ti/F7 JsTERqNDt/nrc3eXYr2KvpbP6YSTuzejXjKOvEewH1pJG2lmzVmAfk3zRBMgABvDJ7 DSeTwoeELnIZzMuHP/hs8c7TcR0F7A0bb53gJ6T7Nu/5KqRuj/ruKk4rRQDX4OPtNl wmIUOs2E3RARg== Date: Sat, 18 Jul 2026 15:05:25 +0100 From: "Lorenzo Stoakes (ARM)" To: Pedro Falcato Cc: Liav Albani , 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 Message-ID: References: <20260718123417.16339-1-liavalb@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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: > > > > __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