From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:41982) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QAkq3-0004g9-8T for qemu-devel@nongnu.org; Fri, 15 Apr 2011 11:21:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QAkpz-0001L7-Iz for qemu-devel@nongnu.org; Fri, 15 Apr 2011 11:21:39 -0400 Received: from e4.ny.us.ibm.com ([32.97.182.144]:45029) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QAkpz-0000wU-Fb for qemu-devel@nongnu.org; Fri, 15 Apr 2011 11:21:35 -0400 Received: from d01relay02.pok.ibm.com (d01relay02.pok.ibm.com [9.56.227.234]) by e4.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p3FF1P9M016914 for ; Fri, 15 Apr 2011 11:01:25 -0400 Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d01relay02.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p3FFGKqP282770 for ; Fri, 15 Apr 2011 11:21:31 -0400 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p3FF2DKL008941 for ; Fri, 15 Apr 2011 09:02:14 -0600 Message-ID: <4DA85DF3.4010605@linux.vnet.ibm.com> Date: Fri, 15 Apr 2011 08:02:11 -0700 From: Venkateswararao Jujjuri MIME-Version: 1.0 References: <1302858357-11309-1-git-send-email-mohan@in.ibm.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] virtio-9p: Make rpath function thread safe List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: "M. Mohan Kumar" , qemu-devel@nongnu.org On 04/15/2011 02:42 AM, Stefan Hajnoczi wrote: > On Fri, Apr 15, 2011 at 10:05 AM, M. Mohan Kumar wrote: >> Use dynamically allocated memory to return concatenated path >> (fs_root and file path) instead of a static buffer. Caller has to free >> the memory. >> >> Signed-off-by: M. Mohan Kumar >> --- >> This patch depends on my chroot patchset. >> http://patchwork.ozlabs.org/patch/89033/ >> >> hw/9pfs/virtio-9p-local.c | 551 ++++++++++++++++++++++++++------------------- >> hw/file-op-9p.h | 10 +- >> 2 files changed, 322 insertions(+), 239 deletions(-) > Would it be possible to build a const char *fs_ctx->path once in the > PDU handler for each operation that takes a path instead of pushing so > many rpath() calls down into virtio-9p-local.c? That way you don't > need to worry about constructing and freeing paths over and over. This is very good suggestion. I think we should explore this. > I think it's time to kill errno once and for all in virtio-9p. The > errno global variable usage is getting out of hand; pretty much every > patch is adding boilerplate code to juggle around errno and avoid > trampling it. This new qemu_free() call means we need to juggle in > new places. > > Please stop adding more errno juggling and just use -errno return > values. That change needs to happen before this patch. This is ideal and we should do it where ever possible. But I am not sure if this is really achievable. In the case of passthrough we need to pass this information from chroot() process to qemu. In other security models we don't have 1-1 correspondence between 9p operations and system calls. Will catch you on IRC for further discussion. Thanks, JV >> @@ -137,29 +164,10 @@ static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf) >> } >> if (getxattr(rpath(fs_ctx, path), "user.virtfs.rdev",&tmp_dev, >> sizeof(dev_t))> 0) { > Leak. > >> tsize = readlink(rpath(fs_ctx, path), buf, bufsz); > Leak. > >> -static void local_rewinddir(FsContext *ctx, DIR *dir) >> +static void local_rewinddir(FsContext *fs_ctx, DIR *dir) >> { >> return rewinddir(dir); >> } > Please do this in a separate patch. > >> err = local_set_xattr(rpath(fs_ctx, newpath), credp); > Leak. > > Stefan >