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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1D8AEC021AA for ; Wed, 19 Feb 2025 13:50:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 91B96280232; Wed, 19 Feb 2025 08:50:50 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8CC1E28022F; Wed, 19 Feb 2025 08:50:50 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 71E9E280232; Wed, 19 Feb 2025 08:50:50 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 4D37028022F for ; Wed, 19 Feb 2025 08:50:50 -0500 (EST) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 7F4DE51B44 for ; Wed, 19 Feb 2025 13:50:30 +0000 (UTC) X-FDA: 83136829020.16.96A3DE3 Received: from mail-qt1-f176.google.com (mail-qt1-f176.google.com [209.85.160.176]) by imf17.hostedemail.com (Postfix) with ESMTP id 5A8174000D for ; Wed, 19 Feb 2025 13:50:28 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=C3CtPqxl; spf=pass (imf17.hostedemail.com: domain of jackmanb@google.com designates 209.85.160.176 as permitted sender) smtp.mailfrom=jackmanb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1739973028; a=rsa-sha256; cv=none; b=l5qD8lW4hSbLQ4SOiGDLkA6Vt31elAMbKSDUNCoz5eNBATAm39jbeiX8m9rU8VChddwLMw pDVBb0pf3TiJFGm08LXoZg/GEoy22TNUJoC2+pV/Rlftf7NEeSHsuBcJ59+xdh21fF0m2X BcuB5qHKZvKCRdkwAIoxAq8SjXaDLRY= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=C3CtPqxl; spf=pass (imf17.hostedemail.com: domain of jackmanb@google.com designates 209.85.160.176 as permitted sender) smtp.mailfrom=jackmanb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1739973028; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=cpRpVwT6nybSiQD8y0Vvf6uWA9zfVLaJVGGslB/AiT0=; b=6Ofk8w0Ozw6u5ytXjdRd8phGvqWBS289RAtDewlEX0ZrghWau/jimbJ/ef10Dz7UgQSaTr b5V78WM4ziEItyDssdU/kTvMMOMrJsWT0c1RzgURIBFRG64PZ4wT4FNgZgZdPIVjSgKa79 IfxA38Tk6BlgJx3pyfBLTUgvjc0lKlg= Received: by mail-qt1-f176.google.com with SMTP id d75a77b69052e-471fa3b19bcso605191cf.0 for ; Wed, 19 Feb 2025 05:50:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1739973027; x=1740577827; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=cpRpVwT6nybSiQD8y0Vvf6uWA9zfVLaJVGGslB/AiT0=; b=C3CtPqxlkvX74I9qU3/qnNfHuDpbFV45XXlEEZBQUQJwh9yUM5q2NEkTk+R+AiIdj6 IDHsxbJ91EIx9NzTlfvvIauSHFKZ0iy0cJmNIB6tb13cnxs50PzgGXSNt5wd11hjys8y f0uAwSRYSuPoFc88I9TL0vE+c0wl/BYE1ed/dVXJAoYqrr04hW+5hkNmeaTq3XWW+Utv KT4WPvENhYH6p+xLL3xJRQq7eKxWHYqLlscHH2u4/lvPcdzcbfJA6cAcjQN1I2b2djgE K88qnv9oZfWlXjQyspfhJHNFCAf0nqMSGfJ6aP4vjGH+ZOk1pqF7Fca6hybdibSO+Lji TkUA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739973027; x=1740577827; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=cpRpVwT6nybSiQD8y0Vvf6uWA9zfVLaJVGGslB/AiT0=; b=QcJSYBpNNe5TdxDGGbaj1+o1AtT2jhOKL0htNnpoDzUIUahYYxg6GjV2czGvJCi07u SGzQWVvqZ99Ci7NJfP2p71wRyhp9fVnIVq6Xjw34UUf94oaugWqj4BX6oXEwYaiGJAVx AmFMcCPZv70DKsTVz/5d5kOTrhsVnReIr6CXkXS/5ICcE33h0ZkbXQgpMqepdWaCMNo2 4R51MEymBV5Ndc5AfqvkvsEaI8yRup4atGZ1oSTuUyaLi84LUf0AXc+X2QwhNXtUsjZf TBzSEEMjmcZ7Li/JaimuXPT4BqujXvufZldylIBiaD4HQWQ2V4n822BSHJ4tGAIejcPn 128A== X-Forwarded-Encrypted: i=1; AJvYcCX9GWtLbEtNsa6ydDAQ2UWsCCR8NfcBtTrYWAKpNtJfbrdDwgDuKDQ5H2QeksvnFgTVIqOoxfAOXw==@kvack.org X-Gm-Message-State: AOJu0Yzpz4RYzn+VoMRzguKypkQBiCCQk2I7Cb6Wztmz/mZWvrWm6MRK MstZGCmiJNls1oS80BNr4k+Hw/No3P+4YNim20Am7RwbQODrFsud1wAPt6W5XGabj28FTPj2cDy 8GWzjd7cAYy5bH8WmobJXeqtmGiQ6XZL9tg1O X-Gm-Gg: ASbGncu+M4U5cYKTULqgJo3hQgcNcsksRm6WNzn0KAaPXzklClBb6IHej9DKA9G6Iz3 D+3eBBB3eWkAKrZh7KENPNJBCKYsWKyZKgITywBk/hRjubeMH8V+97SFUaz+RggxKlx0dolZrkk 4qLHPDgLkiEhnUEjjjqhpxr+EOFwI= X-Google-Smtp-Source: AGHT+IGxA0ST0+Qs2En53BsPu4c7HcNH6zxgVPUvf4A/emGOU6uwfm55gzfEizcmtTiC/0i6qUTM3HoPfZ3vHe3EBBI= X-Received: by 2002:a05:622a:1984:b0:471:812b:508 with SMTP id d75a77b69052e-4720965d0d5mr3550131cf.14.1739973026785; Wed, 19 Feb 2025 05:50:26 -0800 (PST) MIME-Version: 1.0 References: <20250110-asi-rfc-v2-v2-0-8419288bc805@google.com> <20250110-asi-rfc-v2-v2-3-8419288bc805@google.com> <20250219105503.GKZ7W4h6QW1CNj48U9@fat_crate.local> In-Reply-To: <20250219105503.GKZ7W4h6QW1CNj48U9@fat_crate.local> From: Brendan Jackman Date: Wed, 19 Feb 2025 14:50:15 +0100 X-Gm-Features: AWEUYZkwkfdbftDgaNmZfIEB1SbLThhdC5JS-Qi6ViHsp1K5UHegLhk89BGfq9g Message-ID: Subject: Re: [PATCH RFC v2 03/29] mm: asi: Introduce ASI core API To: Borislav Petkov Cc: Thomas Gleixner , Ingo Molnar , Dave Hansen , "H. Peter Anvin" , Andy Lutomirski , Peter Zijlstra , Richard Henderson , Matt Turner , Vineet Gupta , Russell King , Catalin Marinas , Will Deacon , Guo Ren , Brian Cain , Huacai Chen , WANG Xuerui , Geert Uytterhoeven , Michal Simek , Thomas Bogendoerfer , Dinh Nguyen , Jonas Bonn , Stefan Kristiansson , Stafford Horne , "James E.J. Bottomley" , Helge Deller , Michael Ellerman , Nicholas Piggin , Christophe Leroy , Naveen N Rao , Madhavan Srinivasan , Paul Walmsley , Palmer Dabbelt , Albert Ou , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Christian Borntraeger , Sven Schnelle , Yoshinori Sato , Rich Felker , John Paul Adrian Glaubitz , "David S. Miller" , Andreas Larsson , Richard Weinberger , Anton Ivanov , Johannes Berg , Chris Zankel , Max Filippov , Arnd Bergmann , Andrew Morton , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Valentin Schneider , Uladzislau Rezki , Christoph Hellwig , Masami Hiramatsu , Mathieu Desnoyers , Mike Rapoport , Arnaldo Carvalho de Melo , Namhyung Kim , Mark Rutland , Alexander Shishkin , Jiri Olsa , Ian Rogers , Adrian Hunter , Dennis Zhou , Tejun Heo , Christoph Lameter , Sean Christopherson , Paolo Bonzini , Ard Biesheuvel , Josh Poimboeuf , Pawan Gupta , x86@kernel.org, linux-kernel@vger.kernel.org, linux-alpha@vger.kernel.org, linux-snps-arc@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-csky@vger.kernel.org, linux-hexagon@vger.kernel.org, loongarch@lists.linux.dev, linux-m68k@lists.linux-m68k.org, linux-mips@vger.kernel.org, linux-openrisc@vger.kernel.org, linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, linux-sh@vger.kernel.org, sparclinux@vger.kernel.org, linux-um@lists.infradead.org, linux-arch@vger.kernel.org, linux-mm@kvack.org, linux-trace-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, kvm@vger.kernel.org, linux-efi@vger.kernel.org, Ofir Weisse , Junaid Shahid Content-Type: multipart/alternative; boundary="0000000000002ec785062e7f0a0e" X-Rspamd-Queue-Id: 5A8174000D X-Stat-Signature: ik9ndqttrmnb3ctaoppyp91na64a6ufs X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1739973028-362712 X-HE-Meta: U2FsdGVkX1/j+/uVlWmAuEGxFIHsZoDDPpZsZKwNU18y/VXUbU5WCrrg4eYOBam23ihBRAt30t3qK9iU9hOtEoOl8kAQzQfdWXS18xh4kuE4DrXb97DfPdxAzSz1leBgp1yywQu//mOi2O6OhdH7TA8IQAyY00EpSGRnWQHk4tyokwniJcqTaNOXnFuA59B5GnkaOIcCT6o/cqPhrDmW8CabbHW60rt0wvHZyXYd0mFbgRiRoBBkKmJIp8HJpYotDJkB1lI7utNPtuahxK6U2UNjglGZf6sA43JWFwkd+KQbZzW1QpwO3C0sIsaP9f7bME/QVL6YTNJwO7mra+gRp9KG36Tk1h1DVN1hEBw5YcDrzJuxes0l5ahgIeFidqEtYlmZLi5iTwQTsXYll0QnakKM51AKBv+eDPh5LRlntd237vRfz4JOcntPOxIvxB1R8FqbCU++HaBSHd7CQOXOPLpkJC9nCKtjRqzWZF2IHEqr6XWOzbMxH75tC8aJPtOUzsG9+HiAdg0cCl1qAx40m8UgMrA+ieS6z8Q6EGUmmopgaBqd+74Dysn+vKQahI73MT8H+GO35BwA9Sz9/kFrBqwGPcQSXLJueGGiszOsWJ3LAY2T1l5I6p6DvKKhVKWfHHKwihxVkDuG70hMho3SDWXJxFNMyFbrcvdygQ8dIELwMDcK/UlmRMHNLBFdzCKvxT6TakUkV13nHMQy93NNGML101cyW6cDVkJKy89YXySvk8B4NZTp+EYj2pHw7FLnfa1PAPbiK6RtpvO2GZdhhDdlLyFBQsCHIIYMCF6w5zDNT7JZASrUo4rRFCZZfhuUX6WmA9hQB/EaMoPu+YVYCuV4QU4w/SHVi9ofC3x2kiEo4gLJ99maDIE2ux4ymp2QDFNNV1K9r0ha11uD3yHbPQeHc/HW4zOWQnAvVoK/fLaTrb5qGMPuI8uwxKegLLpmxUKBy6q5QuBb/PS7H6x kPUm33I2 aTr+JAdmfbw148+ApcsjJPR2aQbVu4+xQLouyd3SRv9YVQ65Gogf27eDY7dCgqR4ydLk9hhU89MAhpzirF3LfR0aFKmzbMGE0oLdCdFaOuRLYI95XcnYD5bfimYenXqLdLcvxQm0ynpb9lvAE1nCp3MlJC6GXS01W/aJfJMyGeI8MG/Pmr2OZl3EHfn315Fdzs/E6BEcW9k/Q0XbJnqwylwmP4sKd9hnyelIA+MJg7nmvNSAnExFyHrq0m6LNgnFIcWtOXvBhgqqMYFl6fYk9aB5xqBHEkw+ZgwioHdRUWxw4hJ4= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: --0000000000002ec785062e7f0a0e Content-Type: text/plain; charset="UTF-8" On Wed, 19 Feb 2025 at 11:57, Borislav Petkov wrote: > > + * Runtime usage: > > + * > > + * 1. Call asi_enter() to switch to the restricted address space. This > can't be > > + * from an interrupt or exception handler and preemption must be > disabled. > > + * > > + * 2. Execute untrusted code. > > + * > > + * 3. Call asi_relax() to inform the ASI subsystem that untrusted code > execution > > + * is finished. This doesn't cause any address space change. This > can't be > > + * from an interrupt or exception handler and preemption must be > disabled. > > + * > > + * 4. Either: > > + * > > + * a. Go back to 1. > > + * > > + * b. Call asi_exit() before returning to userspace. This immediately > > + * switches to the unrestricted address space. > > So only from reading this, it does sound weird. Maybe the code does it > differently - I'll see soon - but this basically says: > > I asi_enter(), do something, asi_relax() and then I decide to do something > more and to asi_enter() again!? And then I can end it all with a *single* > asi_exit() call? > > Hm, definitely weird API. Why? > OK, sounds like I need to rewrite this explanation! It's only been read before by people who already knew how this thing worked so this might take a few attempts to make it clear. Maybe the best way to make it clear is to explain this with reference to KVM. At a super high level, That looks like: ioctl(KVM_RUN) { enter_from_user_mode() while !need_userspace_handling() { asi_enter(); // part 1 vmenter(); // part 2 asi_relax(); // part 3 } asi _exit(); // part 4b exit_to_user_mode() } So part 4a is just referring to continuation of the loop. This explanation was written when that was the only user of this API so it was probably clearer, now we have userspace it seems a bit odd. With my pseudocode above, does it make more sense? If so I'll try to think of a better way to explain it. /* > * Leave the "tense" state if we are in it, i.e. end the critical section. > We > * will stay relaxed until the next asi_enter. > */ > void asi_relax(void); > > Yeah, so there's no API functions balance between enter() and relax()... > asi_enter() is actually balanced with asi_relax(). The comment says "if we are in it" because technically if you call this asi_relax() outside of the critical section, it's a nop. But, there's no reason to do that, so we could definitely change the comment and WARN if that happens. > > +#define ASI_TAINT_OTHER_MM_CONTROL ((asi_taints_t)BIT(6)) > > +#define ASI_NUM_TAINTS 6 > > +static_assert(BITS_PER_BYTE * sizeof(asi_taints_t) >= ASI_NUM_TAINTS); > > Why is this a typedef at all to make the code more unreadable than it > needs to > be? Why not a simple unsigned int or char or whatever you need? > My thinking was just that it's nicer to see asi_taints_t and know that it means "it holds taint flags and it's big enough" instead of having to remember the space needed for these flags. But yeah I'm fine with making it a raw integer type. > +#define ASI_TAINTS_CONTROL_MASK \ > > + (ASI_TAINT_USER_CONTROL | ASI_TAINT_GUEST_CONTROL | > ASI_TAINT_OTHER_MM_CONTROL) > > + > > +#define ASI_TAINTS_DATA_MASK \ > > + (ASI_TAINT_KERNEL_DATA | ASI_TAINT_USER_DATA | > ASI_TAINT_OTHER_MM_DATA) > > + > > +#define ASI_TAINTS_GUEST_MASK (ASI_TAINT_GUEST_DATA | > ASI_TAINT_GUEST_CONTROL) > > +#define ASI_TAINTS_USER_MASK (ASI_TAINT_USER_DATA | > ASI_TAINT_USER_CONTROL) > > + > > +/* The taint policy tells ASI how a class interacts with the CPU taints > */ > > +struct asi_taint_policy { > > + /* > > + * What taints would necessitate a flush when entering the domain, > to > > + * protect it from attack by prior domains? > > + */ > > + asi_taints_t prevent_control; > > So if those necessitate a flush, why isn't this var called > "taints_to_flush" > or whatever which exactly explains what it is? > Well it needs to be disambiguated from the field below (currently protect_data) but it could be control_to_flush (and data_to_flush). The downside of that is that having one say "prevent" and one say "protect" is quite meaningful. prevent_control is describing things we need to do to protect the system from this domain, protect_data is about protecting the domain from the system. However, while that difference is meaningful it might not actually be helpful for the reader of the code so I'm not wed to it. Also worth noting that we could just combine these fields. At present they should have disjoint bits set. But, they're used in separate contexts and have separate (although conceptually very similar) meanings, so I think that would reduce clarity. > Spellchecker please. Go over your whole set. > Ack, I've set up a local thingy to spellcheck all my commits so hopefully you should encounter less of that noise in future. For the pronouns stuff I will do my best but you might still spot violations in older text, sorry about that. > + /* What taints should be set when entering the domain? */ > > + asi_taints_t set; > > > So "required_taints" or so... hm? > What this field is describing is: when we run the untrusted code, what happens? I don't mean "what does the kernel do" but what physically happens on the CPU from an exploit point of view. For example setting ASI_TAINT_USER_DATA in this field means "when we run the untrusted code (i.e. userspace), userspace data gets left behind in sidechannels". "Should be set" in the comment means "this field should be set to record that a thing has happened" not "this field being set is a requirement for some API" or something. So I don't think "required" is right but this is hard to name. That commentary should also be expanded I think, since "should be set" is pretty ambiguous. And maybe if we called it "to_set" it would be more obvious that "set" is a verb? I'm very open to suggestions. > > + > > +void asi_init_mm_state(struct mm_struct *mm); > > + > > +int asi_init_class(enum asi_class_id class_id, struct asi_taint_policy > *taint_policy); > > +void asi_uninit_class(enum asi_class_id class_id); > > "uninit", meh. "exit" perhaps? or "destroy" > And you have "asi_destroy" already so I guess you can do: > > asi_class_init() > asi_class_destroy() > > to have the namespace correct. > Yeah, not sure what I was thinking here! > > +static __always_inline bool asi_is_tense(void) > > +{ > > + return !asi_is_relaxed(); > > +} > > So can we tone down the silly helpers above? You don't really need > asi_is_tense() for example. It is still very intuitive if I read > > if (!asi_is_relaxed()) > > Yep that sounds good. --0000000000002ec785062e7f0a0e Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Wed, 19 Feb 2025 at 11:57, Borislav Pe= tkov <bp@alien8.de> wrote:
<= div class=3D"gmail_quote gmail_quote_container">
> + * Runtime usage:
> + *
> + * 1. Call asi_enter() to switch to the restricted address space. Thi= s can't be
> + *=C2=A0 =C2=A0 from an interrupt or exception handler and preemption= must be disabled.
> + *
> + * 2. Execute untrusted code.
> + *
> + * 3. Call asi_relax() to inform the ASI subsystem that untrusted cod= e execution
> + *=C2=A0 =C2=A0 is finished. This doesn't cause any address space= change. This can't be
> + *=C2=A0 =C2=A0 from an interrupt or exception handler and preemption= must be disabled.
> + *
> + * 4. Either:
> + *
> + *=C2=A0 =C2=A0 a. Go back to 1.
> + *
> + *=C2=A0 =C2=A0 b. Call asi_exit() before returning to userspace. Thi= s immediately
> + *=C2=A0 =C2=A0 =C2=A0 =C2=A0switches to the unrestricted address spa= ce.

So only from reading this, it does sound weird. Maybe the code does it
differently - I'll see soon - but this basically says:

I asi_enter(), do something, asi_relax() and then I decide to do something<= br> more and to asi_enter() again!? And then I can end it all with a *single* asi_exit() call?

Hm, definitely weird API. Why?

OK, soun= ds like I need to rewrite this explanation! It's only been read before = by people who already knew how this thing worked so this might take a few a= ttempts to make it clear.

Maybe the best way to make it clear is to = explain this with reference to KVM. At a super high level, That looks like:=

ioctl(KVM_RUN) {
=C2=A0 =C2=A0 enter_from_user_mode()
=C2=A0 = =C2=A0 while !need_userspace_handling() {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 as= i_enter();=C2=A0 // part 1
=C2=A0 =C2=A0 =C2=A0 =C2=A0 vmenter();=C2=A0 = // part 2
=C2=A0 =C2=A0 =C2=A0 =C2=A0 asi_relax(); // part 3
=C2=A0 = =C2=A0 }
=C2=A0 =C2=A0 asi _exit(); // part 4b
=C2=A0 =C2=A0 exit_to_= user_mode()
}

So part 4a is just referring to continuation= of the loop.

This explanation was written when that was the only us= er of this API so it was probably clearer, now we have userspace it seems a= bit odd.

With my pseudocode above, does it make more sense? If so I= 'll try to think of a better way to explain it.

/*
=C2=A0* Leave the "tense" state if we are in it, i.e. end the cri= tical section. We
=C2=A0* will stay relaxed until the next asi_enter.
=C2=A0*/
void asi_relax(void);

Yeah, so there's no API functions balance between enter() and relax()..= .

asi_enter() is actually balanced with= asi_relax(). The comment says "if we are in it" because technica= lly if you call this asi_relax() outside of the critical section, it's = a nop. But, there's no reason to do that, so we could definitely change= the comment and WARN if that happens.
=C2=A0
> +#define ASI_TAINT_OTHER_MM_CONTROL=C2=A0 =C2=A0((asi_taints_t)BIT(6))=
> +#define ASI_NUM_TAINTS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A06
> +static_assert(BITS_PER_BYTE * sizeof(asi_taints_t) >=3D ASI_NUM_TA= INTS);

Why is this a typedef at all to make the code more unreadable than it needs= to
be? Why not a simple unsigned int or char or whatever you need?

My thinking was just that it's nicer to see as= i_taints_t and know that it means "it holds taint flags and it's b= ig enough" instead of having to remember the space needed for these fl= ags. But yeah I'm fine with making it a raw integer type.
> +#define ASI_TAINTS_CONTROL_MASK \
> +=C2=A0 =C2=A0 =C2=A0(ASI_TAINT_USER_CONTROL | ASI_TAINT_GUEST_CONTROL= | ASI_TAINT_OTHER_MM_CONTROL)
> +
> +#define ASI_TAINTS_DATA_MASK \
> +=C2=A0 =C2=A0 =C2=A0(ASI_TAINT_KERNEL_DATA | ASI_TAINT_USER_DATA | AS= I_TAINT_OTHER_MM_DATA)
> +
> +#define ASI_TAINTS_GUEST_MASK (ASI_TAINT_GUEST_DATA | ASI_TAINT_GUEST= _CONTROL)
> +#define ASI_TAINTS_USER_MASK (ASI_TAINT_USER_DATA | ASI_TAINT_USER_CO= NTROL)
> +
> +/* The taint policy tells ASI how a class interacts with the CPU tain= ts */
> +struct asi_taint_policy {
> +=C2=A0 =C2=A0 =C2=A0/*
> +=C2=A0 =C2=A0 =C2=A0 * What taints would necessitate a flush when ent= ering the domain, to
> +=C2=A0 =C2=A0 =C2=A0 * protect it from attack by prior domains?
> +=C2=A0 =C2=A0 =C2=A0 */
> +=C2=A0 =C2=A0 =C2=A0asi_taints_t prevent_control;

So if those necessitate a flush, why isn't this var called "taints= _to_flush"
or whatever which exactly explains what it is?

Wel= l it needs to be disambiguated from the field below (currently protect_data= ) but it could be control_to_flush (and data_to_flush).

The downside= of that is that having one say "prevent" and one say "prote= ct" is quite meaningful. prevent_control is describing things we need = to do to protect the system from this domain, protect_data is about protect= ing the domain from the system. However, while that difference is meaningfu= l it might not actually be helpful for the reader of the code so I'm no= t wed to it.

Also worth noting that we could just combine these fiel= ds. At present they should have disjoint bits set. But, they're used in= separate contexts and have separate (although conceptually very similar) m= eanings, so I think that would reduce clarity.
=C2=A0
Spellchecker please. Go over your whole set.

Ack, = I've set up a local thingy to spellcheck all my commits so hopefully yo= u should encounter less of that noise in future.

For the pronouns st= uff I will do my best but you might still spot violations in older text, so= rry about that.

> +=C2=A0 =C2=A0 =C2=A0/* What taints should be set when entering the do= main? */
> +=C2=A0 =C2=A0 =C2=A0asi_taints_t set;


So "required_taints" or so... hm?

=
What this field is describing is: when we run the untrusted code, what= happens? I don't mean "what does the kernel do" but what phy= sically happens on the CPU from an exploit point of view.=C2=A0

For = example setting ASI_TAINT_USER_DATA in this field means "when we run t= he untrusted code (i.e. userspace), userspace data gets left behind in side= channels".

"Should be set" in the comment means "= ;this field should be set to record that a thing has happened" not &qu= ot;this field being set is a requirement for some API" or something. S= o I don't think "required" is right but this is hard to name.=

That commentary should also be expanded I think, since "should= be set" is pretty ambiguous. And maybe if we called it "to_set&q= uot; it would be more obvious that "set" is a verb? I'm very = open to suggestions.
=C2=A0
> +
> +void asi_init_mm_state(struct mm_struct *mm);
> +
> +int asi_init_class(enum asi_class_id class_id, struct asi_taint_polic= y *taint_policy);
> +void asi_uninit_class(enum asi_class_id class_id);

"uninit", meh. "exit" perhaps? or "destroy"
=C2=A0
And you have "asi_destroy" already so I guess you can do:

asi_class_init()
asi_class_destroy()

to have the namespace correct.

Yeah, not sur= e what I was thinking here!
=C2=A0
> +static __always_inline bool asi_is_tense(void)
> +{
> +=C2=A0 =C2=A0 =C2=A0return !asi_is_relaxed();
> +}

So can we tone down the silly helpers above? You don't really need
asi_is_tense() for example. It is still very intuitive if I read

=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!asi_is_relaxed())

Yep that sounds good.=C2=A0

--0000000000002ec785062e7f0a0e--