From patchwork Thu Nov 10 15:21:09 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "G. Campana" X-Patchwork-Id: 9421237 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 7288E60720 for ; Thu, 10 Nov 2016 15:22:18 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6964529782 for ; Thu, 10 Nov 2016 15:22:18 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5E3FC29783; Thu, 10 Nov 2016 15:22:18 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DF5D829784 for ; Thu, 10 Nov 2016 15:22:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933955AbcKJPWP (ORCPT ); Thu, 10 Nov 2016 10:22:15 -0500 Received: from mail.quarkslab.com ([195.154.215.101]:51354 "EHLO mail.quarkslab.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933842AbcKJPWN (ORCPT ); Thu, 10 Nov 2016 10:22:13 -0500 Received: by mail.quarkslab.com (Postfix, from userid 108) id F0A7E901062; Thu, 10 Nov 2016 17:07:05 +0100 (CET) Received: from kwak.home (LFbn-1-2739-239.w86-247.abo.wanadoo.fr [86.247.243.239]) (Authenticated sender: gcampana) by mail.quarkslab.com (Postfix) with ESMTPSA id 4320B901061; Thu, 10 Nov 2016 17:07:05 +0100 (CET) From: "G. Campana" To: Will.Deacon@arm.com Cc: kvm@vger.kernel.org, andre.przywara@arm.com, gcampana+kvm@quarkslab.com Subject: [PATCH 3/5] kvmtool: 9p: fix strcpy vulnerabilities Date: Thu, 10 Nov 2016 16:21:09 +0100 Message-Id: <1478791271-7558-4-git-send-email-gcampana+kvm@quarkslab.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1478791271-7558-1-git-send-email-gcampana+kvm@quarkslab.com> References: <1478791271-7558-1-git-send-email-gcampana+kvm@quarkslab.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Use strncpy instead of strcpy to avoid buffer overflow vulnerabilities. Signed-off-by: G. Campana --- virtio/9p.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 56 insertions(+), 17 deletions(-) diff --git a/virtio/9p.c b/virtio/9p.c index 4116252..22aa268 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->abs_path)) { + free(pfid); + return NULL; + } + 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)); + pfid->path = pfid->abs_path + strlen(pfid->abs_path); insert_new_fid(dev, pfid); @@ -386,6 +393,19 @@ err_out: return; } +static int join_path(struct p9_fid *fid, const char *name) +{ + size_t len, size; + + size = sizeof(fid->abs_path) - (fid->path - fid->abs_path); + len = strlen(name); + if (len >= size) + return -1; + + strncpy(fid->path, name, size); + return 0; +} + static void virtio_p9_walk(struct p9_dev *p9dev, struct p9_pdu *pdu, u32 *outlen) { @@ -404,7 +424,11 @@ static void virtio_p9_walk(struct p9_dev *p9dev, if (nwname) { struct p9_fid *fid = get_fid(p9dev, fid_val); - strcpy(new_fid->path, fid->path); + if (join_path(new_fid, fid->path) != 0) { + errno = ENAMETOOLONG; + goto err_out; + } + /* skip the space for count */ pdu->write_offset += sizeof(u16); for (i = 0; i < nwname; i++) { @@ -429,7 +453,10 @@ static void virtio_p9_walk(struct p9_dev *p9dev, goto err_out; stat2qid(&st, &wqid); - strcpy(new_fid->path, tmp); + if (join_path(new_fid, tmp) != 0) { + errno = ENAMETOOLONG; + goto err_out; + } new_fid->uid = fid->uid; nwqid++; virtio_p9_pdu_writef(pdu, "Q", &wqid); @@ -440,7 +467,10 @@ static void virtio_p9_walk(struct p9_dev *p9dev, */ pdu->write_offset += sizeof(u16); old_fid = get_fid(p9dev, fid_val); - strcpy(new_fid->path, old_fid->path); + if (join_path(new_fid, old_fid->path) != 0) { + errno = ENAMETOOLONG; + goto err_out; + } new_fid->uid = old_fid->uid; } *outlen = pdu->write_offset; @@ -476,7 +506,10 @@ static void virtio_p9_attach(struct p9_dev *p9dev, fid = get_fid(p9dev, fid_val); fid->uid = uid; - strcpy(fid->path, "/"); + if (join_path(fid, "/") != 0) { + errno = ENAMETOOLONG; + goto err_out; + } virtio_p9_pdu_writef(pdu, "Q", &qid); *outlen = pdu->write_offset; @@ -1120,20 +1153,24 @@ static int virtio_p9_ancestor(char *path, char *ancestor) return 0; } -static void virtio_p9_fix_path(char *fid_path, char *old_name, char *new_name) +static int virtio_p9_fix_path(struct p9_fid *fid, char *old_name, char *new_name) { - char tmp_name[PATH_MAX]; + int ret; + char *p, tmp_name[PATH_MAX]; size_t rp_sz = strlen(old_name); - if (rp_sz == strlen(fid_path)) { + if (rp_sz == strlen(fid->path)) { /* replace the full name */ - strcpy(fid_path, new_name); - return; + p = new_name; + } else { + /* save the trailing path details */ + ret = snprintf(tmp_name, sizeof(tmp_name), "%s%s", new_name, fid->path + rp_sz); + if (ret >= (int)sizeof(tmp_name)) + return -1; + p = tmp_name; } - /* save the trailing path details */ - strcpy(tmp_name, fid_path + rp_sz); - sprintf(fid_path, "%s%s", new_name, tmp_name); - return; + + return join_path(fid, p); } static void rename_fids(struct p9_dev *p9dev, char *old_name, char *new_name) @@ -1144,7 +1181,7 @@ static void rename_fids(struct p9_dev *p9dev, char *old_name, char *new_name) struct p9_fid *fid = rb_entry(node, struct p9_fid, node); if (fid->fid != P9_NOFID && virtio_p9_ancestor(fid->path, old_name)) { - virtio_p9_fix_path(fid->path, old_name, new_name); + virtio_p9_fix_path(fid, old_name, new_name); } node = rb_next(node); } @@ -1544,7 +1581,9 @@ int virtio_9p__register(struct kvm *kvm, const char *root, const char *tag_name) goto free_p9dev; } - strcpy(p9dev->root_dir, root); + strncpy(p9dev->root_dir, root, sizeof(p9dev->root_dir)); + p9dev->root_dir[sizeof(p9dev->root_dir)-1] = '\x00'; + p9dev->config->tag_len = strlen(tag_name); if (p9dev->config->tag_len > MAX_TAG_LEN) { err = -EINVAL;