From patchwork Tue Mar 15 17:35:05 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 8590831 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id A3AEE9F758 for ; Tue, 15 Mar 2016 17:35:39 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C12732021B for ; Tue, 15 Mar 2016 17:35:38 +0000 (UTC) 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.kernel.org (Postfix) with ESMTPS id CA66320304 for ; Tue, 15 Mar 2016 17:35:37 +0000 (UTC) Received: from localhost ([::1]:50274 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afssb-0006Lr-4n for patchwork-qemu-devel@patchwork.kernel.org; Tue, 15 Mar 2016 13:35:37 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57735) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afssJ-0006Gl-Qm for qemu-devel@nongnu.org; Tue, 15 Mar 2016 13:35:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1afssI-0003mT-Mp for qemu-devel@nongnu.org; Tue, 15 Mar 2016 13:35:19 -0400 Received: from mail-wm0-x22b.google.com ([2a00:1450:400c:c09::22b]:33771) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afssI-0003mB-CD for qemu-devel@nongnu.org; Tue, 15 Mar 2016 13:35:18 -0400 Received: by mail-wm0-x22b.google.com with SMTP id l68so155482598wml.0 for ; Tue, 15 Mar 2016 10:35:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=x0edWqC1pIDpCnI2mx9xebX4JWYjJ1dVoor2BvG9ReU=; b=mdy3ik8qYi8b1CdeLbsUY+5rbMn9xzXZ9IaitzkUVxnq+15P2AloSA4uslOy3hR8uS /R6/yHJ+Vd/+wZhXXs7JeSnhaOAlrlQtyXbiEjwr12h8tuGqh982qukSrNLMcpGiOQNH 48ABFHVLVl894r8CZSTp0ltFyfVz5N34OGB2CMMSj2km15RtR1Igb39RqLsb54lyHnGg rwK0CKPiW7oCFr3ME9J+ey6w1KLEHgubeneVSOExdZMA8hsyoVwOlZZ85YYkDGiCAPaE b8pBsspCtCxYUiXzmnu2V/5diSP5yY4O/bfoMQF7nf9/wrBH0ENP3VihYBkRKAgzd76W y/+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=x0edWqC1pIDpCnI2mx9xebX4JWYjJ1dVoor2BvG9ReU=; b=BOiEhM6sCjs0GGQlx7oZ4cVA+ykVxUCe2CAaLBRx0s2jc7rTSjZln9A4KCZH/N6XAE RTptRGctLyAuZEGzsTU+0LLILrmz6ZVGVQzUy1LfseXFu2gujS6JhZy1O7dkGKFFcMZi 1mg5xnuSbLcfvGekh7M3Wy5xbQ3cAPbqBKT1nlJnGzX/1tGAar7VNXX78/iV3qPl+dl+ uP+coNqAp1g73BSclb9UuHhV/uloF8nKp1Aeytg8vsjD1seLU/sD6QDWf4mssjkcT6Np cejktQ3Khvav/Oj+Ol1kWOk2caOtHtaTk4dNswCDXQ40Dgg/+YDUcV68AmmdLsZuincE 0ktw== X-Gm-Message-State: AD7BkJL5fMLTXwce3QsPrw2I9zUYhWTco+8UFwB980P+0HmvvKRIIKGkB3obuzN1Zx81pg== X-Received: by 10.194.81.103 with SMTP id z7mr30905181wjx.25.1458063317709; Tue, 15 Mar 2016 10:35:17 -0700 (PDT) Received: from 640k.lan (94-39-161-17.adsl-ull.clienti.tiscali.it. [94.39.161.17]) by smtp.gmail.com with ESMTPSA id z127sm21827680wme.5.2016.03.15.10.35.16 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 15 Mar 2016 10:35:16 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Tue, 15 Mar 2016 18:35:05 +0100 Message-Id: <1458063310-128525-4-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1458063310-128525-1-git-send-email-pbonzini@redhat.com> References: <1458063310-128525-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:400c:c09::22b Cc: Markus Armbruster Subject: [Qemu-devel] [PULL 3/8] exec: Fix memory allocation when memory path names new file X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Markus Armbruster Commit 8d31d6b extended file_ram_alloc() to accept file names in addition to directory names. Even though it passes O_CREAT to open(), it actually works only for existing files. Reproducer adapted from the commit's qemu-doc.texi update: $ qemu-system-x86_64 -object memory-backend-file,size=2M,mem-path=/dev/hugepages/my-shmem-file,id=mb1 qemu-system-x86_64: -object memory-backend-file,size=2M,mem-path=/dev/hugepages/my-shmem-file,id=mb1: failed to get page size of file /dev/hugepages/my-shmem-file: No such file or directory This is because we first get the page size for @path, then open the actual file. Unwise even before the flawed commit, because the directory could change in between, invalidating the page size. Unlikely to bite in practice. Rearrange the code to create the file (if necessary) before getting its page size. Carefully avoid TOCTTOU conditions with a method suggested by Paolo Bonzini. While there, replace "hugepages" by "guest RAM" in error messages, because host memory backends can be used for purposes other than huge pages, e.g. /dev/shm/ shared memory. Help text of -mem-path agrees. Cc: Paolo Bonzini Signed-off-by: Markus Armbruster Message-Id: <1457378754-21649-2-git-send-email-armbru@redhat.com> Signed-off-by: Paolo Bonzini --- exec.c | 107 +++++++++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 64 insertions(+), 43 deletions(-) diff --git a/exec.c b/exec.c index f09dd4e..3380836 100644 --- a/exec.c +++ b/exec.c @@ -1233,19 +1233,17 @@ void qemu_mutex_unlock_ramlist(void) #define HUGETLBFS_MAGIC 0x958458f6 -static long gethugepagesize(const char *path, Error **errp) +static long gethugepagesize(int fd) { struct statfs fs; int ret; do { - ret = statfs(path, &fs); + ret = fstatfs(fd, &fs); } while (ret != 0 && errno == EINTR); if (ret != 0) { - error_setg_errno(errp, errno, "failed to get page size of file %s", - path); - return 0; + return -1; } return fs.f_bsize; @@ -1256,60 +1254,79 @@ static void *file_ram_alloc(RAMBlock *block, const char *path, Error **errp) { - struct stat st; + bool unlink_on_error = false; char *filename; char *sanitized_name; char *c; void *area; int fd; - uint64_t hpagesize; - Error *local_err = NULL; - - hpagesize = gethugepagesize(path, &local_err); - if (local_err) { - error_propagate(errp, local_err); - goto error; - } - block->mr->align = hpagesize; - - if (memory < hpagesize) { - error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " - "or larger than huge page size 0x%" PRIx64, - memory, hpagesize); - goto error; - } + int64_t hpagesize; if (kvm_enabled() && !kvm_has_sync_mmu()) { error_setg(errp, "host lacks kvm mmu notifiers, -mem-path unsupported"); - goto error; + return NULL; } - if (!stat(path, &st) && S_ISDIR(st.st_mode)) { - /* Make name safe to use with mkstemp by replacing '/' with '_'. */ - sanitized_name = g_strdup(memory_region_name(block->mr)); - for (c = sanitized_name; *c != '\0'; c++) { - if (*c == '/') { - *c = '_'; - } + for (;;) { + fd = open(path, O_RDWR); + if (fd >= 0) { + /* @path names an existing file, use it */ + break; } + if (errno == ENOENT) { + /* @path names a file that doesn't exist, create it */ + fd = open(path, O_RDWR | O_CREAT | O_EXCL, 0644); + if (fd >= 0) { + unlink_on_error = true; + break; + } + } else if (errno == EISDIR) { + /* @path names a directory, create a file there */ + /* Make name safe to use with mkstemp by replacing '/' with '_'. */ + sanitized_name = g_strdup(memory_region_name(block->mr)); + for (c = sanitized_name; *c != '\0'; c++) { + if (*c == '/') { + *c = '_'; + } + } - filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path, - sanitized_name); - g_free(sanitized_name); + filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path, + sanitized_name); + g_free(sanitized_name); - fd = mkstemp(filename); - if (fd >= 0) { - unlink(filename); + fd = mkstemp(filename); + if (fd >= 0) { + unlink(filename); + g_free(filename); + break; + } + g_free(filename); } - g_free(filename); - } else { - fd = open(path, O_RDWR | O_CREAT, 0644); + if (errno != EEXIST && errno != EINTR) { + error_setg_errno(errp, errno, + "can't open backing store %s for guest RAM", + path); + goto error; + } + /* + * Try again on EINTR and EEXIST. The latter happens when + * something else creates the file between our two open(). + */ } - if (fd < 0) { - error_setg_errno(errp, errno, - "unable to create backing store for hugepages"); + hpagesize = gethugepagesize(fd); + if (hpagesize < 0) { + error_setg_errno(errp, errno, "can't get page size for %s", + path); + goto error; + } + block->mr->align = hpagesize; + + if (memory < hpagesize) { + error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " + "or larger than page size 0x%" PRIx64, + memory, hpagesize); goto error; } @@ -1328,7 +1345,7 @@ static void *file_ram_alloc(RAMBlock *block, area = qemu_ram_mmap(fd, memory, hpagesize, block->flags & RAM_SHARED); if (area == MAP_FAILED) { error_setg_errno(errp, errno, - "unable to map backing store for hugepages"); + "unable to map backing store for guest RAM"); close(fd); goto error; } @@ -1341,6 +1358,10 @@ static void *file_ram_alloc(RAMBlock *block, return area; error: + if (unlink_on_error) { + unlink(path); + } + close(fd); return NULL; } #endif