Re: [PATCH v2 5/7] rust: file: add `Kuid` wrapper

[Peter Zijlstra <peterz@infradead.org>]

On Wed, Dec 06, 2023 at 01:57:52PM +0100, Alice Ryhl wrote:
> On Wed, Dec 6, 2023 at 1:34 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Wed, Dec 06, 2023 at 11:59:50AM +0000, Alice Ryhl wrote:
> >
> > > diff --git a/rust/helpers.c b/rust/helpers.c
> > > index fd633d9db79a..58e3a9dff349 100644
> > > --- a/rust/helpers.c
> > > +++ b/rust/helpers.c
> > > @@ -142,6 +142,51 @@ void rust_helper_put_task_struct(struct task_struct *t)
> > >  }
> > >  EXPORT_SYMBOL_GPL(rust_helper_put_task_struct);
> > >
> > > +kuid_t rust_helper_task_uid(struct task_struct *task)
> > > +{
> > > +     return task_uid(task);
> > > +}
> > > +EXPORT_SYMBOL_GPL(rust_helper_task_uid);
> > > +
> > > +kuid_t rust_helper_task_euid(struct task_struct *task)
> > > +{
> > > +     return task_euid(task);
> > > +}
> > > +EXPORT_SYMBOL_GPL(rust_helper_task_euid);
> >
> > So I still object to these on the ground that they're obvious and
> > trivial speculation gadgets.
> >
> > We should not have (exported) functions that are basically a single
> > dereference of a pointer argument.
> >
> > And I do not appreciate my feedback on the previous round being ignored.
> 
> I'm sorry about that. I barely know what speculation gadgets are, so I
> didn't really know what to respond. But I should have responded by
> saying that.

Sigh... how many years since Meltdown are we now? Oh well, the basic
idea is to abuse the basic speculative execution of the CPU to make it
disclose secrets. The way this is done is by training the various branch
predictors such that you gain control over the speculative control flow.

You then use this control to cause micro-architectural side-effects to
observe state, eg. cache state.

In the case above, if you train the indirect branch predictor to jump to
the above functions after you've loaded a secret into %rdi (first
argument) then it will dereference your pointer + offset.

This causes the CPU to populate the cacheline, even if the actual
execution path never does this.

So by wiping the cache, doing your tricky thing, and then probing the
cache to find out which line is present, you can infer the secret value.

Anywhoo, the longer a function is, the harder it becomes, since you need
to deal with everything a function does and consider the specuation
window length. So trivial functions like the above that do an immediate
dereference and are (and must be) a valid indirect target (because
EXPORT) are ideal.

> I can reimplement these specific functions as inline Rust functions,

That would be good, but how are you going to do that without duplicating
the horror that is struct task_struct ?

> but I don't think I can give you a general solution to the
> rust_helper_* problem in this patch series.

Well, I really wish the Rust community would address the C
interoperability in a hurry. Basically make it a requirement for
in-kernel Rust.

I mean, how hard can it be to have clang parse the C headers and inject
them into the Rust IR as if they're external FFI things.