From patchwork Wed Oct 26 09:34:21 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Cooper X-Patchwork-Id: 9396479 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 EA79D60236 for ; Wed, 26 Oct 2016 09:37:03 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A69FC299AE for ; Wed, 26 Oct 2016 09:37:03 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9AF55299B3; Wed, 26 Oct 2016 09:37:03 +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=-2.6 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, URIBL_SBL autolearn=ham version=3.3.1 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.wl.linuxfoundation.org (Postfix) with ESMTPS id D5B46299AE for ; Wed, 26 Oct 2016 09:37:01 +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 1bzKbN-0003sZ-PO; Wed, 26 Oct 2016 09:34:29 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bzKbM-0003sT-Mh for xen-devel@lists.xen.org; Wed, 26 Oct 2016 09:34:28 +0000 Received: from [85.158.139.211] by server-12.bemta-5.messagelabs.com id AF/EF-09561-3A870185; Wed, 26 Oct 2016 09:34:27 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrBLMWRWlGSWpSXmKPExsXitHSDve6iCoE Ig8PrjCyWfFzM4sDocXT3b6YAxijWzLyk/IoE1oz9XycyF8wUrWi8e4WtgfGeYBcjB4eEgL/E zX7hLkZODjYBfYndLz4xgdgiAuoSpzsusnYxcnEwC0xjlHjQeZQRJCEsECyxdfVGMJtFQFVi4 u75zCA2r4CHxNrdzWC2hICcxPnjP8FsIQE1iWv9l9ghagQlTs58wgJiMwtISBx88QKqnlvi9u mpzBMYeWYhKZuFpGwBI9MqRvXi1KKy1CJdY72kosz0jJLcxMwcXUMDU73c1OLixPTUnMSkYr3 k/NxNjMAQYQCCHYx7/zkdYpTkYFIS5c31FogQ4kvKT6nMSCzOiC8qzUktPsQow8GhJMHrVw6U EyxKTU+tSMvMAQYrTFqCg0dJhNcDJM1bXJCYW5yZDpE6xagoJc4bBpIQAElklObBtcEi5BKjr JQwLyPQIUI8BalFuZklqPKvGMU5GJWEeXtBpvBk5pXATX8FtJgJaPH0dLDFJYkIKakGxh1npl tMSFG51MPY2e/pkOYZNPnGguKnzN+69rc969ZNL3F+stf42ITHVzcuzFzN+mDevCmHZgtuWHz qn0aX1YrXId+WXj9U/bK5i70r37vf6+Xdc48fzah5tP3hoShzH+caOROH3J0x3sVMryOkq6tz P67ua783lfWen5TyMu3ExOzAZ/d7fyqxFGckGmoxFxUnAgBpf0tAiwIAAA== X-Env-Sender: prvs=1002db643=Andrew.Cooper3@citrix.com X-Msg-Ref: server-9.tower-206.messagelabs.com!1477474465!66974345!1 X-Originating-IP: [66.165.176.63] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogNjYuMTY1LjE3Ni42MyA9PiAzMDYwNDg=\n, received_headers: No Received headers X-StarScan-Received: X-StarScan-Version: 9.0.13; banners=-,-,- X-VirusChecked: Checked Received: (qmail 52691 invoked from network); 26 Oct 2016 09:34:26 -0000 Received: from smtp02.citrix.com (HELO SMTP02.CITRIX.COM) (66.165.176.63) by server-9.tower-206.messagelabs.com with RC4-SHA encrypted SMTP; 26 Oct 2016 09:34:26 -0000 X-IronPort-AV: E=Sophos;i="5.31,550,1473120000"; d="scan'208";a="394692938" From: Andrew Cooper To: Xen-devel Date: Wed, 26 Oct 2016 10:34:21 +0100 Message-ID: <1477474461-3843-1-git-send-email-andrew.cooper3@citrix.com> X-Mailer: git-send-email 2.1.4 MIME-Version: 1.0 X-DLP: MIA2 Cc: Andrew Cooper , Ian Jackson , Wei Liu , David Scott Subject: [Xen-devel] [PATCH for-4.8] tools/oxenstored: Avoid allocating invalid transaction ids 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-Virus-Scanned: ClamAV using ClamSMTP The transaction id of 0 is reserved, meaning "not in a transaction". It is up to the xenstored server to allocate transaction ids. While oxenstored starts its ids at 1, but insufficient care is taken with truncation cases. A 32bit oxenstored has an int with 31 bits of width, meaning that the transaction id will wrap around to 0 after 2 billion transactions. A 64bit oxenstored has an int with 63 bits of width, meaning that once 4 billion transactions are used, the allocated id will be truncated when written into the uin32_t field in the ring. This causes the client to reply with the truncated id, breaking any further attempt to use any transactions. Limit all transaction ids to the range between 1 and 0x7ffffffe. This is the best which can be done without making oxenstored depend on Stdint or Cstruct, yet still work for 32bit builds. Also check that the proposed new transaction id isn't currently in use. For the first 2 billion transactions there is no chance of a collision, and after that, the chance is at most 20 (the default open transaction quota) in 2 billion. Signed-off-by: Andrew Cooper Acked-by: David Scott --- CC: Ian Jackson CC: Wei Liu CC: David Scott --- tools/ocaml/xenstored/connection.ml | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tools/ocaml/xenstored/connection.ml b/tools/ocaml/xenstored/connection.ml index b18336f..0b47009 100644 --- a/tools/ocaml/xenstored/connection.ml +++ b/tools/ocaml/xenstored/connection.ml @@ -216,14 +216,23 @@ let fire_watch watch path = let data = Utils.join_by_null [ new_path; watch.token; "" ] in send_reply watch.con Transaction.none 0 Xenbus.Xb.Op.Watchevent data -let find_next_tid con = - let ret = con.next_tid in con.next_tid <- con.next_tid + 1; ret +(* Search for a valid unused transaction id. *) +let rec valid_transaction_id con proposed_id = + (* Clip proposed_id to the range [1, 0x7ffffffe] *) + let id = if proposed_id <= 0 || proposed_id >= 0x7fffffff then 1 else proposed_id in + + if Hashtbl.mem con.transactions id then ( + (* Outstanding transaction with this id. Try the next. *) + valid_transaction_id con (id + 1) + ) else + id let start_transaction con store = if !Define.maxtransaction > 0 && not (is_dom0 con) && Hashtbl.length con.transactions > !Define.maxtransaction then raise Quota.Transaction_opened; - let id = find_next_tid con in + let id = valid_transaction_id con con.next_tid in + con.next_tid <- id + 1; let ntrans = Transaction.make id store in Hashtbl.add con.transactions id ntrans; Logging.start_transaction ~tid:id ~con:(get_domstr con);