On Sat, Mar 01, 2025 at 07:46:13AM +0300, Alexey Dobriyan wrote:
> On Sat, Mar 01, 2025 at 11:40:24AM +0800, Ye Bin wrote:
> > There's a issue as follows:
> > BUG: unable to handle page fault for address: fffffbfff80a702b
>
> > Above issue may happen as follows:
> > rmmod lookup
> > sys_delete_module
> > proc_lookup_de
> > read_lock(&proc_subdir_lock);
> > pde_get(de);
> > read_unlock(&proc_subdir_lock);
> > proc_get_inode(dir->i_sb, de);
> > mod->exit()
> > proc_remove
> > remove_proc_subtree
> > write_lock(&proc_subdir_lock);
> > write_unlock(&proc_subdir_lock);
> > proc_entry_rundown(de);
> > free_module(mod);
> >
> > if (S_ISREG(inode->i_mode))
> > if (de->proc_ops->proc_read_iter)
> > --> As module is already freed, will trigger UAF
>
> Hey look, vintage 17.5 year old /proc bug.
> This just shows how long I didn't ran rmmod test. :-(
>
> > To solve above issue there's need to get 'in_use' before use proc_dir_entry
> > in proc_get_inode().
> >
> > Fixes: fd5a13f4893c ("proc: add a read_iter method to proc proc_ops")
>
> OK, this is copy of the original sin below.
>
> > Fixes: 778f3dd5a13c ("Fix procfs compat_ioctl regression")
>
> This one is.
>
> Let me think a little.
>
> > --- a/fs/proc/inode.c
> > +++ b/fs/proc/inode.c
> > @@ -644,6 +644,11 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
> > return inode;
> > }
> >
> > + if (!pde_is_permanent(de) && !use_pde(de)) {
> > + pde_put(de);
> > + return NULL;
> > + }
> > +
> > if (de->mode) {
> > inode->i_mode = de->mode;
> > inode->i_uid = de->uid;
> > @@ -677,5 +682,9 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
> > } else {
> > BUG();
> > }
> > +
> > + if (!pde_is_permanent(de))
> > + unuse_pde(de);
> > +
I can't reproduce. Can you test new patch -- it avoid 2 atomic ops on
common path.
If the bug is looking into pde->proc_ops, then don't do it.
