From patchwork Thu Oct 13 10:09:43 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Li Qiang X-Patchwork-Id: 681728 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3svmjf6bbvz9sD6 for ; Thu, 13 Oct 2016 21:11:50 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=MgRICQL+; dkim-atps=neutral Received: from localhost ([::1]:39086 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buczL-00042s-H6 for incoming@patchwork.ozlabs.org; Thu, 13 Oct 2016 06:11:47 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48259) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bucxu-0002rM-11 for qemu-devel@nongnu.org; Thu, 13 Oct 2016 06:10:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bucxr-0005u1-Js for qemu-devel@nongnu.org; Thu, 13 Oct 2016 06:10:16 -0400 Received: from mail-oi0-x244.google.com ([2607:f8b0:4003:c06::244]:35926) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bucxr-0005to-Ed for qemu-devel@nongnu.org; Thu, 13 Oct 2016 06:10:15 -0400 Received: by mail-oi0-x244.google.com with SMTP id e12so5206712oib.3 for ; Thu, 13 Oct 2016 03:10:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:from:to:cc:subject:date:in-reply-to:references; bh=sMVFlVpHjY37jwv5O8u0zkQlXgIGVQIp7aIV51ogNZc=; b=MgRICQL+QDY7bmwQoqXuI6towhMxjngX5Dqji636gqWbUNdMBwPHGDCf09un8uVEpn liQ+3huZ27hDi3MlZ3YLR9lLSkQBiRk7BR/BUEQFpgdKPogSMadn9eQD5PAMSPx1N/Wl eaOmeGxc3+dwc8qFDL+lOlYwoxiaE6on4xn5cjvggLZvV6Q3qhj2dM2HmfrR5vqBy9pW 8pclv1bcCbY2c7avTXPYL7nFYlTLERPV7YT7KsRTLUM1MYcl83e5+ezakvotUxuWjh0s aDHjAz//AbCJVg589TOPDATAbey5heYclpr/ebR6tfGYu7OkacjUW6WBze6xuiz5bYf4 l4fw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:from:to:cc:subject:date:in-reply-to :references; bh=sMVFlVpHjY37jwv5O8u0zkQlXgIGVQIp7aIV51ogNZc=; b=jMW0+dJktLiO8P3AudZ2bd1dZCJ5hUggI5db8TLZ0gDYcPbev8f7FtocHAz9qhg2z6 G2tvCUHMX0IcyNjP/HiU2TZJxvuV61n3RoFgQiMNxtzedsG8wXAsegnlxpqkBaT6PLoa RIvRb7GCUQn4OCmp3AyReQFb7JpYOBAslw72za5izekjbymOcKI4fB+G35PqiBj0/9Lq iUBVEwbGJVnSXQcjqralJzYnWFi8wP4kk4Lil3ioRhKVUNZ9ADynmr8iluoZ9vGm9XUr XnsljYhy9j78d6qDB6zpWyjk/fIa2oGnlY8HvH9lNC7Y4pS+ZafPtMwNJ5Q8oMfz7V3j W6Cg== X-Gm-Message-State: AA6/9Rm41sTJbXTd9+mlWFUSe6g1a02gDI4BNYkVlwx5wlSyWIiaFgdqNGgBS26wI6hoaA== X-Received: by 10.157.38.217 with SMTP id i25mr2984644otd.79.1476353415142; Thu, 13 Oct 2016 03:10:15 -0700 (PDT) Received: from localhost.localdomain.localdomain ([104.192.110.250]) by smtp.gmail.com with ESMTPSA id j10sm4245734ote.9.2016.10.13.03.10.13 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 13 Oct 2016 03:10:14 -0700 (PDT) Message-ID: <57ff5d86.4a3c9d0a.90936.609f@mx.google.com> X-Google-Original-Message-ID: <1476353383-4679-4-git-send-email-Qiang(liqiang6-s@360.cn)> From: Li Qiang X-Google-Original-From: Li Qiang(liqiang6-s@360.cn) To: groug@kaod.org, qemu-devel@nongnu.org Date: Thu, 13 Oct 2016 03:09:43 -0700 X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1476353383-4679-1-git-send-email-Qiang(liqiang6-s@360.cn)> References: <1476353383-4679-1-git-send-email-Qiang(liqiang6-s@360.cn)> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2607:f8b0:4003:c06::244 Subject: [Qemu-devel] [PATCH v3 3/3] 9pfs: fix integer overflow issue in xattr read/write 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: Li Qiang Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" From: Li Qiang The v9fs_xattr_read() and v9fs_xattr_write() are passed a guest originated offset: they must ensure this offset does not go beyond the size of the extended attribute that was set in v9fs_xattrcreate(). Unfortunately, the current code implement these checks with unsafe calculations on 32 and 64 bit values, which may allow a malicious guest to cause OOB access anyway. Fix this by comparing the offset and the xattr size, which are both uint64_t, before trying to compute the effective number of bytes to read or write. Suggested-by: Greg Kurz Signed-off-by: Li Qiang Reviewed-by: Greg Kurz --- Changes since v2: -make the solution of 'copied_len/len' in V9fsXattr type issue to a separate patch. -add detailed changelog. Changes since v1: -delete 'xattr_len'. hw/9pfs/9p.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index e902eed..6df85b8 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -1642,20 +1642,17 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp, { ssize_t err; size_t offset = 7; - int read_count; - int64_t xattr_len; + uint64_t read_count; V9fsVirtioState *v = container_of(s, V9fsVirtioState, state); VirtQueueElement *elem = v->elems[pdu->idx]; - xattr_len = fidp->fs.xattr.len; - read_count = xattr_len - off; + if (fidp->fs.xattr.len < off) { + read_count = 0; + } else { + read_count = fidp->fs.xattr.len - off; + } if (read_count > max_count) { read_count = max_count; - } else if (read_count < 0) { - /* - * read beyond XATTR value - */ - read_count = 0; } err = pdu_marshal(pdu, offset, "d", read_count); if (err < 0) { @@ -1982,23 +1979,18 @@ static int v9fs_xattr_write(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp, { int i, to_copy; ssize_t err = 0; - int write_count; - int64_t xattr_len; + uint64_t write_count; size_t offset = 7; - xattr_len = fidp->fs.xattr.len; - write_count = xattr_len - off; - if (write_count > count) { - write_count = count; - } else if (write_count < 0) { - /* - * write beyond XATTR value len specified in - * xattrcreate - */ + if (fidp->fs.xattr.len < off) { err = -ENOSPC; goto out; } + write_count = fidp->fs.xattr.len - off; + if (write_count > count) { + write_count = count; + } err = pdu_marshal(pdu, offset, "d", write_count); if (err < 0) { return err;