| Message ID | 20190510173049.28171-1-richard.henderson@linaro.org |
|---|---|
| Headers | show |
| Series | Add qemu_getrandom and ARMv8.5-RNG etc | expand |
"make check-unit" fails for me: TEST check-unit: tests/test-crypto-tlscredsx509 Unexpected error in object_new_with_propv() at /work/armbru/qemu/qom/object.c:674: invalid object type: tls-creds-x509 and TEST check-unit: tests/test-io-channel-tls Unexpected error in object_new_with_propv() at /work/armbru/qemu/qom/object.c:674: invalid object type: tls-creds-x509 I haven't looked further.
On Tue, May 14, 2019 at 05:15:31PM +0200, Markus Armbruster wrote: > "make check-unit" fails for me: > > TEST check-unit: tests/test-crypto-tlscredsx509 > Unexpected error in object_new_with_propv() at /work/armbru/qemu/qom/object.c:674: > invalid object type: tls-creds-x509 > > and > > TEST check-unit: tests/test-io-channel-tls > Unexpected error in object_new_with_propv() at /work/armbru/qemu/qom/object.c:674: > invalid object type: tls-creds-x509 > > I haven't looked further. I have a nasty feeling it is caused by Subject: [PATCH v6 02/25] crypto: Merge crypto-obj-y into libqemuutil.a The QOM objects are not directly used by most of the code. We rely on the constructor registering the QOM object and then we request an instance of it via the type name. So there's no direct function calls from any code into the crypto object impls. When we put the crypto objects into libqemuutil.a the linker is not intelligent enough to see the constructor and so thinks all these QOM object impls are unused and discards them when linking the final binary. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 5/14/19 8:23 AM, Daniel P. Berrangé wrote: > On Tue, May 14, 2019 at 05:15:31PM +0200, Markus Armbruster wrote: >> "make check-unit" fails for me: >> >> TEST check-unit: tests/test-crypto-tlscredsx509 >> Unexpected error in object_new_with_propv() at /work/armbru/qemu/qom/object.c:674: >> invalid object type: tls-creds-x509 >> >> and >> >> TEST check-unit: tests/test-io-channel-tls >> Unexpected error in object_new_with_propv() at /work/armbru/qemu/qom/object.c:674: >> invalid object type: tls-creds-x509 >> >> I haven't looked further. > > I have a nasty feeling it is caused by > > Subject: [PATCH v6 02/25] crypto: Merge crypto-obj-y into libqemuutil.a > > The QOM objects are not directly used by most of the code. We rely on > the constructor registering the QOM object and then we request an > instance of it via the type name. So there's no direct function calls > from any code into the crypto object impls. > > When we put the crypto objects into libqemuutil.a the linker is not > intelligent enough to see the constructor and so thinks all these > QOM object impls are unused and discards them when linking the final > binary. Yes, that would do it. We would need something in the test that forces the objects into the link. Without having yet looked at the test cases, any ideas? r~
On Tue, May 14, 2019 at 09:14:57AM -0700, Richard Henderson wrote: > On 5/14/19 8:23 AM, Daniel P. Berrangé wrote: > > On Tue, May 14, 2019 at 05:15:31PM +0200, Markus Armbruster wrote: > >> "make check-unit" fails for me: > >> > >> TEST check-unit: tests/test-crypto-tlscredsx509 > >> Unexpected error in object_new_with_propv() at /work/armbru/qemu/qom/object.c:674: > >> invalid object type: tls-creds-x509 > >> > >> and > >> > >> TEST check-unit: tests/test-io-channel-tls > >> Unexpected error in object_new_with_propv() at /work/armbru/qemu/qom/object.c:674: > >> invalid object type: tls-creds-x509 > >> > >> I haven't looked further. > > > > I have a nasty feeling it is caused by > > > > Subject: [PATCH v6 02/25] crypto: Merge crypto-obj-y into libqemuutil.a > > > > The QOM objects are not directly used by most of the code. We rely on > > the constructor registering the QOM object and then we request an > > instance of it via the type name. So there's no direct function calls > > from any code into the crypto object impls. > > > > When we put the crypto objects into libqemuutil.a the linker is not > > intelligent enough to see the constructor and so thinks all these > > QOM object impls are unused and discards them when linking the final > > binary. > > Yes, that would do it. We would need something in the test that forces the > objects into the link. Without having yet looked at the test cases, any ideas? I don't think this is only the test suite. I think it will affect all the binaries we build The only way you can force it is to use -Wl,--whole-archive arg to tell ld to include everything from libqemuutil.la, but that will break the way the stubs work, as we want make of the stubs to be discarded. The only other option is to not build $(crypto-obj-y) into libqemuutil.la, but list that variable explicitly everywhere that we list libqemuutil.la Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 5/14/19 9:50 AM, Daniel P. Berrangé wrote: > On Tue, May 14, 2019 at 09:14:57AM -0700, Richard Henderson wrote: >> Yes, that would do it. We would need something in the test that forces the >> objects into the link. Without having yet looked at the test cases, any ideas? > > I don't think this is only the test suite. I think it will affect all the > binaries we build You're right, it does. $ nm aarch64-softmmu/qemu-system-aarch64 \ | grep qcrypto_tls_creds_x509_register_types comes up empty. It didn't occur to me that there was nothing in the object files for the reference. I'll have to drop the crypto-obj-y patch and come up with a different solution. r~
On 5/14/19 12:46 PM, Richard Henderson wrote: > On 5/14/19 9:50 AM, Daniel P. Berrangé wrote: >> On Tue, May 14, 2019 at 09:14:57AM -0700, Richard Henderson wrote: >>> Yes, that would do it. We would need something in the test that forces the >>> objects into the link. Without having yet looked at the test cases, any ideas? >> >> I don't think this is only the test suite. I think it will affect all the >> binaries we build > > You're right, it does. > > $ nm aarch64-softmmu/qemu-system-aarch64 \ > | grep qcrypto_tls_creds_x509_register_types > > comes up empty. > > It didn't occur to me that there was nothing in the object files for the > reference. I'll have to drop the crypto-obj-y patch and come up with a > different solution. Isn't there a gcc annotation for marking a simple as mandatorily included during link? /me goes looking... __attribute__((externally_visible)) sounds promising (it nullifies the effects of -fwhole-program, so that a function remains visible even if the linker would have otherwise suppressed it) __attribute__((used)) also sounds useful (the function must be emitted even if it does not appear to be referenced, which may be enough for the linker to infer that it is used) There may be other tricks, although I didn't go searching very hard. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On 5/14/19 2:43 PM, Eric Blake wrote: >> It didn't occur to me that there was nothing in the object files for the >> reference. I'll have to drop the crypto-obj-y patch and come up with a >> different solution. > > Isn't there a gcc annotation for marking a simple as mandatorily > included during link? No. There's stuff you can mark a single function within an object file that you can use to avoid the function being elided... > __attribute__((externally_visible)) sounds promising (it nullifies the > effects of -fwhole-program, so that a function remains visible even if > the linker would have otherwise suppressed it) > > __attribute__((used)) also sounds useful (the function must be emitted > even if it does not appear to be referenced, which may be enough for the > linker to infer that it is used) ... and you found those. But those do not affect the linker's behaviour with .a files at all. You can force a symbol reference from the ld command-line: -u sym, which can cause the .o containing sym to be included from the .a file. But that doesn't work if there's no global symbol in the .o to reference. You can force all .o from a .a file to be included, with --whole-archive. That is useful when you're using .a files a shorthand for lots and lots of .o files. But in our case that would break the use of stubs. Anyway, see v7 now. r~
Only the new patch 24 lacks review. Changes since v5: * Merge crypto-obj-y into util-obj-y (patch 2). * Fix leftover crypto-obj-aes-y reference (patch 2). * Add ARM_CP_IO to the RNG registers (patch 22). * Issue gen_io_start/end around ppc DARN (new patch 24). * Issue gen_io_start/end around x86 rdrand (patch 25). Changes since v4: * Do not autoenable nettle or gcrypt if linking is broken. Fixes --static on fedora 30. * Delay removal of srand() for -seed. * Do not loop for -1 result for ppc64 DARN. Changes since v3: * Do not autoenable gnutls if linking is broken. Fixes --static on ubuntu 18.04. Changes since v2: * Changes from review. - getrandom is not exclusive of /dev/urandom fallback. - vnc fails gracefully on crypto failure. - a great renaming. * Drop the "nonblock" argument, as it's not deliverable from the backend. * Propagate Error back through qemu_guest_getrandom. * Add qemu_guest_getrandom_nofail to centralize "Argh! Death!". * Convert hw/misc/ * Implement ppc darn. * Implement x86 rdrand. Changes since v1: * Build crypto-obj-y for linux-user as well. * Several patches to tidy crypto/random-platform.c. * Use getrandom(2) in crypto/random-platform.c. * Use qcrypto_random_bytes in ui/vnc.c. * In qemu_getrandom: - Use g_rand_int instead of srand48. - Use qcrypto_random_bytes instead of getrandom directly. r~ Richard Henderson (25): configure: Link test before auto-enabling crypto libraries crypto: Merge crypto-obj-y into libqemuutil.a crypto: Reverse code blocks in random-platform.c crypto: Do not fail for EINTR during qcrypto_random_bytes crypto: Use O_CLOEXEC in qcrypto_random_init crypto: Use getrandom for qcrypto_random_bytes crypto: Change the qcrypto_random_bytes buffer type to void* ui/vnc: Split out authentication_failed ui/vnc: Use gcrypto_random_bytes for start_auth_vnc util: Add qemu_guest_getrandom and associated routines cpus: Initialize pseudo-random seeds for all guest cpus linux-user: Initialize pseudo-random seeds for all guest cpus linux-user: Call qcrypto_init if not using -seed linux-user: Use qemu_guest_getrandom_nofail for AT_RANDOM linux-user/aarch64: Use qemu_guest_getrandom for PAUTH keys linux-user: Remove srand call aspeed/scu: Use qemu_guest_getrandom_nofail hw/misc/nrf51_rng: Use qemu_guest_getrandom_nofail hw/misc/bcm2835_rng: Use qemu_guest_getrandom_nofail hw/misc/exynos4210_rng: Use qemu_guest_getrandom target/arm: Put all PAC keys into a structure target/arm: Implement ARMv8.5-RNG target/ppc: Use qemu_guest_getrandom for DARN target/ppc: Use gen_io_start/end around DARN target/i386: Implement CPUID_EXT_RDRAND Makefile | 14 ++-- Makefile.objs | 8 +-- Makefile.target | 4 -- include/crypto/random.h | 2 +- include/qemu/guest-random.h | 68 ++++++++++++++++++ include/qom/cpu.h | 1 + linux-user/aarch64/target_syscall.h | 2 - target/arm/cpu.h | 17 +++-- target/i386/helper.h | 2 + cpus.c | 9 +++ crypto/random-gcrypt.c | 2 +- crypto/random-gnutls.c | 2 +- crypto/random-platform.c | 104 +++++++++++++++++----------- hw/misc/aspeed_scu.c | 10 +-- hw/misc/bcm2835_rng.c | 32 ++++----- hw/misc/exynos4210_rng.c | 11 ++- hw/misc/nrf51_rng.c | 4 +- linux-user/aarch64/cpu_loop.c | 25 +------ linux-user/elfload.c | 8 +-- linux-user/main.c | 34 +++++---- linux-user/syscall.c | 34 +++++++-- target/arm/cpu64.c | 1 + target/arm/helper.c | 64 ++++++++++++++--- target/arm/pauth_helper.c | 18 ++--- target/i386/cpu.c | 5 +- target/i386/int_helper.c | 21 ++++++ target/i386/translate.c | 62 +++++++++++++---- target/ppc/int_helper.c | 39 +++++++---- target/ppc/translate.c | 21 ++++-- ui/vnc.c | 53 ++++++-------- util/guest-random.c | 93 +++++++++++++++++++++++++ vl.c | 4 ++ configure | 87 +++++++++++++++-------- crypto/Makefile.objs | 77 ++++++++++---------- qemu-options.hx | 10 +++ tests/Makefile.include | 3 +- util/Makefile.objs | 1 + 37 files changed, 644 insertions(+), 308 deletions(-) create mode 100644 include/qemu/guest-random.h create mode 100644 util/guest-random.c -- 2.17.1