Re: [PATCH v3 2/5] configfs-tsm: Introduce a shared ABI for attestation reports

[Dionna Amalie Glaze]

This is clean and approachable. Thanks for your implementation.

> +static int try_advance_write_generation(struct tsm_report *report)
> +{
> +       lockdep_assert_held_write(&tsm_rwsem);
> +
> +       /*
> +        * malicious or broken userspace is attempting to wrap read_generation,
> +        * stop accepting updates until current report configuration is read.
> +        */
> +       if (report->write_generation == report->read_generation - 1)
> +               return -EBUSY;
> +       report->write_generation++;
> +       return 0;
> +}
> +

This took me a while to wrap my head around.
The property we want is that when we read outblob, it is the result of
the attribute changes since the last read. If we write to an attribute
2^64 times, we could get write_generation to wrap around to equal
read_generation without actually reading outblob. So we could end up
given a badly cached result when we read outblob? Is that what this is
preventing?

I think I would word this to say,

"Malicious or broken userspace has written enough times for
read_generation == write_generation by modular arithmetic without an
interim read. Stop accepting updates until the current report
configuration is read."

I think that at least in the SEV-SNP case, we can double-check from
userspace that the report has values that we expected to configure the
get_report with, so this isn't really an issue. I'm not sure what the
use is of configuration that doesn't lead to observable (and
checkable) differences, but I suppose this check doesn't hurt.

-- 
-Dionna Glaze, PhD (she/her)