From owner-freebsd-virtualization@FreeBSD.ORG Wed Jun 18 06:40:06 2008 Return-Path: Delivered-To: freebsd-virtualization@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C055010656A9 for ; Wed, 18 Jun 2008 06:40:06 +0000 (UTC) (envelope-from julian@elischer.org) Received: from outQ.internet-mail-service.net (outq.internet-mail-service.net [216.240.47.240]) by mx1.freebsd.org (Postfix) with ESMTP id A4D218FC28 for ; Wed, 18 Jun 2008 06:40:06 +0000 (UTC) (envelope-from julian@elischer.org) Received: from idiom.com (mx0.idiom.com [216.240.32.160]) by out.internet-mail-service.net (Postfix) with ESMTP id 4D2222447; Tue, 17 Jun 2008 23:40:06 -0700 (PDT) Received: from julian-mac.elischer.org (localhost [127.0.0.1]) by idiom.com (Postfix) with ESMTP id 020882D6006; Tue, 17 Jun 2008 23:40:05 -0700 (PDT) Message-ID: <4858ADCC.1050909@elischer.org> Date: Tue, 17 Jun 2008 23:40:12 -0700 From: Julian Elischer User-Agent: Thunderbird 2.0.0.14 (Macintosh/20080421) MIME-Version: 1.0 To: James Gritton References: <48588595.7020709@gritton.org> In-Reply-To: <48588595.7020709@gritton.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-virtualization@freebsd.org Subject: Re: V_* meta-symbols and locking X-BeenThere: freebsd-virtualization@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussion of various virtualization techniques FreeBSD supports." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Jun 2008 06:40:06 -0000 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"