From patchwork Fri Apr 29 01:52:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Konrad Rzeszutek Wilk X-Patchwork-Id: 8976331 Return-Path: X-Original-To: patchwork-xen-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 8892E9F1D3 for ; Fri, 29 Apr 2016 01:54:51 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 85018201CD for ; Fri, 29 Apr 2016 01:54:50 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7F640201CE for ; Fri, 29 Apr 2016 01:54:49 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1avxbR-000088-Cs; Fri, 29 Apr 2016 01:52:21 +0000 Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1avxbQ-000080-IG for xen-devel@lists.xenproject.org; Fri, 29 Apr 2016 01:52:20 +0000 Received: from [85.158.137.68] by server-1.bemta-3.messagelabs.com id B2/10-07924-35EB2275; Fri, 29 Apr 2016 01:52:19 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprKKsWRWlGSWpSXmKPExsVyMfTOEd2gfUr hBv2LNCy+b5nM5MDocfjDFZYAxijWzLyk/IoE1ox/U+ezF3yWqviw9ClTA+MX0S5GTg4hgRmM Ep8+lXYxcnGwCJxilWhoecAG4kgIPGOV+L9+CwtIlYRAjMSmpXOZIewaiY1905khupUktkx+z AjSICSwhUni+ofL7CAJYQE9icnfbjOC2CwCqhI/Lu5jArHZBPQlnq69BtYsIqAr8WzBM7BtzA LzmCVub9jLCtFcItH/YivYZl4BK4kTB1axQ2zoZJRYcfAbM0RCUOLkzCdgRcwCOhI7t94BmsQ BZEtLLP/HARGWl2jeOpsZJMwpoCVx9kYMiCkqoCLx6mD9BEbRWUjmzEIyZxbCnFlI5ixgZFnF qF6cWlSWWqRrqpdUlJmeUZKbmJmja2hgrJebWlycmJ6ak5hUrJecn7uJERgt9QwMjDsYL391O sQoycGkJMp7MFUpXIgvKT+lMiOxOCO+qDQntfgQowwHh5IEr9FeoJxgUWp6akVaZg4wbmHSEh w8SiK8V/cApXmLCxJzizPTIVKnGC05tvy+tpaJY9vUe0Cyb/eDtUxCLHn5ealS4rx/QBoEQBo ySvPgxsFSyyVGWSlhXkYGBgYhnoLUotzMElT5V4ziHIxKwryiIFfxZOaVwG19BXQQE9BBApsU QQ4qSURISTUwMlj2SXraik07sH/5rPm1H16o1mxttdjFHKy8fSbj7uvTF4SZZzzZ6/ShNVCzL /mkXRejGLPIbMH0bLYpL8+EL59Xttz3evqEhdlH5bXz1+gsa5/YObEytLGfhctAeeJPhdoKe7 1c5UyJL+GqjAlCXBqrWj7JOM49frc2euvE98LiK9LjhCWUWIozEg21mIuKEwGdySBzKAMAAA= = X-Env-Sender: ketuzsezr@gmail.com X-Msg-Ref: server-11.tower-31.messagelabs.com!1461894737!11298984!1 X-Originating-IP: [209.85.220.196] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 8.34; banners=-,-,- X-VirusChecked: Checked Received: (qmail 43346 invoked from network); 29 Apr 2016 01:52:18 -0000 Received: from mail-qk0-f196.google.com (HELO mail-qk0-f196.google.com) (209.85.220.196) by server-11.tower-31.messagelabs.com with AES128-GCM-SHA256 encrypted SMTP; 29 Apr 2016 01:52:18 -0000 Received: by mail-qk0-f196.google.com with SMTP id l68so3580888qkf.3 for ; Thu, 28 Apr 2016 18:52:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=TzBE7nam3QzfQfDj81CKgwUUnTRBC/GIY+yWcs7juuk=; b=BuGZUlf5Jm8w8u7zjlduToRnAC3VFUc3+f/L8Xw4qkQKbUK5CP2hdqJsT7WCEP8EII JFIva7F4d9warg3M3NdgpKSt059uwmwThRCX22MIMP/UEF2WLPcf79QBfew1WzRpSodf dGu4o1IjJHwM69N1hmpde2rxNDZ/GCQAbh0SFVcRsliLm2NQyMDwKOYaagsJvFgk9H6+ lJ5FFmmn2iTpPxvPXi3JQEPT1h28Tu48bUPBK1tNgTAE1LxIYnJXb6VMHaK5avdi46TG qo4UXLII1RyWMST00eEylmYEJEzboz+fG98B6RIQuVjxLdd2B0GOEGeDWGI/Gu4Aq6tq z3Pw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition :content-transfer-encoding:in-reply-to:user-agent; bh=TzBE7nam3QzfQfDj81CKgwUUnTRBC/GIY+yWcs7juuk=; b=aVvgLJNAI2fBufM1eNjJaHK/HYAThhE9/vcFUQxQsYEvd074vylteO5wsdoIHz80CZ PZGNa9aap/c+n1TcYPaoQY0kLpHgPOg8ONMp0++hCaR2bjg3XfO7pOBxo0D76g+Lcy0x PfhZaJZrtlOxus4DpXagBVP5RV4UAbWEfuZeU1b002DTO/CovIMbG9Nv7grlW+NKE4rD RzITbhGr6ut7AFQJNCtIp+FCGZV19ikfbDZJHL3g6M4+ORBi3tkHUqSo7SPxPc6ZVI4+ dI5OOys2LVB5+rUjLjYSmDDBN2/++mboTBJtDeoJtHt6LTlUZ4Ec+pNFyBglvzMigaIj x2Vg== X-Gm-Message-State: AOPr4FXx5+4IMdDL9duDPXkL4PNhjXCztADmZNjYDsCLMdZxmisPl2oJNTfyBrNnQp67jA== X-Received: by 10.55.124.196 with SMTP id x187mr18009843qkc.204.1461894737377; Thu, 28 Apr 2016 18:52:17 -0700 (PDT) Received: from localhost.localdomain (209-6-196-81.c3-0.smr-ubr2.sbo-smr.ma.cable.rcn.com. [209.6.196.81]) by smtp.gmail.com with ESMTPSA id d145sm3720930qhc.31.2016.04.28.18.52.15 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 28 Apr 2016 18:52:16 -0700 (PDT) Date: Thu, 28 Apr 2016 21:52:14 -0400 From: Konrad Rzeszutek Wilk To: Andrew Cooper Message-ID: <20160429015212.GA12857@localhost.localdomain> References: <20160428114710.GE20738@citrix.com> <1461867412-3635-1-git-send-email-konrad.wilk@oracle.com> <57229B7A.7030306@citrix.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <57229B7A.7030306@citrix.com> User-Agent: Mutt/1.5.24 (2015-08-30) Cc: Stefano Stabellini , Wei Liu , George Dunlap , Tim Deegan , Ian Jackson , Ross Lagerwall , Jan Beulich , xen-devel@lists.xenproject.org, Roger Pau =?iso-8859-1?Q?Monn=E9?= Subject: Re: [Xen-devel] [PATCH] xsplice: Don't perform multiple operations on same payload once work is scheduled. X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable 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 On Fri, Apr 29, 2016 at 12:23:38AM +0100, Andrew Cooper wrote: > On 28/04/2016 19:16, Konrad Rzeszutek Wilk wrote: > > diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c > > index 1b67d39..48088ce 100644 > > --- a/xen/common/xsplice.c > > +++ b/xen/common/xsplice.c > > @@ -1099,6 +1099,13 @@ static void xsplice_do_action(void) > > data->rc = rc; > > } > > > > +static bool_t is_work_scheduled(struct payload *data) > > const struct payload *data Yes! > > Otherwise, Reviewed-by: Andrew Cooper And of course 5 hours later I realized there is a more straightforward for this. It follows the same idea but it piggyback on data->rc being set by 'schedule_work' to -EAGAIN once work is scheduled: It could even be rolled in "xsplice: Implement support for applying/reverting/replacing patches." From 83053e3f984b67dfae74cb64e75b8871b9d236ca Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Thu, 28 Apr 2016 21:22:49 -0400 Subject: [PATCH] xsplice: Check data->rc for -EAGAIN to guard against races. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently it is possible to: 1) xc_xsplice_apply() \-> xsplice_action spin_lock(payload_lock) \- schedule_work() data->rc=-EAGAIN spin_unlock(payload_lock); 2) xc_xsplice_unload() \-> xsplice_action spin_lock(payload_lock) free_payload(data); spin_unlock(payload_lock); .. all CPUs are quiesced. 3) check_for_xsplice_work() \-> apply_payload \-> arch_xsplice_apply_jmp BOOM data->rc =0 The reason is that state is in 'CHECKED' which changes to 'APPLIED' once check_for_xsplice_work finishes (and it updates data->rc to zero). But we have a race between 1) -> 3) where one can manipulate the payload (as the state is in 'CHECKED' from which you can apply/revert and unload). This patch adds a simple check on data->rc to see if it is in -EAGAIN which means that schedule_work has been called for this payload. If the payload aborts in check_for_xsplice_work (timed out, etc), the data->rc will be -EBUSY -so one can still unload the payload or retry the operation. Signed-off-by: Konrad Rzeszutek Wilk Reported-by: Roger Pau Monné --- CC: Andrew Cooper CC: George Dunlap CC: Ian Jackson CC: Jan Beulich CC: Konrad Rzeszutek Wilk CC: Stefano Stabellini CC: Tim Deegan CC: Wei Liu CC: Ross Lagerwall --- --- xen/common/xsplice.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c index 1b67d39..0bc7e0f 100644 --- a/xen/common/xsplice.c +++ b/xen/common/xsplice.c @@ -1363,6 +1363,9 @@ static int xsplice_action(xen_sysctl_xsplice_action_t *action) return PTR_ERR(data); } + if ( data->rc == -EAGAIN ) /* schedule_work has been called. */ + goto out; + switch ( action->cmd ) { case XSPLICE_ACTION_UNLOAD: @@ -1423,6 +1426,7 @@ static int xsplice_action(xen_sysctl_xsplice_action_t *action) break; } + out: spin_unlock(&payload_lock); return rc;