On Mon 02-11-26 15:43:28, Christian Brauner wrote:
> proc_get_link() pins the entry with use_pde() and the delayed call
> unpins it. vfs_readlink() copies the link to userspace in between. So a
> page fault in that copy holds up remove_proc_entry() just like the
> copies of reads and writes do, and the callers hold locks while they
> wait. /proc/asound/<id> is removed with snd_card_mutex and info_mutex
> held when a card changes its id.
>
> The pin doesn't keep anything alive. proc_symlink() copies the target
> and pde_free() frees it with the entry, and the inode holds a reference
> to the entry. What the pin does is keep a removed symlink from being
> followed. So check ->in_use instead and drop the delayed call. That
> also takes an atomic pair off every traversal of /proc/net and
> /proc/mounts.
>
> Fixes: 7e0e953bb0cf ("procfs: fix race between symlink removals and
> traversals")
> Cc: [email protected]
> Signed-off-by: Christian Brauner (Amutable) <[email protected]>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <[email protected]>
Honza
> ---
> fs/proc/inode.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/fs/proc/inode.c b/fs/proc/inode.c
> index 9933f03ab9fb..2d65f3c8ec03 100644
> --- a/fs/proc/inode.c
> +++ b/fs/proc/inode.c
> @@ -629,19 +629,19 @@ static const struct file_operations
> proc_iter_file_ops_compat = {
> };
> #endif
>
> -static void proc_put_link(void *p)
> -{
> - unuse_pde(p);
> -}
> -
> static const char *proc_get_link(struct dentry *dentry,
> struct inode *inode,
> struct delayed_call *done)
> {
> struct proc_dir_entry *pde = PDE(inode);
> - if (!use_pde(pde))
> +
> + /*
> + * No pin. The link lives with the entry, which the inode holds, and a
> + * reader could fault while vfs_readlink() copies it out. Just don't
> + * follow a removed one.
> + */
> + if (unlikely(atomic_read(&pde->in_use) < 0))
> return ERR_PTR(-EINVAL);
> - set_delayed_call(done, proc_put_link, pde);
> return pde->data;
> }
>
>
> --
> 2.53.0
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
