k.shutemov no longer @ intel -> use gmail.
On 18. 08. 25, 10:08, Jiri Slaby wrote:
On 18. 08. 25, 10:01, wangzijie wrote:
Hi,
On 18. 08. 25, 6:05, wangzijie wrote:
For avoiding pde->proc_ops->... dereference(which may cause UAF in rmmod race scene), we call pde_set_flags() to save this kind of information in PDE itself before proc_register() and call pde_has_proc_XXX() to replace pde-
proc_ops->... dereference.
But there has omission of pde_set_flags() in net related proc file create, which cause the wroing behavior of FMODE_LSEEK clearing in proc_reg_open() for net related proc file after commit ff7ec8dc1b64("proc: use the same treatment to check proc_lseek as ones for proc_read_iter et.al"). Lars reported it in this link[1]. So call pde_set_flags() when create net related proc file to fix this bug.
I wonder, why is pde_set_flags() not a part of proc_register()?
Not all proc_dir_entry have proc_ops, so you mean this will be a better modification?
diff --git a/fs/proc/generic.c b/fs/proc/generic.c index 76e800e38..d52197c35 100644 --- a/fs/proc/generic.c +++ b/fs/proc/generic.c @@ -367,6 +367,20 @@ static const struct inode_operations proc_dir_inode_operations = { .setattr = proc_notify_change, };
+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; +}
/* 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 +388,9 @@ struct proc_dir_entry *proc_register(struct proc_dir_entry *dir, if (proc_alloc_inum(&dp->low_ino)) goto out_free_entry;
+ if (dp->proc_ops)
And to be honest, I would possibly move the 'if' into pde_set_flags().
+ pde_set_flags(dp);
As here, it is not completely clear why you would want to test "dp->proc_ops" for something called "pde_set_flags".