svn commit: r279335 - in user/dchagin/lemul/sys: compat/linprocfs fs/procfs fs/pseudofs modules/procfs

[Mateusz Guzik]

On Thu, Feb 26, 2015 at 09:30:41PM +0000, Dmitry Chagin wrote:
> +int
> +procfs_dofdlink(PFS_FILL_ARGS)
> +{
> +	char *fullpath, *freepath, *endfileno;
> +	struct filedesc *fdp;
> +	struct vnode *vp;
> +	struct file *fp;
> +	int fileno, error;
> +
> +	if (vnode_name == NULL)
> +		return (ENOENT);
> +
> +	fileno = (int)strtol(vnode_name, &endfileno, 10);
> +	if (fileno == 0 && (vnode_namelen > 1 ||
> +	    (vnode_namelen == 1 && vnode_name[0] != '0')))
> +		return (ENOENT);
> +	if (vnode_namelen != endfileno - vnode_name)
> +		return (ENOENT);
> +
> +	fdp = fdhold(p);
> +	if (fdp == NULL)
> +		return (ENOENT);
> +
> +	error = fget_unlocked(fdp, fileno, NULL, &fp, NULL);
> +	if (error != 0)
> +		goto out;
> +
> +	freepath = NULL;
> +	fullpath = "-";
> +	vp = fp->f_vnode;
> +	if (vp != NULL) {
> +		vref(vp);
> +		error = vn_fullpath(td, vp, &fullpath, &freepath);
> +		vrele(vp);
> +	}
> +	if (error == 0)
> +		error = sbuf_printf(sb, "%s", fullpath);
> +	if (freepath != NULL)
> +		free(freepath, M_TEMP);
> +	fdrop(fp, td);
> +
> + out:
> +	fddrop(fdp);
> +	return (error);
> +}
>


fdhold does not protect file descriptor table, it only makes sure struct
filedesc itself is not freed.

Here you need to lock it and inspect fd_refcnt. See e.g.
kern_proc_filedesc_out.

While this guarantees data consistency, is in fact still incorrect since
the process you are inspecing can exec  setuid in the meantime and thus
make security checks (if any performed) stale.

I have an old WIP patch which provides appropriate interfaces to ensure
stability of the process (no exit, no exec), but this needs additional
changes. HOpefully i'll have the time to deal with it in March.
-- 
Mateusz Guzik <mjguzik gmail.com>