From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44728) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buazO-0008DE-AH for qemu-devel@nongnu.org; Thu, 13 Oct 2016 04:03:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1buazL-0002x7-4T for qemu-devel@nongnu.org; Thu, 13 Oct 2016 04:03:42 -0400 Received: from 5.mo179.mail-out.ovh.net ([46.105.43.140]:58482) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buazK-0002w9-Qz for qemu-devel@nongnu.org; Thu, 13 Oct 2016 04:03:39 -0400 Received: from player792.ha.ovh.net (b7.ovh.net [213.186.33.57]) by mo179.mail-out.ovh.net (Postfix) with ESMTP id 01203FFBD4C for ; Thu, 13 Oct 2016 10:03:37 +0200 (CEST) Date: Thu, 13 Oct 2016 10:03:34 +0200 From: Greg Kurz Message-ID: <20161013100334.6a0ede60@bahia> In-Reply-To: <57fef7be.857bc20a.4ca2.95a9@mx.google.com> References: <57fef7be.857bc20a.4ca2.95a9@mx.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] 9pfs: add xattrwalk_fid field in V9fsFidState struct List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Li Qiang Cc: qemu-devel@nongnu.org, Li Qiang On Wed, 12 Oct 2016 19:55:42 -0700 Li Qiang wrote: > From: Li Qiang > > Currently, 9pfs sets the fs.xattr.copied_len field in V9fsFidState > to -1 to indicate a xattr walk fid. As the fs.xattr.copied_len is also > used to account for copied bytes, this may cause confusion. This patch > add a bool variable to represent the xattr walk fid. > > Signed-off-by: Li Qiang > --- Please send a patchset instead of individual patches... it makes reviewing difficult. For example, this patch and the "9pfs: fix integer overflow issue in xattr read/write" one are about fixing how xattr.copied_len and xattr.len are being used. They definitely should be sent together. I suggest you send a new patchset with a cover letter and 3 patches: - the cover letter and the patches should be tagged v3, since this will be your third post on the same topic - the cover letter should describe the overall problem: wrong types, unsafe computations, copied_len used both to account bytes and to tag the xattr fid. - patch 1/3: this patch, with changes since the previous version below the --- - patch 2/3: conversion of copied_len/len to uint64_t - patch 3/3: "9pfs: fix integer overflow...", with changes since the previous version below the --- > hw/9pfs/9p.c | 7 ++++--- > hw/9pfs/9p.h | 1 + > 2 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > index 8c7488f..9625296 100644 > --- a/hw/9pfs/9p.c > +++ b/hw/9pfs/9p.c > @@ -325,7 +325,7 @@ static int v9fs_xattr_fid_clunk(V9fsPDU *pdu, V9fsFidState *fidp) > { > int retval = 0; > > - if (fidp->fs.xattr.copied_len == -1) { > + if (fidp->xattrwalk_fid) { > /* getxattr/listxattr fid */ > goto free_value; > } > @@ -3181,7 +3181,7 @@ static void v9fs_xattrwalk(void *opaque) > */ > xattr_fidp->fs.xattr.len = size; > xattr_fidp->fid_type = P9_FID_XATTR; > - xattr_fidp->fs.xattr.copied_len = -1; > + xattr_fidp->xattrwalk_fid = true; > if (size) { > xattr_fidp->fs.xattr.value = g_malloc(size); > err = v9fs_co_llistxattr(pdu, &xattr_fidp->path, > @@ -3214,7 +3214,7 @@ static void v9fs_xattrwalk(void *opaque) > */ > xattr_fidp->fs.xattr.len = size; > xattr_fidp->fid_type = P9_FID_XATTR; > - xattr_fidp->fs.xattr.copied_len = -1; > + xattr_fidp->xattrwalk_fid = true; > if (size) { > xattr_fidp->fs.xattr.value = g_malloc(size); > err = v9fs_co_lgetxattr(pdu, &xattr_fidp->path, > @@ -3269,6 +3269,7 @@ static void v9fs_xattrcreate(void *opaque) > /* Make the file fid point to xattr */ > xattr_fidp = file_fidp; > xattr_fidp->fid_type = P9_FID_XATTR; > + xattr_fidp->xattrwalk_fid = false; > xattr_fidp->fs.xattr.copied_len = 0; > xattr_fidp->fs.xattr.len = size; > xattr_fidp->fs.xattr.flags = flags; > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h > index 22198f6..7e1e70b 100644 > --- a/hw/9pfs/9p.h > +++ b/hw/9pfs/9p.h > @@ -214,6 +214,7 @@ struct V9fsFidState > uid_t uid; > int ref; > int clunked; > + bool xattrwalk_fid; This belongs to the V9fsXattr type. > V9fsFidState *next; > V9fsFidState *rclm_lst; > }; Cheers. -- Greg