Re: [PATCH] subdom: Fix -Werror=address failure in tmp_emulator

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.08.2023 22: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.

Looking at the code, I'm not sure about this. There could as well have been
the intention to allow pointers there, then permitting them to be left at
NULL. Who knows where that code came from originally.

> Adjust it to compile.  No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Juergen Gross <jgross@xxxxxxxx>
> CC: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> CC: Jason Andryuk <jandryuk@xxxxxxxxx>
> CC: Daniel Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> CC: Christopher Clark <christopher.w.clark@xxxxxxxxx>
> 
> 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().
> 
> vtpm-stubdom isn't credibly component of a Xen system, and we're wasting loads
> of CI cycles testing it...

Question is whether people might be using it, and I'm afraid that's a
question we can't answer. Would it be an alternative to disable vtpm in
this container's stubdom configure invocation? Obviously as other
containers have their compilers updated, the same issue may surface
there then ...

Jan