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=-9.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 28A85C48BDF for ; Tue, 22 Jun 2021 18:39:41 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 EC877611CE for ; Tue, 22 Jun 2021 18:39:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EC877611CE Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.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=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=EVsdM/l9vKCg/UL1xMhcXxpPLvpdERoqS3HOYvRjYcA=; b=stQGWHrQMDuKzL gViQVgs0fS/b8QIuMITUioSjBPvhsVD8kbsm5d2gfOu74SMJA+iALaAFvTz64Vvy9J1jpBvZPKzZw kZ0dLMChpSfW95wSLreeLi2Ue4F3teca+iLjwmPFB3JCmMImCG1bkeLPUxdGPdJIHIW2Hy4+9MieK 1txgPQ6EmfAIdOfHqiEoOgpZlSVOifWpgR1YhNLkQI08ELUqXELUPsmqtuUuVuFDsJaptIYjKAuuU 7KLTvQaCWYrUcdHwreSRQ5Zq2uGV/gu43IhtZ+DLA4r0HcaAXV6Jyx1SSGu9jDzSH1fp3HGlXIjBr V4LFDJvwYAnZxT4kWWTw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lvlHj-008AYF-Uy; Tue, 22 Jun 2021 18:38:08 +0000 Received: from mail-il1-x136.google.com ([2607:f8b0:4864:20::136]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lvlHf-008AXd-BE for linux-arm-kernel@lists.infradead.org; Tue, 22 Jun 2021 18:38:05 +0000 Received: by mail-il1-x136.google.com with SMTP id a11so218278ilf.2 for ; Tue, 22 Jun 2021 11:38:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=0xkBITuvnt1C6jGNX27ED/iTdpintDLJvtGOu91EBIY=; b=FBX7BLLTu3cuWlSbRJU7dQHh1sWRNUcOyryGbE7CchX5zRvQgXJJ8jOvtslR/0bsBX BavSJ1YhcmuLojVrjVRo2S6Vs4bsPZBzPKzbmsn0kV16S0mBN5xf9IKqj8WF3iOKyM+t EmOx2NJL6PKjd9jPfSiBS45oeI/99uRwYmDDOR3VdpHfUPduEImoK2mZjMx1CyjcEBwf HTjul/J76+qjI+/PnzI16TvDrWvbF939VpabHF//as+QHm0U3GUdbLTF7ikD8PeDVLAP JGQKdOxzXopzNblAiBzi3bZdSKOV416WlKIqLnNc2s/7gbFhKjgoZvfaDuZrb3cNEE0b xdNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=0xkBITuvnt1C6jGNX27ED/iTdpintDLJvtGOu91EBIY=; b=UdHX7tbYSZ7Ka6gECJKt6ag+03hDQA2c2teDzAtSG8O7Q9xoUIswOjtFWPlyEciLCA +0xbUL8iSBs+afwiO/C/2Eh3BNOBwqmTSe7Al9vSxy6wXJ5LqFlSOPj0CPmogV3sTPjW /f5B8/Dimcw/iMKjRFEC0H949HHJdciJWFayzVdH3InVjsMGyQkerF0jvgNW+djGtunk eGraIgRKORjpHz1fMZtJ/kr7EmB48nFdD3Y81LcMCFroWYuTVHZi07sIl7X1WXc1PZdi zQlczrL/ofJRqD7lj/GY+MlaPC+VxM5t+WvOJ5Jkdav/Brfv8LncKHvmWKa5KBxtmbtA u9Uw== X-Gm-Message-State: AOAM532l28OjnVjhbpTcVaf8KBG0925OIFA+xIW3gpmZAww/NbwJocqA whBehf6r3bRUyt1Lf0bBwTNOd5cxbher/vmzPArf5Q== X-Google-Smtp-Source: ABdhPJxMRRRsFrDnDqljb90xHHxeElUqbPZDkgfC/yfIg8/ntRkoBgmiwd6gCn04maGyMSXvwO1LpopmaXF4Rtm4+Ss= X-Received: by 2002:a05:6e02:10c3:: with SMTP id s3mr45756ilj.37.1624387082246; Tue, 22 Jun 2021 11:38:02 -0700 (PDT) MIME-Version: 1.0 References: <20210615203807.3582804-1-pcc@google.com> <20210617215829.GD25403@willie-the-truck> <20210618150953.GH16116@arm.com> <20210621123936.GB29283@willie-the-truck> <20210621151858.GC11552@arm.com> <20210621173902.GA29713@willie-the-truck> <20210621185036.GD11552@arm.com> In-Reply-To: <20210621185036.GD11552@arm.com> From: Peter Collingbourne Date: Tue, 22 Jun 2021 11:37:50 -0700 Message-ID: Subject: Re: [PATCH v5] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis To: Catalin Marinas Cc: Will Deacon , Vincenzo Frascino , Evgenii Stepanov , Linux ARM , Szabolcs Nagy X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210622_113803_454765_02B6172F X-CRM114-Status: GOOD ( 80.22 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 On Mon, Jun 21, 2021 at 11:50 AM Catalin Marinas wrote: > > On Mon, Jun 21, 2021 at 06:39:02PM +0100, Will Deacon wrote: > > On Mon, Jun 21, 2021 at 04:18:59PM +0100, Catalin Marinas wrote: > > > On Mon, Jun 21, 2021 at 01:39:37PM +0100, Will Deacon wrote: > > > > On Fri, Jun 18, 2021 at 04:09:55PM +0100, Catalin Marinas wrote: > > > > > On Thu, Jun 17, 2021 at 10:58:30PM +0100, Will Deacon wrote: > > > > > > On Tue, Jun 15, 2021 at 01:38:07PM -0700, Peter Collingbourne wrote: > > > > > > > +Upgrading to stricter tag checking modes > > > > > > > +---------------------------------------- > > > > > > > + > > > > > > > +On some CPUs the performance of MTE in stricter tag checking modes > > > > > > > +is similar to that of less strict tag checking modes. This makes it > > > > > > > +worthwhile to enable stricter checks on those CPUs when a less strict > > > > > > > +checking mode is requested, in order to gain the error detection > > > > > > > +benefits of the stricter checks without the performance downsides. To > > > > > > > +opt into upgrading to a stricter checking mode on those CPUs, the user > > > > > > > +can set the ``PR_MTE_DYNAMIC_TCF`` flag bit in the ``flags`` argument > > > > > > > +to the ``prctl(PR_SET_TAGGED_ADDR_CTRL, flags, 0, 0, 0)`` system call. > > > > > > > + > > > > > > > +This feature is currently only supported for upgrading from > > > > > > > +asynchronous mode. To configure a CPU to upgrade from asynchronous mode > > > > > > > +to synchronous mode, a privileged user may write the value ``1`` to > > > > > > > +``/sys/devices/system/cpu/cpu/mte_upgrade_async``, and to disable > > > > > > > +upgrading they may write the value ``0``. By default the feature is > > > > > > > +disabled on all CPUs. > > > > > > > + > > > > > > > Initial process state > > > > > > > --------------------- > > > > > > > > > > > > > > @@ -128,6 +147,7 @@ On ``execve()``, the new process has the following configuration: > > > > > > > - ``PR_TAGGED_ADDR_ENABLE`` set to 0 (disabled) > > > > > > > - Tag checking mode set to ``PR_MTE_TCF_NONE`` > > > > > > > - ``PR_MTE_TAG_MASK`` set to 0 (all tags excluded) > > > > > > > +- ``PR_MTE_DYNAMIC_TCF`` set to 0 (disabled) > > > > > > > - ``PSTATE.TCO`` set to 0 > > > > > > > - ``PROT_MTE`` not set on any of the initial memory maps > > > > > > > > > > > > Something about this doesn't sit right with me, as we're mixing a per-task > > > > > > interface with a per-cpu interface for selecting async/sync MTE and the > > > > > > priorities are somewhat confusing. > > > > > > > > > > > > I think a better interface would be for the sysfs entry for each CPU to > > > > > > allow selection between: > > > > > > > > > > > > task : Honour the prctl() (current behaviour) > > > > > > async : Force async for tasks using MTE > > > > > > sync : Force sync for tasks using MTE > > > > > > none : MTE disabled > > > > > > > > > > > > i.e. the per-cpu setting is an override. > > > > > > > > > > As Peter mentioned, forcing it is a potential ABI break, so such feature > > > > > would need backporting to 5.10. There's also a minor use-case that came > > > > > up in the early discussions - an app may want to use async mode only for > > > > > reporting but forcing it to sync would break such application (since > > > > > sync mode prevents the faulty access from taking place). > > > > > > > > Why is it an ABI break? The default will be "task" which behaves exactly as > > > > things do today. If the policy is explicitly changed by userspace, then you > > > > get new behaviour. I don't really see why this is different to e.g. writing > > > > /proc/sys/vm/mmap_min_addr and having some applications fail because they > > > > rely on the default setting. > > > > > > That's slightly different from mmap_min_addr, it depends on the user > > > expectations. Most applications have no interest in a MAP_FIXED of > > > address 0, so they wouldn't notice a non-zero setting. > > > > Not sure; if I set mmap_min_addr to 1GB then applications start to break > > (32-bit applications SEGV instantly). So I think it's pretty similar. > > You don't expect applications to be tolerant to randomly high > mmap_min_addr, hence admins don't set this to more than a page. However, > for the proposed TCF sysfs interface, we do expect applications to cope > with any forced value, either by documenting that the PR_* settings are > useless or allowing an application to opt in to being forced. Either > way, it's a significant difference between mmap_min_addr and the > proposed MTE control. > > If you imply that people shouldn't use the new TCF sysfs controls > because it may break applications, that's fine as well, we document it > as a dangerous feature, only to be used if you have control of the > user-space deployment (Android is an ideal target here). > > > > The semantics of MTE TCF none/sync/async are different and an > > > application may have different expectations. For example, it may not > > > want any tag checking, just being able to read/write tags. Or it may > > > want just some lightweight monitoring with a simple restart after an > > > async signal (sync requires either emulating the access or setting the > > > PSTATE.TCO bit). > > > > I think this is an argument against doing this at all. Realistically, > > if PR_MTE_DYNAMIC_TCF is the "go fast" mode, then people will set it > > blindly and applications will just be expected to deal with a combination > > of sync and async faults; I really don't think the flag changes anything in > > that regards. It's not a buy-in from the application, given that the visible > > behaviour is unknown and dynamic. > > Most applications won't care about the mode, they just want to crash > when some incorrect access happens. But not having an opt-in (or > opt-out, it works either way) a minority of other use-cases are no > longer possible. Apps can't even rely on having the same TCF for the > lifetime of a thread. > > > > Forcing the TCF via sysfs may be seen as a user problem but that's > > > pretty much rendering the application choice of the tag check mode > > > useless since an admin could override it. > > > > I think the application choice _is_ useless if we decide to offer a > > mechanism where it is set on a per-cpu basis instead. > > It depends on how we implement this mechanism. Do we want to preserve > the application choice via an opt-in/out or we remove such option > entirely? That's a significant ABI change IMO. > > Just stating that setting the sysfs to anything other than "task" breaks > applications looks like discouraging people from using it. Maybe we can > even add an EXPERT dependency. > > > > > > So I'd rather leave it up to the user task to decide whether its choice > > > > > can be changed. Peter introduced a new PR_MTE_DYNAMIC_TCF for this > > > > > purpose (or a different name if you have a better suggestion). > > > > > > > > I don't see how PR_MTE_DYNAMIC_TCF is useful to userspace, really. It's an > > > > extra bit of logic to go and set it, but then what? It doesn't know which > > > > behaviour it's getting, so it just feels like an extra hoop to jump through > > > > without actually adding anything useful. > > > > > > Well, without this additional bit, an application can't rely on the mode > > > it requested. With an additional forced tagged address enable, we might > > > as well remove the prctl() altogether (well, that wouldn't be a bad > > > thing). > > > > I think the prctl() is still useful to say whether an application wants MTE > > or not, but I'm inclined to agree that sync vs async shouldn't be part of > > the prctl() call. > > That means that if the user choice was PR_MTE_TCF_NONE, it won't be > forced to any of the sync vs async, which makes sense if we go this > route. The async/sync (and a new asymmetric mode in v8.7) could become > just a PR_MTE_TCF_CHECKED. However, if glibc upstreamed the MTE support > already, it will be harder to remove the PR_* controls already defined > (well, we can still keep the sync/async as a hint) For Android I think we will always want there to be a way to select the "minimum" mode. This is because sync mode provides slightly more security than async, as well as being relied on for deterministic error reporting. If we just have PR_MTE_TCF_CHECKED and sync/async is taken as a hint, it means that we can't rely on the processor to be in sync mode when it takes a tag check fault, so we won't necessarily see the error report or detect the fault early enough. > > > Given that there are no real users of MTE yet, we have some choice of > > > tweaking the ABI, backporting to 5.10. The question: is the expectation > > > that the sysfs forcing of TCF is limited to deployments where the user > > > space is tightly controlled (e.g. Android with most apps starting from > > > zygote) or we allow it to become more of a general hint of what's the > > > fastest check on a CPU? If the former, I'm fine with forcing without any > > > additional bit, though I'd still prefer the opt-in. For the latter, I'd > > > like some wider discussion with non-Android folk on what they expect > > > from the TCF setting. Otherwise simply using PROT_MTE would may lead to > > > tag check faults. > > > > I don't think there's anything Android-specific here. The problem being > > solved concerns big/little SoCs with MTE, and I think it's up to the > > distribution how the sysfs stuff is used. > > But distros don't control what applications are running, so most likely > they would disable the sysfs control entirely. At that point, the > feature becomes primarily an Android play. > > Anyway, I'm not dead against forcing of the TCF mode regardless of the > user choice but I'd like to ensure that we don't miss other use-cases or > that we don't make the sysfs control an expert-only feature. > > Adding Szabolcs to get a view from the glibc perspective. Given these diverging opinions my view is that we should choose whichever option leaves our options open for the future. For example, imagine that we make the ABI change now such that upgrades may happen for all applications and we don't have PR_MTE_DYNAMIC_TCF. This means that applications no longer have a guarantee of their TCF mode which may preclude some use cases (if we add an opt out later, applications will be affected when running on the kernel versions between when we changed the ABI and when we added the opt out). On the other hand, if we introduce PR_MTE_DYNAMIC_TCF, we can always make the ABI change later and start ignoring the PR_MTE_DYNAMIC_TCF flag at that point. Maybe the best compromise would be to change the ABI and at the same time add the opt out, but I don't have a strong opinion. Peter _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel