From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44367) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WKQTQ-0007Tb-M3 for qemu-devel@nongnu.org; Mon, 03 Mar 2014 05:52:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WKQTI-0006kp-81 for qemu-devel@nongnu.org; Mon, 03 Mar 2014 05:51:52 -0500 Received: from mail-pb0-x22a.google.com ([2607:f8b0:400e:c01::22a]:38528) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WKQTH-0006kZ-Sa for qemu-devel@nongnu.org; Mon, 03 Mar 2014 05:51:44 -0500 Received: by mail-pb0-f42.google.com with SMTP id rr13so3663429pbb.1 for ; Mon, 03 Mar 2014 02:51:42 -0800 (PST) Message-ID: <53145EB2.9080106@gmail.com> Date: Mon, 03 Mar 2014 18:51:30 +0800 From: Chen Gang MIME-Version: 1.0 References: <52EF68CA.9060604@gmail.com> <20140203103429.GB10408@redhat.com> <52EF71DC.3000309@gmail.com> <52F0C8BA.7020709@gmail.com> <20140204110631.GD5632@redhat.com> <52F0CD67.5070601@gmail.com> <87siry3l7t.fsf@linux.vnet.ibm.com> <52F17B5E.1050602@gmail.com> <52FF3182.9090106@gmail.com> <53097D8E.1030803@gmail.com> <87sir850ho.fsf@blackfin.pond.sub.org> <87ha7o3c5x.fsf@blackfin.pond.sub.org> <530FCBAD.10305@gmail.com> <531219CC.4050505@gmail.com> <53121A12.5050105@gmail.com> <53121A4B.70308@gmail.com> <53121A93.8080301@gmail.com> <87siqz65q0.fsf@blackfin.pond.sub.org> In-Reply-To: <87siqz65q0.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/3] hw/9pfs: use g_strdup_printf() instead of PATH_MAX limitation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: "Aneesh Kumar K.V" , aliguori@amazon.com, QEMU Developers On 03/03/2014 04:34 PM, Markus Armbruster wrote: > Chen Gang writes: > >> When path is truncated by PATH_MAX limitation, it causes QEMU to access >> incorrect file. So use original full path instead of PATH_MAX within >> 9pfs (need check/process ENOMEM for related memory allocation). >> >> The related test: > [...] >> Signed-off-by: Chen Gang >> --- >> hw/9pfs/cofs.c | 15 ++- >> hw/9pfs/virtio-9p-handle.c | 9 +- >> hw/9pfs/virtio-9p-local.c | 285 +++++++++++++++++++++++++++-------------- >> hw/9pfs/virtio-9p-posix-acl.c | 52 ++++++-- >> hw/9pfs/virtio-9p-xattr-user.c | 27 +++- >> hw/9pfs/virtio-9p-xattr.c | 9 +- >> hw/9pfs/virtio-9p-xattr.h | 27 +++- >> hw/9pfs/virtio-9p.h | 6 +- >> 8 files changed, 292 insertions(+), 138 deletions(-) >> >> diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c >> index 3891050..739bad0 100644 >> --- a/hw/9pfs/cofs.c >> +++ b/hw/9pfs/cofs.c >> @@ -20,18 +20,24 @@ >> int v9fs_co_readlink(V9fsPDU *pdu, V9fsPath *path, V9fsString *buf) >> { >> int err; >> - ssize_t len; >> + ssize_t len, maxlen = PATH_MAX; >> V9fsState *s = pdu->s; >> >> if (v9fs_request_cancelled(pdu)) { >> return -EINTR; >> } >> - buf->data = g_malloc(PATH_MAX); >> + buf->data = g_malloc(maxlen); >> v9fs_path_read_lock(s); >> v9fs_co_run_in_worker( >> - { >> + while (1) { >> len = s->ops->readlink(&s->ctx, path, >> - buf->data, PATH_MAX - 1); >> + buf->data, maxlen - 1); >> + if (len == maxlen - 1) { >> + g_free(buf->data); >> + maxlen *= 2; >> + buf->data = g_malloc(maxlen); >> + continue; >> + } >> if (len > -1) { >> buf->size = len; >> buf->data[len] = 0; > err = 0; >> } else { >> err = -errno; >> } >> + break; >> }); >> v9fs_path_unlock(s); >> if (err) { > > Harmless off-by-one: you double the buffer even when the link contents > plus terminating null fits the buffer exactly (len == maxlen - 1). > > I prefer to have the exceptional stuff handled in conditionals, and not > the normal stuff, like this: > > for (;;) { > len = s->ops->readlink(&s->ctx, path, buf->data, maxlen); > if (len < 0) { > err = -errno; > break; > } > if (len == maxlen) { > g_free(buf->data); > maxlen *= 2; > buf->data = g_malloc(maxlen); > continue; > } > buf->size = len; > buf->data[len] = 0; > err = 0; > break; > } > > Matter of taste. > That sounds good to me, after this patch pass checking, I will/should send patch v2 for it. > [...] > > I skimmed a few more hunks, and they look good to me. Leaving full > review to Aneesh. > Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed