Date: Tue, 17 Jun 2008 21:48:37 -0600 From: James Gritton <jamie@gritton.org> To: freebsd-virtualization@freebsd.org Subject: V_* meta-symbols and locking Message-ID: <48588595.7020709@gritton.org>
next in thread | raw e-mail | index | archive | help
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.
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).
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.
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.
So in summary:
I won't use V_hostname (or G_hostname), opting for explicit locking.
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?48588595.7020709>