From patchwork Tue Aug 27 10:07:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Richard Earnshaw \(lists\)" X-Patchwork-Id: 172270 Delivered-To: patch@linaro.org Received: by 2002:a92:d204:0:0:0:0:0 with SMTP id y4csp5563178ily; Tue, 27 Aug 2019 03:07:30 -0700 (PDT) X-Google-Smtp-Source: APXvYqzAa6LOIiRmFp5V2AdhLIWPkVwHfUPRC+yTfl4KKASp2KuJToX6nAqjHPqdFiFWFhwaOTzK X-Received: by 2002:a17:90a:2767:: with SMTP id o94mr23859660pje.25.1566900450014; Tue, 27 Aug 2019 03:07:30 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1566900450; cv=none; d=google.com; s=arc-20160816; b=AVzZFbSnIYvZtQ54//UcA3xLPMPp/zsvDK3t9H0o9qmoSeYbCrDB0JCQ0TnIVZlru6 41np2EvKBl+G2WJv9KyHvSKSPXfUbNf6VV3fIieGbR2dl9TdQGa7mVLjgQGE5JChQrbC BcHDxqAvtqLht9jVKdr+4g5AVhGQH1cTE/EfLvuD5cnSzwQ1/rRdR/cRFYDNHQzNh1Ze 6wiiozgAGUDOUhJkNRYRR6asWN0J4uw5BNPz8TdJEfWDxZ20Z1azp+RPSCSQgNehXF+W OFXcExvUs+gVmlMo9nlCisv9BrWsJY1besP4jmbWk3PWxVlC2ku4yKVDzGi91YYiRNHz HDUA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=mime-version:user-agent:date:message-id:subject:from:to :delivered-to:sender:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:mailing-list:dkim-signature :domainkey-signature; bh=EBnFlUCIU7vHlhzTbDc8oJInuEuSDSFisHlvo0zeRSo=; b=A02D2xdAvA++Aa8EyyIFn0DccZfBEUNZ+cyHgy0J0KgkfW/6FgDOFhDrzfBtNlajOD 9cMwdj+sEReqvLFqV2ABAPeFmTT+t0qDLPLtsQ3VbBuZ+j0fTPGLzXxB2ljG1XFQhots nDrRDSdJ348no2jIefBcB6mpjx3f7loJ6bY8T0RCgglZTmoflMDy69d+55jC07RHWaoT QkeOUdESGAP3jtaLooyOF5lZ9r9hc47A5ehhrDmuAP7eJGTtUzufrJP6T+OMJh01eEqu ciWZ9xaM7xj+uJeg9yvOLARb0fdDFzQShnNSpqamzd9yFunH8SZvBfSAv2SRfxWulByG BlXA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=whVVnNuO; spf=pass (google.com: domain of gcc-patches-return-507763-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="gcc-patches-return-507763-patch=linaro.org@gcc.gnu.org" Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id 97si12432954pld.245.2019.08.27.03.07.29 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 27 Aug 2019 03:07:30 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-507763-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=whVVnNuO; spf=pass (google.com: domain of gcc-patches-return-507763-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="gcc-patches-return-507763-patch=linaro.org@gcc.gnu.org" DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=L7omA9p1NiXdxnnB+V7Hijk7TFAPcrnU+cXAbvd6yBvshng8ne 7IXzq+NHg2c7OrI+zfoURszkEK1lTg71lMBEVmZm2XTqVzP76kGmJogiAGzo5FFI ubh/NNkP17XDdKyYzoRndImYGcagHXVl1r6bU7N66vHSFe3Ua7p4sOE1M= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; s= default; bh=yZFlPAp1G2zcpAnaxQiYeiY8ibc=; b=whVVnNuOFt4u9E01nHXH +5RZr+MnQRSmrFB2Z7yEmwHZ15cMmGa6jEhZSTfcK9iNcWFKn14AQycqUfXLvUDG Xrs5mXQbSt5kH+nqRfKnEnZaOa/eTWI5PxCZ7rmKygzD0xt2CZHRjuJ+2/ws9gPP +opC6drd7G28jMfXajRi250= Received: (qmail 41838 invoked by alias); 27 Aug 2019 10:07:18 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 41696 invoked by uid 89); 27 Aug 2019 10:07:17 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, SPF_PASS autolearn=ham version=3.3.1 spammy=identified, warranted, SPLIT, mm X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.110.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 27 Aug 2019 10:07:16 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D5CD1337; Tue, 27 Aug 2019 03:07:14 -0700 (PDT) Received: from e120077-lin.cambridge.arm.com (e120077-lin.cambridge.arm.com [10.2.206.91]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7D5633F718; Tue, 27 Aug 2019 03:07:14 -0700 (PDT) To: gcc-patches@gcc.gnu.org From: "Richard Earnshaw (lists)" Subject: [arm][aarch64] Add comments warning that stack-protector initializer insns shouldn't be split Message-ID: Date: Tue, 27 Aug 2019 11:07:13 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 Following the publication of https://kb.cert.org/vuls/id/129209/ I've been having a look at GCC's implementation for Arm and AArch64. I haven't identified any issues yet, but it's a bit early to be completely sure. One observation, however, is that the instruction sequence that initializes the stack canary might be vulnerable to producing a reusable value if it were ever split early. I don't think we ever would, because the memory locations involved with the stack protector are all marked volatile to ensure that the values are only loaded at the point in time when the test is intended to happen, and that also has the effect of making it unlikely that the value would be reused without reloading. Nevertheless, defence in depth is probably warranted here. So this patch just adds some comments warning that the patterns should not be split. * config/arm/arm.md (stack_protect_set_insn): Add security-related comment. * config/aarch64/aarch64.md (stack_protect_set_): Likewise. committed to trunk. Index: gcc/config/aarch64/aarch64.md =================================================================== --- gcc/config/aarch64/aarch64.md (revision 274945) +++ gcc/config/aarch64/aarch64.md (working copy) @@ -7016,6 +7016,8 @@ } [(set_attr "type" "mrs")]) +;; DO NOT SPLIT THIS PATTERN. It is important for security reasons that the +;; canary value does not live beyond the life of this sequence. (define_insn "stack_protect_set_" [(set (match_operand:PTR 0 "memory_operand" "=m") (unspec:PTR [(match_operand:PTR 1 "memory_operand" "m")] @@ -7022,7 +7024,7 @@ UNSPEC_SP_SET)) (set (match_scratch:PTR 2 "=&r") (const_int 0))] "" - "ldr\\t%2, %1\;str\\t%2, %0\;mov\t%2,0" + "ldr\\t%2, %1\;str\\t%2, %0\;mov\t%2, 0" [(set_attr "length" "12") (set_attr "type" "multiple")]) Index: gcc/config/arm/arm.md =================================================================== --- gcc/config/arm/arm.md (revision 274945) +++ gcc/config/arm/arm.md (working copy) @@ -8208,6 +8208,8 @@ [(set_attr "arch" "t1,32")] ) +;; DO NOT SPLIT THIS INSN. It's important for security reasons that the +;; canary value does not live beyond the life of this sequence. (define_insn "*stack_protect_set_insn" [(set (match_operand:SI 0 "memory_operand" "=m,m") (unspec:SI [(mem:SI (match_operand:SI 1 "register_operand" "+&l,&r"))] @@ -8215,8 +8217,8 @@ (clobber (match_dup 1))] "" "@ - ldr\\t%1, [%1]\;str\\t%1, %0\;movs\t%1,#0 - ldr\\t%1, [%1]\;str\\t%1, %0\;mov\t%1,#0" + ldr\\t%1, [%1]\;str\\t%1, %0\;movs\t%1, #0 + ldr\\t%1, [%1]\;str\\t%1, %0\;mov\t%1, #0" [(set_attr "length" "8,12") (set_attr "conds" "clob,nocond") (set_attr "type" "multiple")