From patchwork Mon May 10 12:53:38 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anthony PERARD X-Patchwork-Id: 12247965 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CC84FC433ED for ; Mon, 10 May 2021 12:57:30 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 69FBE611BD for ; Mon, 10 May 2021 12:57:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 69FBE611BD Authentication-Results: mail.kernel.org; dmarc=pass (p=none dis=none) header.from=nongnu.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:40656 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lg5TV-00028n-E4 for qemu-devel@archiver.kernel.org; Mon, 10 May 2021 08:57:29 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:43210) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lg5Q2-00058h-El for qemu-devel@nongnu.org; Mon, 10 May 2021 08:53:54 -0400 Received: from esa2.hc3370-68.iphmx.com ([216.71.145.153]:29307) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lg5Q0-0002LF-Ik for qemu-devel@nongnu.org; Mon, 10 May 2021 08:53:54 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1620651232; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=qxnni0A9eL6zKhHtldQizgIwzPgiDEvjSI22O1dRui0=; b=YqUGlJXCjXoh0tFVy1aUSqTzWypJSJC7F/WZFFE3fXy7Wd0EMwJG1pdv kGd3RR6LrbJkyLlq2YLfjY/X4ctRoZLsPXuhDes/sFnbYsq0+zuRYpkHp 6O5izd5+mZfrLPciWjxOy6Ne9yYVN6d52dyNxHD9X79txM+hRDi4sURcO 0=; Authentication-Results: esa2.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none IronPort-SDR: PkKXSDtAjaDFOOQRbZn0k9IaLjO3BMQWCnzZ85V1fHRFwePEEdABWzUO+LbkP83ZVQg6tJ8lJs +Q7DZ5yE1JttZ72LgzK2SehRl30S+hCwMW5tWLM/Fklrr989sIkeb3MYw8K7jv+HgzcvDdMTdV fTg7YbsCP42ww1HzOxFW4XZlDRl3YKI0uXLvU+0dGSFzD19slg7IC4Z1FY3sC7WESG3M6xd+Jq NVUE0eAhZ6GT1UNo2A/R/tfQNrLV/dxvgdskWzAne2r3kqUFXWo42h9wHlTvE6xBQHGC83b0+l gN8= X-SBRS: 5.1 X-MesageID: 43429923 X-Ironport-Server: esa2.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED IronPort-HdrOrdr: A9a23:vHHm6aCSYganiFLlHemq55DYdb4zR+YMi2TC1yhKJiC9Ffbo8P xG/c5rrCMc5wxxZJhNo7290ey7MBHhHP1OkO0s1NWZPDUO0VHAROoJ0WKh+UyEJ8SXzJ866U 4KScZD4bPLYWSS9fyKgzWFLw== X-IronPort-AV: E=Sophos;i="5.82,287,1613451600"; d="scan'208";a="43429923" To: CC: Peter Maydell , Igor Druzhinin , Anthony PERARD Subject: [PULL 1/3] xen-mapcache: avoid a race on memory map while using MAP_FIXED Date: Mon, 10 May 2021 13:53:38 +0100 Message-ID: <20210510125340.903323-2-anthony.perard@citrix.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210510125340.903323-1-anthony.perard@citrix.com> References: <20210510125340.903323-1-anthony.perard@citrix.com> MIME-Version: 1.0 Received-SPF: pass client-ip=216.71.145.153; envelope-from=anthony.perard@citrix.com; helo=esa2.hc3370-68.iphmx.com X-Spam_score_int: -50 X-Spam_score: -5.1 X-Spam_bar: ----- X-Spam_report: (-5.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.698, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Reply-to: Anthony PERARD X-Patchwork-Original-From: Anthony PERARD via From: Anthony PERARD From: Igor Druzhinin When we're replacing the existing mapping there is possibility of a race on memory map with other threads doing mmap operations - the address being unmapped/re-mapped could be occupied by another thread in between. Linux mmap man page recommends keeping the existing mappings in place to reserve the place and instead utilize the fact that the next mmap operation with MAP_FIXED flag passed will implicitly destroy the existing mappings behind the chosen address. This behavior is guaranteed by POSIX / BSD and therefore is portable. Note that it wouldn't make the replacement atomic for parallel accesses to the replaced region - those might still fail with SIGBUS due to xenforeignmemory_map not being atomic. So we're still not expecting those. Tested-by: Anthony PERARD Signed-off-by: Igor Druzhinin Reviewed-by: Paul Durrant Message-Id: <1618889702-13104-1-git-send-email-igor.druzhinin@citrix.com> Signed-off-by: Anthony PERARD --- hw/i386/xen/xen-mapcache.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c index 5b120ed44b..e82b7dcdd2 100644 --- a/hw/i386/xen/xen-mapcache.c +++ b/hw/i386/xen/xen-mapcache.c @@ -171,7 +171,20 @@ static void xen_remap_bucket(MapCacheEntry *entry, if (!(entry->flags & XEN_MAPCACHE_ENTRY_DUMMY)) { ram_block_notify_remove(entry->vaddr_base, entry->size); } - if (munmap(entry->vaddr_base, entry->size) != 0) { + + /* + * If an entry is being replaced by another mapping and we're using + * MAP_FIXED flag for it - there is possibility of a race for vaddr + * address with another thread doing an mmap call itself + * (see man 2 mmap). To avoid that we skip explicit unmapping here + * and allow the kernel to destroy the previous mappings by replacing + * them in mmap call later. + * + * Non-identical replacements are not allowed therefore. + */ + assert(!vaddr || (entry->vaddr_base == vaddr && entry->size == size)); + + if (!vaddr && munmap(entry->vaddr_base, entry->size) != 0) { perror("unmap fails"); exit(-1); }