Re: [PATCH v2] kernfs: fix race to increment for nr_mmapped in kernfs_fop_mmap()

On Thu, Jul 30, 2026 at 06:09:12PM -0700, Kevin Mitchell wrote:
> Counts of files to be released (nr_to_release) and mmapped (nr_mmapped)
> files were added to kernfs_open_node in commit bdb2fd7fc56e ("kernfs:
> Skip kernfs_drain_open_files() more aggressively") to optimize
> kernfs_drain_open_files(). A WARN_ON_ONCE sanity check was also added in
> kernfs_drain_open_files() to ensure that these counters were brought to
> zero once all files had been drained.
> 
> Modifications to these counters were protected by kernfs_open_file_mutex
> everywhere except for in kernfs_fop_mmap(). This caused a race condition
> where some nr_mmapped increments could get overwritten even while the
> correct number of kernfs_open_files with mmapped == true were present in
> the kernfs_open_node's files list. Consequently, the iteration in
> kernfs_drain_open_files() would underflow nr_mmapped and the WARNING
> would fire.
> 
> To fix this, acquire kernfs_open_file_mutex around nr_mmapped updates in
> kernfs_fop_mmap.
> 
> The nesting of->mutex -> kernfs_open_file_mutex is safe as
> kernfs_open_file_mutex is acquired last avoiding the possible cycles
> highlighted in commit f83f3c515654 ("kernfs: fix locking around
> kernfs_ops->release() callback").
> 
> Fixes: bdb2fd7fc56e ("kernfs: Skip kernfs_drain_open_files() more aggressively")
> Signed-off-by: Kevin Mitchell <[email protected]>
> ---
> Changes in v2:
> - Rework the approach to use kernfs_open_file_mutex_lock instead of
>   atomics based on feedback from Greg KH.
> - Link to v1: https://lore.kernel.org/all/[email protected]
> 
>  fs/kernfs/file.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
> index 8e0e90c93372..6f63eb6be1cd 100644
> --- a/fs/kernfs/file.c
> +++ b/fs/kernfs/file.c
> @@ -495,8 +495,12 @@ static int kernfs_fop_mmap(struct file *file, struct vm_area_struct *vma)
>  
>  	rc = 0;
>  	if (!of->mmapped) {
> -		of->mmapped = true;
> +		struct mutex *mutex = kernfs_open_file_mutex_lock(of->kn);
> +
>  		of_on(of)->nr_mmapped++;
> +		mutex_unlock(mutex)
> +;

That line looks very odd, didn't checkpatch catch it?