Re: [PATCH] proc: fix use-after-free in proc_get_inode()

[Alexey Dobriyan <adobriyan@gmail.com>]

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);
> +
>  	return inode;
>  }