From mboxrd@z Thu Jan 1 00:00:00 1970 From: Will Deacon Subject: Re: [PATCH 3/7] kvmtool: fix strcpy vulnerabilities Date: Tue, 8 Nov 2016 02:08:48 +0000 Message-ID: <20161108020847.GV20591@arm.com> References: <1476806572-9782-1-git-send-email-gcampana+kvm@quarkslab.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, andre.przywara@arm.com To: "G. Campana" Return-path: Received: from foss.arm.com ([217.140.101.70]:49582 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752045AbcKHCIq (ORCPT ); Mon, 7 Nov 2016 21:08:46 -0500 Content-Disposition: inline In-Reply-To: <1476806572-9782-1-git-send-email-gcampana+kvm@quarkslab.com> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Oct 18, 2016 at 06:02:52PM +0200, G. Campana wrote: > --- > virtio/9p.c | 60 +++++++++++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 43 insertions(+), 17 deletions(-) > > diff --git a/virtio/9p.c b/virtio/9p.c > index 9695540..cd93d06 100644 > --- a/virtio/9p.c > +++ b/virtio/9p.c > @@ -28,6 +28,7 @@ static struct p9_fid *find_or_create_fid(struct p9_dev *dev, u32 fid) > { > struct rb_node *node = dev->fids.rb_node; > struct p9_fid *pfid = NULL; > + size_t len; > > while (node) { > struct p9_fid *cur = rb_entry(node, struct p9_fid, node); > @@ -45,9 +46,15 @@ static struct p9_fid *find_or_create_fid(struct p9_dev *dev, u32 fid) > if (!pfid) > return NULL; > > + len = strlen(dev->root_dir); > + if (len >= sizeof(pfid->path)) { > + free(pfid); > + return NULL; > + } This doesn't make sense to me -- pfid->path is just a NULL ptr at this stage. Did you mean abs_path? > + > pfid->fid = fid; > - strcpy(pfid->abs_path, dev->root_dir); > - pfid->path = pfid->abs_path + strlen(dev->root_dir); > + strncpy(pfid->abs_path, dev->root_dir, sizeof(pfid->abs_path)); But if you did mean abs_path, why use strncpy here? Will