Date: Tue, 17 Jun 2008 23:40:12 -0700 From: Julian Elischer <julian@elischer.org> To: James Gritton <jamie@gritton.org> Cc: freebsd-virtualization@freebsd.org Subject: Re: V_* meta-symbols and locking Message-ID: <4858ADCC.1050909@elischer.org> In-Reply-To: <48588595.7020709@gritton.org> References: <48588595.7020709@gritton.org>
next in thread | previous in thread | raw e-mail | index | archive | help
James Gritton wrote:
> Like everything I have to say about the V_* issue, perhaps this doesn't
> apply to the vnet stuff. But to the two symbols I currently care about,
> hostname and rootvnode, locking is a problem.
>
yes and I for one have probably not thought enough about it.
> Current kernel code plays fast and loose with both these symbols. Check
> out getcredhostname for example:
>
> void
> getcredhostname(struct ucred *cred, char *buf, size_t size)
> {
> struct prison *pr;
>
> pr = cred->cr_prison;
> if (pr != &prison0) {
> mtx_lock(&pr->pr_mtx);
> strlcpy(buf, (pr->pr_flags & PR_NOHOST)
> ? hostname : pr->pr_host, size);
> mtx_unlock(&pr->pr_mtx);
> } else
> strlcpy(buf, hostname, size);
> }
>
> In the prison case, it nicely locks the prison record. But for the
> global hostname, it just copies it. The hostname sysctl is no better
> about setting it. And rootvnode is referred to all over the place
> without any sort of lock - pretty safe since it's not expected to change
> (though it theoretically can).
I'm not sure there is much of a problem because the hostname
associated with a virtual machine is a fixed array of bytes.
it is true that one might be able (though unlikely) to get half of one
hostname and half of another but you will never get invalid memory..
I think that the only readers of the hostname in a vm are processes in
that VM so the VM is not going anywhere and thus the hostname is not
going anywhere..
>
> This same no-locking assumption seems to be going on with V_hostname.
> But now this macro applies not only to the "real" hostname but to the
> "virtual" one as well - no locking the vimage record. As I try to add a
> similar macro to my new jail framework, I find I can't. Instead of a
> mere variable redirection, I need to lock-copy-unlock much like
> getcredhostname does. Luckily, much hostname access is already
> jail-aware. But anything using the "real" hostname should have the same
> locking on prison0. Perhaps not wholly necessary since it's just a
> string that we know will always have a null byte at the end of the
> buffer, but still good form and unknown prevention. And in the case of
> actually virtual hostnames, it's essential since they'll be changing
> from fixed arrays in struct prison into pointers that may be freed.
I think in the vimage code it is not freeable unless the vimage is
freed and in that case there is no-one to read the string.
vimage0 is of course not going away under any situation.
>
> Rootvnode is a stickier problem. There's much more code that refers to
> it, and it's a more essential part of the system. I don't relish
> digging in everywhere and changing the whole rootvnode paradigm with
> locking. So instead my solution is to make the jail "path" parameter
> (and thus root vnode) set-once. So as long as the V_rootvnode is taken
> from a context that will remain for the duration of its use (curthread
> is a good bet), it will be safe to access it without locks. In
> particular, the real rootvnode that lives at prison0 isn't going anywhere.
teh man page for vimage(8) says for the chroot parameter:
chroot
Set the chroot directory for the virtual image. All new processes
spawned into the target virtual image using the vimage command
will be initially chrooted to that directory. This parameter can
be changed only when no processes are running within the target
virtual image. Note that it is not required to have a chrooted
environment for a virtual image operate, which is also the
default behavior.
so the croot is fixed unless there is no-one using it.
>
> So in summary:
>
> I won't use V_hostname (or G_hostname), opting for explicit locking.
I'm not sure you need this.
>
> I will V_rootvnode (and perhaps G_rootvnode).
>
> All the other network-related V_stuff may deserve a look, but it out of
> my purview.
>
> - Jamie
> _______________________________________________
> freebsd-virtualization@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
> To unsubscribe, send any mail to
> "freebsd-virtualization-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4858ADCC.1050909>