On Tue, Aug 8, 2023 at 10:27 PM Daniel P. Smith <
dpsm...@apertussolutions.com> wrote:
> On 8/3/23 16:36, Andrew Cooper wrote:
> > The opensuse-tumbleweed build jobs currently fail with:
> >
> > /builds/xen-project/xen/stubdom/tpm_emulator-x86_64/crypto/rsa.c: In
> function 'rsa_private':
> >
> /builds/xen-project/xen/stubdom/tpm_emulator-x86_64/crypto/rsa.c:56:7:
> error: the comparison will always evaluate as 'true' for the address of 'p'
> will never be NULL [-Werror=address]
> > 56 | if (!key->p || !key->q || !key->u) {
> > | ^
> > In file included from
> /builds/xen-project/xen/stubdom/tpm_emulator-x86_64/crypto/rsa.c:17:
> >
> /builds/xen-project/xen/stubdom/tpm_emulator-x86_64/crypto/rsa.h:28:12:
> note: 'p' declared here
> > 28 | tpm_bn_t p;
> > | ^
> >
> > This is because all tpm_bn_t's are 1-element arrays (of either a GMP or
> > OpenSSL BIGNUM flavour). The author was probably meaning to do value
> checks,
> > but that's not what the code does.
> >
> > Adjust it to compile. No functional change.
> >
> > Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
> > ---
> > CC: George Dunlap <george.dun...@eu.citrix.com>
> > CC: Jan Beulich <jbeul...@suse.com>
> > CC: Stefano Stabellini <sstabell...@kernel.org>
> > CC: Wei Liu <w...@xen.org>
> > CC: Julien Grall <jul...@xen.org>
> > CC: Juergen Gross <jgr...@suse.com>
> > CC: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
> > CC: Jason Andryuk <jandr...@gmail.com>
> > CC: Daniel Smith <dpsm...@apertussolutions.com>
> > CC: Christopher Clark <christopher.w.cl...@gmail.com>
> >
> > While I've confirmed this to fix the build issue:
> >
> >
> https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/955160430
> >
> > I'm -1 overall to the change, and would prefer to disable vtpm-stubdom
> > entirely.
> >
> > It's TPM 1.2 only, using decades-old libs, and some stuff in the upstream
> > https://github.com/PeterHuewe/tpm-emulator (which is still abandaonded
> as of
> > 2018) is just as concerning as the basic error here in rsa_private().
>
> For semantics sake, the Guest PV interface is 1.2 compliant but the PV
> backend, vtpmmgr, is capable of using TPM2.0.
>
> > vtpm-stubdom isn't credibly component of a Xen system, and we're wasting
> loads
> > of CI cycles testing it...
>
> Unfortunately, I cannot disagree here. This is the only proper vTPM,
> from a trustworthy architecture perspective, that I know of existing
> today. Until I can find someone willing to fund updating the
> implementation and moving it to being an emulated vTPM and not a PV
> interface, it is likely to stay in this state for some time.
>
Did you mean "I cannot *agree* here"? "Cannot disagree" means you agree
that we're "wasting loads of CI cycles testing it", but the second sentence
seems to imply that it's (currently) irreplacable.
-George
