From patchwork Tue Aug 30 15:01:05 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg Kurz X-Patchwork-Id: 9305507 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 3E31F607D2 for ; Tue, 30 Aug 2016 15:03:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2ECD828C00 for ; Tue, 30 Aug 2016 15:03:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2392228C14; Tue, 30 Aug 2016 15:03:27 +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 lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 31C4228C00 for ; Tue, 30 Aug 2016 15:03:23 +0000 (UTC) Received: from localhost ([::1]:49558 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bekZO-0004mt-6E for patchwork-qemu-devel@patchwork.kernel.org; Tue, 30 Aug 2016 11:03:22 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53358) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bekXS-0003qS-KM for qemu-devel@nongnu.org; Tue, 30 Aug 2016 11:01:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bekXN-0000Pp-VS for qemu-devel@nongnu.org; Tue, 30 Aug 2016 11:01:22 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:49235) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bekXN-0000Pk-NK for qemu-devel@nongnu.org; Tue, 30 Aug 2016 11:01:17 -0400 Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.11/8.16.0.11) with SMTP id u7UExQ4J043591 for ; Tue, 30 Aug 2016 11:01:17 -0400 Received: from e38.co.us.ibm.com (e38.co.us.ibm.com [32.97.110.159]) by mx0a-001b2d01.pphosted.com with ESMTP id 255364hvfg-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 30 Aug 2016 11:01:16 -0400 Received: from localhost by e38.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 30 Aug 2016 09:01:14 -0600 Received: from d03dlp03.boulder.ibm.com (9.17.202.179) by e38.co.us.ibm.com (192.168.1.138) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 30 Aug 2016 09:01:11 -0600 X-IBM-Helo: d03dlp03.boulder.ibm.com X-IBM-MailFrom: groug@kaod.org Received: from b01cxnp22036.gho.pok.ibm.com (b01cxnp22036.gho.pok.ibm.com [9.57.198.26]) by d03dlp03.boulder.ibm.com (Postfix) with ESMTP id 6F0DC19D80C7; Tue, 30 Aug 2016 09:00:39 -0600 (MDT) Received: from b01ledav005.gho.pok.ibm.com (b01ledav005.gho.pok.ibm.com [9.57.199.110]) by b01cxnp22036.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u7UF174g63963282; Tue, 30 Aug 2016 15:01:07 GMT Received: from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 652ADAE05C; Tue, 30 Aug 2016 11:01:07 -0400 (EDT) Received: from bahia.lab.toulouse-stg.fr.ibm.com (unknown [9.101.4.41]) by b01ledav005.gho.pok.ibm.com (Postfix) with ESMTP id 76DA9AE062; Tue, 30 Aug 2016 11:01:06 -0400 (EDT) From: Greg Kurz To: qemu-devel@nongnu.org Date: Tue, 30 Aug 2016 17:01:05 +0200 In-Reply-To: <147256922286.23141.1863740247797922944.stgit@bahia.lab.toulouse-stg.fr.ibm.com> References: <147256922286.23141.1863740247797922944.stgit@bahia.lab.toulouse-stg.fr.ibm.com> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16083015-0028-0000-0000-0000057A324E X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00005676; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000184; SDB=6.00751654; UDB=6.00355285; IPR=6.00524459; BA=6.00004680; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00012518; XFM=3.00000011; UTC=2016-08-30 15:01:14 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16083015-0029-0000-0000-00002ECBEC0C Message-Id: <147256922950.23141.8320400394951173272.stgit@bahia.lab.toulouse-stg.fr.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-08-30_07:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=1 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1608300145 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] X-Received-From: 148.163.156.1 Subject: [Qemu-devel] [PATCH v3 1/3] 9pfs: forbid illegal path names X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Maydell , Felix Wilhelm , "Michael S. Tsirkin" , Greg Kurz , P J P , "Aneesh Kumar K.V" Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Empty path components don't make sense for most commands and may cause undefined behavior, depending on the backend. A notable exception would be the target of a symbolic link: POSIX requires implementations to support the empty string for target names. But 9p is only supported on linux hosts, which don't support empty target names yet, and the proposal to error out with ENOENT got approved, as indicated here: http://austingroupbugs.net/view.php?id=649 For the sake of consistency and simplicity, symlink target names will be treated the same as any other file name argument. Also, the walk request described in the 9P spec [1] clearly shows that the client is supposed to send individual path components: the official linux client never sends portions of path containing the / character for example. Moreover, the 9P spec [2] also states that a system can decide to restrict the set of supported characters used in path components, with an explicit mention "to remove slashes from name components". This patch introduces a new name_is_illegal() helper that checks the names sent by the client are not empty and don't contain unwanted chars. Since 9pfs is only supported on linux hosts, only the / character is checked at the moment. When support for other hosts (AKA. win32) is added, other chars may need to be blacklisted as well. If a client sends an illegal path component, the request will fail and ENOENT is returned to the client. [1] http://man.cat-v.org/plan_9/5/walk [2] http://man.cat-v.org/plan_9/5/intro Suggested-by: Peter Maydell Signed-off-by: Greg Kurz --- v3: - fix empty string check in name_is_illegal() - return ENOENT instead of EINVAL - added checking of target names for symlinks in create (9P2000.u) - added some details about symlink target names in the changelog --- hw/9pfs/9p.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index b6b02b46a9da..fe2d27f12ae1 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -1256,6 +1256,11 @@ static int v9fs_walk_marshal(V9fsPDU *pdu, uint16_t nwnames, V9fsQID *qids) return offset; } +static bool name_is_illegal(const char *name) +{ + return !*name || strchr(name, '/') != NULL; +} + static void v9fs_walk(void *opaque) { int name_idx; @@ -1289,6 +1294,10 @@ static void v9fs_walk(void *opaque) if (err < 0) { goto out_nofid; } + if (name_is_illegal(wnames[i].data)) { + err = -ENOENT; + goto out_nofid; + } offset += err; } } else if (nwnames > P9_MAXWELEM) { @@ -1483,6 +1492,11 @@ static void v9fs_lcreate(void *opaque) } trace_v9fs_lcreate(pdu->tag, pdu->id, dfid, flags, mode, gid); + if (name_is_illegal(name.data)) { + err = -ENOENT; + goto out_nofid; + } + fidp = get_fid(pdu, dfid); if (fidp == NULL) { err = -ENOENT; @@ -2077,6 +2091,11 @@ static void v9fs_create(void *opaque) } trace_v9fs_create(pdu->tag, pdu->id, fid, name.data, perm, mode); + if (name_is_illegal(name.data)) { + err = -ENOENT; + goto out_nofid; + } + fidp = get_fid(pdu, fid); if (fidp == NULL) { err = -EINVAL; @@ -2099,6 +2118,11 @@ static void v9fs_create(void *opaque) } fidp->fid_type = P9_FID_DIR; } else if (perm & P9_STAT_MODE_SYMLINK) { + if (name_is_illegal(extension.data)) { + err = -ENOENT; + goto out_nofid; + } + err = v9fs_co_symlink(pdu, fidp, &name, extension.data, -1 , &stbuf); if (err < 0) { @@ -2242,6 +2266,11 @@ static void v9fs_symlink(void *opaque) } trace_v9fs_symlink(pdu->tag, pdu->id, dfid, name.data, symname.data, gid); + if (name_is_illegal(name.data) || name_is_illegal(symname.data)) { + err = -ENOENT; + goto out_nofid; + } + dfidp = get_fid(pdu, dfid); if (dfidp == NULL) { err = -EINVAL; @@ -2316,6 +2345,11 @@ static void v9fs_link(void *opaque) } trace_v9fs_link(pdu->tag, pdu->id, dfid, oldfid, name.data); + if (name_is_illegal(name.data)) { + err = -ENOENT; + goto out_nofid; + } + dfidp = get_fid(pdu, dfid); if (dfidp == NULL) { err = -ENOENT; @@ -2398,6 +2432,12 @@ static void v9fs_unlinkat(void *opaque) if (err < 0) { goto out_nofid; } + + if (name_is_illegal(name.data)) { + err = -ENOENT; + goto out_nofid; + } + dfidp = get_fid(pdu, dfid); if (dfidp == NULL) { err = -EINVAL; @@ -2504,6 +2544,12 @@ static void v9fs_rename(void *opaque) if (err < 0) { goto out_nofid; } + + if (name_is_illegal(name.data)) { + err = -ENOENT; + goto out_nofid; + } + fidp = get_fid(pdu, fid); if (fidp == NULL) { err = -ENOENT; @@ -2616,6 +2662,11 @@ static void v9fs_renameat(void *opaque) goto out_err; } + if (name_is_illegal(old_name.data) || name_is_illegal(new_name.data)) { + err = -ENOENT; + goto out_err; + } + v9fs_path_write_lock(s); err = v9fs_complete_renameat(pdu, olddirfid, &old_name, newdirfid, &new_name); @@ -2826,6 +2877,11 @@ static void v9fs_mknod(void *opaque) } trace_v9fs_mknod(pdu->tag, pdu->id, fid, mode, major, minor); + if (name_is_illegal(name.data)) { + err = -ENOENT; + goto out_nofid; + } + fidp = get_fid(pdu, fid); if (fidp == NULL) { err = -ENOENT; @@ -2977,6 +3033,11 @@ static void v9fs_mkdir(void *opaque) } trace_v9fs_mkdir(pdu->tag, pdu->id, fid, name.data, mode, gid); + if (name_is_illegal(name.data)) { + err = -ENOENT; + goto out_nofid; + } + fidp = get_fid(pdu, fid); if (fidp == NULL) { err = -ENOENT;