On 8/14/23 07:25, George Dunlap wrote:
On Tue, Aug 8, 2023 at 10:27 PM Daniel P. Smith
<dpsm...@apertussolutions.com <mailto: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
<mailto:andrew.coop...@citrix.com>>
> ---
> CC: George Dunlap <george.dun...@eu.citrix.com
<mailto:george.dun...@eu.citrix.com>>
> CC: Jan Beulich <jbeul...@suse.com <mailto:jbeul...@suse.com>>
> CC: Stefano Stabellini <sstabell...@kernel.org
<mailto:sstabell...@kernel.org>>
> CC: Wei Liu <w...@xen.org <mailto:w...@xen.org>>
> CC: Julien Grall <jul...@xen.org <mailto:jul...@xen.org>>
> CC: Juergen Gross <jgr...@suse.com <mailto:jgr...@suse.com>>
> CC: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com
<mailto:marma...@invisiblethingslab.com>>
> CC: Jason Andryuk <jandr...@gmail.com <mailto:jandr...@gmail.com>>
> CC: Daniel Smith <dpsm...@apertussolutions.com
<mailto:dpsm...@apertussolutions.com>>
> CC: Christopher Clark <christopher.w.cl...@gmail.com
<mailto: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
<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
<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.
The architecture/design is what I don't want to see lost, the
implementation itself, IMHO, has severely bit rotted. So what I was
trying to say is that while it is an important reference design, I
cannot disagree with the sentiment that building the broken
implementation is wasting a significant amount of CI resources, both
network and CPU time. IOW, I am probably the only one that would
potentially make any noise if a patch was submitted to make it default
disabled, and I am saying here that I would not do so.
v/r,
dps