From patchwork Mon May 20 17:31:24 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Aneesh Kumar K.V" X-Patchwork-Id: 245082 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 133B62C00C8 for ; Tue, 21 May 2013 03:32:01 +1000 (EST) Received: from localhost ([::1]:52559 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UeTwE-0001Ml-S6 for incoming@patchwork.ozlabs.org; Mon, 20 May 2013 13:31:58 -0400 Received: from eggs.gnu.org ([208.118.235.92]:50813) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UeTvt-0001Mb-5w for qemu-devel@nongnu.org; Mon, 20 May 2013 13:31:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UeTvr-0005CW-KK for qemu-devel@nongnu.org; Mon, 20 May 2013 13:31:37 -0400 Received: from e28smtp09.in.ibm.com ([122.248.162.9]:59607) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UeTvr-0005CI-21 for qemu-devel@nongnu.org; Mon, 20 May 2013 13:31:35 -0400 Received: from /spool/local by e28smtp09.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 20 May 2013 22:57:38 +0530 Received: from d28dlp03.in.ibm.com (9.184.220.128) by e28smtp09.in.ibm.com (192.168.1.139) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 20 May 2013 22:57:35 +0530 Received: from d28relay02.in.ibm.com (d28relay02.in.ibm.com [9.184.220.59]) by d28dlp03.in.ibm.com (Postfix) with ESMTP id 0C8581258023 for ; Mon, 20 May 2013 23:03:24 +0530 (IST) Received: from d28av02.in.ibm.com (d28av02.in.ibm.com [9.184.220.64]) by d28relay02.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r4KHVLgk57868370 for ; Mon, 20 May 2013 23:01:22 +0530 Received: from d28av02.in.ibm.com (loopback [127.0.0.1]) by d28av02.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r4KHVPM0020035 for ; Tue, 21 May 2013 03:31:25 +1000 Received: from skywalker.linux.vnet.ibm.com ([9.77.212.167]) by d28av02.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id r4KHVPAZ020030; Tue, 21 May 2013 03:31:25 +1000 From: "Aneesh Kumar K.V" To: Michael Tokarev , "M. Mohan Kumar" In-Reply-To: <877giu55oa.fsf@linux.vnet.ibm.com> References: <512EF3A0.5010007@msgid.tls.msk.ru> <87r4k0zgo1.fsf@in.ibm.com> <512F677E.2060403@msgid.tls.msk.ru> <87obf4z6xs.fsf@in.ibm.com> <5198FA06.1000802@msgid.tls.msk.ru> <877giu55oa.fsf@linux.vnet.ibm.com> User-Agent: Notmuch/0.15.2+52~gb714a80 (http://notmuchmail.org) Emacs/24.3.50.1 (x86_64-unknown-linux-gnu) Date: Mon, 20 May 2013 23:01:24 +0530 Message-ID: <8738th5zir.fsf@linux.vnet.ibm.com> MIME-Version: 1.0 X-TM-AS-MML: No X-Content-Scanned: Fidelis XPS MAILER x-cbid: 13052017-2674-0000-0000-0000090AAEEA X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] X-Received-From: 122.248.162.9 Cc: qemu-devel Subject: Re: [Qemu-devel] 9pfs: unWRITAble dirs with random errors in guest X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org "Aneesh Kumar K.V" writes: > Michael Tokarev writes: > >> Rehashing an old thread again... >> >> 28.02.2013 21:25, M. Mohan Kumar wrote: >>> Michael Tokarev writes: >>> >>> >>>> 28.02.2013 17:55, M. Mohan Kumar wrote: >>>>> Michael Tokarev writes: >>>>> >>>>> Hi, >>>>> >>>>> Please try mounting with -oversion=9p2000.L >>>>> >>>>> With qemu-1.4.0 and 9p2000.L, I could not recreate this issue. ie not >>>>> getting Unknown error during directory listing. >>>> >>>> Yes, with 9p2000.L it works fine, both reported >>>> issues are gone. >>>> >>>>> I am using Guest kernel 3.8.0-rc5+. >>>> >>>> But do you see my original error, with the default mount version? >>>> (I'm using 3.2 guest kernel if that matters, and it is also 32bits, >>>> which should not matter). >>> Yes, with default mount option, i am getting same error. >> >> So, is there any intention to fix that for the default mount >> options? Or should the corresponding protocol be just marked >> as broken and qemu to refuse its usage? I think it is either >> one or another, -- either fix or drop, that is, keeping >> known-broken behavor isn't exactly the right thing. > > I am trying to reproduce this. I guess there is some part of config i am > missing > > I have export path /tmp/ and is exported via > > -virtfs local,path=/tmp/,mount_tag=v_tmp,security_model=mapped-file > > There is not .virtfs_metadata directory in /tmp/ > > in /tmp/ i have test/test2 with > > ls -ald test/test2 > dr-xr-xr-x 2 kvaneesh kvaneesh 4096 May 20 15:20 test/test2 > > Now i am mounting the same via > mount -t 9p -oversion=9p2000.u,ro v_tmp /mnt > > root@qemu-guest:~# ls -al /mnt/test/test2/ > total 8 > dr-xr-xr-x 2 virtfs virtfs 4096 May 20 15:20 . > drwxr-xr-x 3 virtfs virtfs 4096 May 20 15:00 .. > -r--r--r-- 1 virtfs virtfs 0 May 20 15:20 k > > Any idea what i am missing ? With lots of help from Mohan, I was able to figure out the details. This turned out to be an error in client code related to how we are handling errors in zero copy request. If you want to try, I am attaching two patches below. Qemu patch is strictly not needed, But should get you the correct error. commit 9b97cc13e5c4455612ae09d72d2ae963c487c202 Author: Aneesh Kumar K.V Date: Mon May 20 22:55:22 2013 +0530 net/9p: Handle error in zero copy request correctly for 9p2000.u For zero copy request, error will be encoded in the user space buffer. So copy the error code correctly using copy_from_user. Here we use the extra bytes we allocate for zero copy request. If total error details are more than P9_ZC_HDR_SZ - 7 bytes, we return -EFAULT. The patch also avoid a memory allocation in the error path. Signed-off-by: Aneesh Kumar K.V diff --git a/net/9p/client.c b/net/9p/client.c index 5e94dab..01f1779 100644 --- a/net/9p/client.c +++ b/net/9p/client.c @@ -562,36 +562,19 @@ static int p9_check_zc_errors(struct p9_client *c, struct p9_req_t *req, if (!p9_is_proto_dotl(c)) { /* Error is reported in string format */ - uint16_t len; - /* 7 = header size for RERROR, 2 is the size of string len; */ - int inline_len = in_hdrlen - (7 + 2); + int len; + /* 7 = header size for RERROR; */ + int inline_len = in_hdrlen - 7; - /* Read the size of error string */ - err = p9pdu_readf(req->rc, c->proto_version, "w", &len); - if (err) - goto out_err; - - ename = kmalloc(len + 1, GFP_NOFS); - if (!ename) { - err = -ENOMEM; + len = req->rc->size - req->rc->offset; + if (len > (P9_ZC_HDR_SZ - 7)) { + err = -EFAULT; goto out_err; } - if (len <= inline_len) { - /* We have error in protocol buffer itself */ - if (pdu_read(req->rc, ename, len)) { - err = -EFAULT; - goto out_free; - } - } else { - /* - * Part of the data is in user space buffer. - */ - if (pdu_read(req->rc, ename, inline_len)) { - err = -EFAULT; - goto out_free; - - } + ename = &req->rc->sdata[req->rc->offset]; + if (len > inline_len) { + /* We have error in external buffer */ if (kern_buf) { memcpy(ename + inline_len, uidata, len - inline_len); @@ -600,19 +583,19 @@ static int p9_check_zc_errors(struct p9_client *c, struct p9_req_t *req, uidata, len - inline_len); if (err) { err = -EFAULT; - goto out_free; + goto out_err; } } } - ename[len] = 0; - if (p9_is_proto_dotu(c)) { - /* For dotu we also have error code */ - err = p9pdu_readf(req->rc, - c->proto_version, "d", &ecode); - if (err) - goto out_free; + ename = NULL; + err = p9pdu_readf(req->rc, c->proto_version, "s?d", + &ename, &ecode); + if (err) + goto out_err; + + if (p9_is_proto_dotu(c)) err = -ecode; - } + if (!err || !IS_ERR_VALUE(err)) { err = p9_errstr2errno(ename, strlen(ename)); @@ -628,8 +611,6 @@ static int p9_check_zc_errors(struct p9_client *c, struct p9_req_t *req, } return err; -out_free: - kfree(ename); out_err: p9_debug(P9_DEBUG_ERROR, "couldn't parse error%d\n", err); return err; commit 04a886359d01c297adeaa2328de5d2598fe77e07 Author: Aneesh Kumar K.V Date: Mon May 20 17:28:29 2013 +0530 hw/9pfs: use O_NOFOLLOW for mapped readlink operation With mapped security models like mapped-xattr and mapped-file, we save the symlink target as file contents. Now if we ever expose a normal directory with mapped security model and find real symlinks in export path, never follow them and return proper error. Signed-off-by: Aneesh Kumar K.V diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c index 6ece6f7..87aa75d 100644 --- a/hw/9pfs/virtio-9p-local.c +++ b/hw/9pfs/virtio-9p-local.c @@ -284,7 +284,7 @@ static ssize_t local_readlink(FsContext *fs_ctx, V9fsPath *fs_path, if ((fs_ctx->export_flags & V9FS_SM_MAPPED) || (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE)) { int fd; - fd = open(rpath(fs_ctx, path, buffer), O_RDONLY); + fd = open(rpath(fs_ctx, path, buffer), O_RDONLY | O_NOFOLLOW); if (fd == -1) { return -1; }