From mboxrd@z Thu Jan 1 00:00:00 1970 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=-3.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 F23B0C2D0A3 for ; Tue, 3 Nov 2020 15:29:02 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 7893D22226 for ; Tue, 3 Nov 2020 15:29:02 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="GdK14lL7" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7893D22226 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=mXOQ5oUMLoZnnzNMZavE4ejruWzKTv1BVr5ROzyN5AE=; b=GdK14lL7IcKJ3rZqCgpHuLqOY NCs3FzzgVLY1gip4xtZSr73rw/8TMLx+U5jEKqBEfyS2zwDgujr2DDUlhd6YQgRvyg6fJL//eY11X QtrD/HiesTIFpt8JCvAQS11cBWbb0RygqefgH18rOrb9dRwv0nMrviyVCx4yRkppSjheWoQd2e1xd fQ1VJdrP8TVz/gvtkdTJ/CC5KA+RWulckzJKqMWXG2Gh+jxryu/N9LYY0d1vn7TlvPZYc1agfUHXa grtAEQEmdTaS9kXIN483Mya0dUz/SbgTUgmPD11hERH0xRFavZ9E7TyLcRXU64aI6ZGFsTmNERB8s X026LPnLw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kZyEa-0001XV-A4; Tue, 03 Nov 2020 15:28:32 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kZyEX-0001Wn-NN for linux-arm-kernel@lists.infradead.org; Tue, 03 Nov 2020 15:28:31 +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 AF6F4139F; Tue, 3 Nov 2020 07:28:26 -0800 (PST) Received: from C02TD0UTHF1T.local (unknown [10.57.57.89]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 737743F66E; Tue, 3 Nov 2020 07:28:25 -0800 (PST) Date: Tue, 3 Nov 2020 15:28:22 +0000 From: Mark Rutland To: Peter Collingbourne Subject: Re: [PATCH] kasan: arm64: support specialized outlined tag mismatch checks Message-ID: <20201103152822.GF40454@C02TD0UTHF1T.local> References: <20201029205944.2769-1-pcc@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201029205944.2769-1-pcc@google.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201103_102829_916287_18B6F865 X-CRM114-Status: GOOD ( 25.07 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Catalin Marinas , Mark Brown , Will Deacon , Linux ARM , Andrey Konovalov Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Peter, On Thu, Oct 29, 2020 at 01:59:44PM -0700, Peter Collingbourne wrote: > Outlined checks require a new runtime function with a custom calling > convention. Add this function to arch/arm64/lib. [...] > +/* > + * Report a tag mismatch detected by tag-based KASAN. > + * > + * This function has a custom calling convention in order to minimize the sizes > + * of the compiler-generated thunks that call it. All registers except for x16 > + * and x17 must be preserved. This includes x0 and x1 which are used by the > + * caller to pass arguments, and x29 (the frame pointer register). In order to > + * allow these registers to be restored the caller spills them to stack. The 256 > + * bytes of stack space allocated by the caller must be deallocated on return. This is a bit difficult to follow. Am I right in understanding that the caller has pre-allocated 256 bytes on the stack by decrementing the SP, yet it's up to the callee to undo that? That seems bizarre to me, and it would be much easier (and more generally useful) for the caller to leave it to the trampoline to allocate/free its own space. > + * > + * This function takes care of transitioning to the standard AAPCS calling > + * convention and calls the C function kasan_tag_mismatch to report the error. > + * > + * Parameters: > + * x0 - the fault address > + * x1 - an encoded description of the faulting access > + */ > +SYM_FUNC_START(__hwasan_tag_mismatch) For non-AAPCS code you should use SYM_CODE_*() rather than SYM_FUNC_*(), and place any BTI instructions necessary manually. We should probably add SYM_NONSTD_FUNC_*() helpers for trampolines, as it looks like we didn't add BTIs into the ftrace trampolines (and module PLTs can get there via indirect branches). I'll put together a point-fix for ftrace. > + add x29, sp, #232 Where has this offset come from? Is there already a frame record created by the caller? If so, that should be mentioned in the description of the calling convention. If not, I don't think this code works. > + stp x2, x3, [sp, #16] > + stp x4, x5, [sp, #32] > + stp x6, x7, [sp, #48] > + stp x8, x9, [sp, #64] > + stp x10, x11, [sp, #80] > + stp x12, x13, [sp, #96] > + stp x14, x15, [sp, #112] Is there a reason this starts at an offset of 16? For offsets please use (16 * n) like we do in entry.S, e.g. | stp x2, x3, [sp, #16 * 0] | stp x4, x5, [sp, #16 * 1] | stp x4, x5, [sp, #16 * 2] ... as it makes the relationship clearer. > +#ifndef CONFIG_SHADOW_CALL_STACK > + str x18, [sp, #128] > +#endif This doesn't look right to me. The shadow call stack should remain balanced across a call, so why do you need to save x18? any manipulation within kasan_tag_mismatch should be balanced. Did you mean to save the return address to the shadow stack, rather than saving the shadow stack pointer? > + mov x2, x30 > + bl kasan_tag_mismatch We didn't preserve x0 and x1 above, so they can be clobbered here, but the comment said we needed to preserve them. > + ldp x29, x30, [sp, #232] This doesn't look right. This function didn't save x29/x30 to the stack, so the only way this could work is if the caller had a redundant frame record pointing to itself. > +#ifndef CONFIG_SHADOW_CALL_STACK > + ldr x18, [sp, #128] > +#endif As above, this doesn't look right. > + ldp x14, x15, [sp, #112] > + ldp x12, x13, [sp, #96] > + ldp x10, x11, [sp, #80] > + ldp x8, x9, [sp, #64] > + ldp x6, x7, [sp, #48] > + ldp x4, x5, [sp, #32] > + ldp x2, x3, [sp, #16] > + ldp x0, x1, [sp], #256 As above, we didn't save x0 and x1 -- did the caller do that? Likewise, did the caller allocate 256 bytes that we have to deallocate? That's *really* confusing. Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel