Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 26 Feb 2015 23:03:42 +0100
From:      Mateusz Guzik <mjguzik@gmail.com>
To:        Dmitry Chagin <dchagin@FreeBSD.org>
Cc:        src-committers@freebsd.org, svn-src-user@freebsd.org
Subject:   Re: svn commit: r279335 - in user/dchagin/lemul/sys: compat/linprocfs fs/procfs fs/pseudofs modules/procfs
Message-ID:  <20150226220342.GC3799@dft-labs.eu>
In-Reply-To: <201502262130.t1QLUfwf027872@svn.freebsd.org>
References:  <201502262130.t1QLUfwf027872@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150226220342.GC3799>