>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@xxxxxxxxxxxxxxxxxxxxx/ >> >> Fixes: ff7ec8dc1b64 ("proc: use the same treatment to check proc_lseek as ones for proc_read_iter et.al) >> Cc: stable@xxxxxxxxxxxxxxx >> Reported-by: Lars Wendler <polynomial-c@xxxxxx> >> Signed-off-by: wangzijie <wangzijie1@xxxxxxxxx> >> --- >> 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 >>
