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.
v/r, dps
