Re: [PATCH RESEND v2] proc: fix missing pde_set_flags() for net proc files

[wangzijie <wangzijie1@honor.com>]

>On Mon, Aug 18, 2025 at 08:31:02PM +0800, wangzijie wrote:
>> To avoid potential UAF issues during module removal races, we use pde_set_flags()
>> to save proc_ops flags in PDE itself before proc_register(), and then use
>> pde_has_proc_*() helpers instead of directly dereferencing pde->proc_ops->*.
>> 
>> However, the pde_set_flags() call was missing when creating net related proc files.
>> This omission caused incorrect behavior which FMODE_LSEEK was being cleared
>> inappropriately in proc_reg_open() for net proc files. Lars reported it in this link[1].
>> 
>> Fix this by ensuring pde_set_flags() is called when register proc entry, and add
>> NULL check for proc_ops in pde_set_flags().
>> 
>> [1]: https://lore.kernel.org/all/20250815195616.64497967@chagall.paradoxon.rec/
>> 
>> Fixes: ff7ec8dc1b64 ("proc: use the same treatment to check proc_lseek as ones for proc_read_iter et.al)
>> Cc: stable@vger.kernel.org
>> Reported-by: Lars Wendler <polynomial-c@gmx.de>
>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>> ---
>> v2:
>> - followed by Jiri's suggestion to refractor code and reformat commit message
>> ---
>>  fs/proc/generic.c | 36 +++++++++++++++++++-----------------
>>  1 file changed, 19 insertions(+), 17 deletions(-)
>> 
>> diff --git a/fs/proc/generic.c b/fs/proc/generic.c
>> index 76e800e38..003031839 100644
>> --- a/fs/proc/generic.c
>> +++ b/fs/proc/generic.c
>> @@ -367,6 +367,23 @@ static const struct inode_operations proc_dir_inode_operations = {
>>  	.setattr	= proc_notify_change,
>>  };
>>  
>> +static void pde_set_flags(struct proc_dir_entry *pde)
>> +{
>
>Stash pde->proc_ops in a local const variable instead of chasing the
>pointer multiple times. Aside from that also makes it easier to read.
>Otherwise seems fine.

Thanks for helping review, Christian. I will follow your suggestion.


>> +	if (!pde->proc_ops)
>> +		return;
>> +
>> +	if (pde->proc_ops->proc_flags & PROC_ENTRY_PERMANENT)
>> +		pde->flags |= PROC_ENTRY_PERMANENT;
>> +	if (pde->proc_ops->proc_read_iter)
>> +		pde->flags |= PROC_ENTRY_proc_read_iter;
>> +#ifdef CONFIG_COMPAT
>> +	if (pde->proc_ops->proc_compat_ioctl)
>> +		pde->flags |= PROC_ENTRY_proc_compat_ioctl;
>> +#endif
>> +	if (pde->proc_ops->proc_lseek)
>> +		pde->flags |= PROC_ENTRY_proc_lseek;
>> +}
>> +
>>  /* returns the registered entry, or frees dp and returns NULL on failure */
>>  struct proc_dir_entry *proc_register(struct proc_dir_entry *dir,
>>  		struct proc_dir_entry *dp)
>> @@ -374,6 +391,8 @@ struct proc_dir_entry *proc_register(struct proc_dir_entry *dir,
>>  	if (proc_alloc_inum(&dp->low_ino))
>>  		goto out_free_entry;
>>  
>> +	pde_set_flags(dp);
>> +
>>  	write_lock(&proc_subdir_lock);
>>  	dp->parent = dir;
>>  	if (pde_subdir_insert(dir, dp) == false) {
>> @@ -561,20 +580,6 @@ struct proc_dir_entry *proc_create_reg(const char *name, umode_t mode,
>>  	return p;
>>  }
>>  
>> -static void pde_set_flags(struct proc_dir_entry *pde)
>> -{
>> -	if (pde->proc_ops->proc_flags & PROC_ENTRY_PERMANENT)
>> -		pde->flags |= PROC_ENTRY_PERMANENT;
>> -	if (pde->proc_ops->proc_read_iter)
>> -		pde->flags |= PROC_ENTRY_proc_read_iter;
>> -#ifdef CONFIG_COMPAT
>> -	if (pde->proc_ops->proc_compat_ioctl)
>> -		pde->flags |= PROC_ENTRY_proc_compat_ioctl;
>> -#endif
>> -	if (pde->proc_ops->proc_lseek)
>> -		pde->flags |= PROC_ENTRY_proc_lseek;
>> -}
>> -
>>  struct proc_dir_entry *proc_create_data(const char *name, umode_t mode,
>>  		struct proc_dir_entry *parent,
>>  		const struct proc_ops *proc_ops, void *data)
>> @@ -585,7 +590,6 @@ struct proc_dir_entry *proc_create_data(const char *name, umode_t mode,
>>  	if (!p)
>>  		return NULL;
>>  	p->proc_ops = proc_ops;
>> -	pde_set_flags(p);
>>  	return proc_register(parent, p);
>>  }
>>  EXPORT_SYMBOL(proc_create_data);
>> @@ -636,7 +640,6 @@ struct proc_dir_entry *proc_create_seq_private(const char *name, umode_t mode,
>>  	p->proc_ops = &proc_seq_ops;
>>  	p->seq_ops = ops;
>>  	p->state_size = state_size;
>> -	pde_set_flags(p);
>>  	return proc_register(parent, p);
>>  }
>>  EXPORT_SYMBOL(proc_create_seq_private);
>> @@ -667,7 +670,6 @@ struct proc_dir_entry *proc_create_single_data(const char *name, umode_t mode,
>>  		return NULL;
>>  	p->proc_ops = &proc_single_ops;
>>  	p->single_show = show;
>> -	pde_set_flags(p);
>>  	return proc_register(parent, p);
>>  }
>>  EXPORT_SYMBOL(proc_create_single_data);
>> -- 
>> 2.25.1
>>