On Fri, 2015-11-13 at 15:49 +0000, Andrew Cooper wrote:
> On 09/11/15 12:00, Ian Campbell wrote:
> > diff --git a/tools/libs/call/linux.c b/tools/libs/call/linux.c
> > new file mode 100644
> > index 0000000..906ca7e
> > --- /dev/null
> > +++ b/tools/libs/call/linux.c
> > @@ -0,0 +1,132 @@
> > +/*
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation;
> > + * version 2.1 of the License.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; If not, see <http://www.gnu.org/li
> > censes/>.
> > + *
> > + * Split out from xc_linus_osdep.c:
> > + *
> > + * Copyright 2006 Sun Microsystems, Inc. All rights reserved.
> > + */
> > +
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <unistd.h>
> > +
> > +#include <sys/mman.h>
> > +#include <sys/ioctl.h>
> > +
> > +#include "private.h"
> > +
> > +int osdep_xencall_open(xencall_handle *xcall)
> > +{
> > + int flags, saved_errno;
> > + int fd = open("/proc/xen/privcmd", O_RDWR);
>
> I know this is a straight copy, but these new libraries really should
> start with the PVOps interface of /dev/xen/privcmd before falling back
> to the older classic-linux interfaces under /proc
Right, I'll pick this up for (almost, modulo resolving a conflict or two)
free when I rebase over Doug's patch to do this, which I'll be doing before
reposting.
> Ideally, once all these library improvements are done, there should be
> no need to mount xenfs in a Linux PVops domain.
>
> > +
> > + if ( fd == -1 )
> > + {
> > + PERROR("Could not obtain handle on privileged command
> > interface");
> > + return -1;
> > + }
> > +
> > + /* Although we return the file handle as the 'xc handle' the API
> > + does not specify / guarentee that this integer is in fact
> > + a file handle. Thus we must take responsiblity to ensure
> > + it doesn't propagate (ie leak) outside the process */
>
> Again, I know this is a straight copy, but it is racy. The open() call
> needs O_CLOEXEC.
>
> There might be some issues here to do with older versions of libc.
Or indeed super old versions of Linux (O_CLOEXEC came with 2.6.23 according
to man).
I'm tempted to hedge and use O_CLOEXEC with /dev/xen/privcmd, and leave the
legacy /proc/xen/privcmd path doing it this way, after all we've managed to
get this far...
> > + /* Do not copy the VMA to child process on fork. Avoid the page
> > being COW
> > + on hypercall. */
> > + rc = madvise(p, npages * PAGE_SIZE, MADV_DONTFORK);
>
> This also seems racy, although it is unclear from the manpages how to
> avoid the race.
pthread_atfork() might be one approach, and libxencall already uses
pthreads (for locking the cache), so it wouldn't be a problem to add more
uses, I don't think.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
