Ecc111 #1
Ecc111 #1
Conversation
The OPENSSL_s390xcap environment variable is used to set bits in the s390x capability vector to zero. This simplifies testing of different code paths. Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#6813) (cherry picked from commit f39ad8d)
Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#6813) (cherry picked from commit d68af00)
Added crypto/perlasm/s390x.pm Perl module. Its primary use is to be independent of binutils version, that is to write byte codes of instructions that are not part of the base instruction set. Currently only gas format is supported. Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#6919) (cherry picked from commit c66bb88)
Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#6919) (cherry picked from commit f760137)
Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#7991) (cherry picked from commit d6f4b0a)
Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#8257) (cherry picked from commit b2b580f)
featuring 6x"horizontal" code path which is up to 25% faster than present 4x"vertical" for larger blocks. Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#8287) (cherry picked from commit d122919)
>=20% faster than present code. Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#8560) (cherry picked from commit 2e6b615)
z14 introduced alignment hints to help vector load/store performance. For its predecessors, alignment hint defaults to 0 (no alignment indicated). Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#8181) (cherry picked from commit 11aad86)
Add non-base instructions which are used by the chacha20 and poly1305 modules. Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#8181) (cherry picked from commit 3062468)
Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#8181) (cherry picked from commit 302aa3c)
Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#8181) (cherry picked from commit 5ee08f4)
ISO C90 forbids specifying subobject to initialize Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from openssl#8693) (cherry picked from commit 61d7045)
Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Shane Lontis <shane.lontis@oracle.com> (Merged from openssl#9258) (cherry picked from commit e382f50)
[skip ci] Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#7829) (cherry picked from commit 4746f25)
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#8338) (cherry picked from commit 4564e77) Signed-off-by: Dimitri John Ledkov <xnox@ubuntu.com>
Add description of capability vector's pcc and kma parts. Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Shane Lontis <shane.lontis@oracle.com> (Merged from openssl#9258) (cherry picked from commit da93b5c) Signed-off-by: Dimitri John Ledkov <xnox@ubuntu.com>
Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#10004) (cherry picked from commit b3681e2) Signed-off-by: Dimitri John Ledkov <xnox@ubuntu.com>
Conflicts:
crypto/s390x_arch.h
crypto/s390xcpuid.pl
This cherrypicks from master of other s390x performance improvements,
that are already shipped in Ubuntu.
|
As far as i understand, this PR holds the same code as your openssl PR, except for history linearity. Would you agree to put the commits from your openssl PR (once review and testing is complete) to a new private branch of mine (say s390x111). Id maintain that one from then on, which means regularly rebasing it onto 1.1.1 stable and putting new s390x stuff on top ? |
Yes! |
|
About testing progress. Many packages in Ubuntu regress across all architectures with the two new patches that were introduced in the stable branch unrelated to our s390x changes. After bisecting from 1.1.1d tag up to all our patches, I finally identified that openssl@db943f4 openssl@22623e0 cause many regressions in perl ssl bindinds and many apps and daemons. I have reverted those for now, and reruning all autopkgtests again. I am expecting them to be all green once they complete, and then will be able to upload that branch, and you can take over the maintaince of rebasing that branch & cherrypicking more stuff to it. |
|
What about actually trying to get that branch merged into 1_1_1-stable? I spoke to Kurt during Fosdem, and in principle they used take hw acceleration backports into stable branches, as long as they are tested. Given that you are testing them across all of the generations of Z machines we should be able to get all of those backports into the stable tree, thus freeing us from ever increasing maintenance burden of rebasing those changes. Hence my upstream repo pull request was kind of a dual-intend to signal that we might want to actually get it merged there! Hence e.g. Kurt was reviewing it and questioning the backport of license changes. Especially since v3 is delayed again, and is no longer expected to be out until next year effectively (Q4 2020 mentioned on https://www.openssl.org/blog/blog/2019/11/07/3.0-update/) |
That would obviously be my preferred solution since i would only do back-ports based on a single upstream LTS branch instead of multiple distributions' openssl packages. I briefly addressed the topic every now and then, the last time on last year's openssl face2face, but the point of view has always been: only bugfixes are back-ported to stable. However, if this solution would also be preferred not just by ibm but also by our distribution partners, that would be a strong argument to get back into the discussion. I mean in the end I develop, test and maintain all of openssl's s390 stuff anyway and every problem will finally find me, no matter upstream or distro package. Maintaining a private branch would be the second best option. If we go with it, i would like to do it in sync with 1.1.1 stable plus s390 patches on top. Everything specific to ubuntu's packaging or non-s390 related (eg, reverting said commits from 1.1.1 stable) would be your responsibility. |
Ubuntu treats the OpenSSL alphabet soup releases as full blown new features upstream releases. In stable series we only ever cherry-pick individual patches, we never take the full Imho "dog slow / as slow as CPU" is a bug whenever crypto is done without hardware acceleration, thus to me it is enough of a bug for backports to stable series. However, as I mentioned above, Ubuntu doesn't treat -stable branch releases as bugfix only.
Correct and I agree. We are on the same page here, that's how my branch proposed on github did look. And it didn't include any reverts, nor none of the Debian specific patches, nor any Ubuntu specific patches. I did rebase those on top of the stable branch & s390 backports myself. Granted that patch stack is quite long standing, self-containted and easy to rebase / resolve conflicts. |
Right, I didnt intend to suggest anything else. Basically, HW acceleration is as close to a bugfix as it gets, while openssl bugfix releases are as close to a feature release as it gets. So lets move the discussion to the openssl PR and in case that one is rejected, I ll host your patch queue on a branch of mine. |
fake_now in the quictestlib is read/written by potentially many threads, and as such should have a surrounding lock to prevent WAR/RAW errors as caught by tsan: 2023-11-03T16:27:23.7184999Z ================== 2023-11-03T16:27:23.7185290Z WARNING: ThreadSanitizer: data race (pid=18754) 2023-11-03T16:27:23.7185720Z Read of size 8 at 0x558f6f9fe970 by main thread: 2023-11-03T16:27:23.7186726Z #0 qtest_create_quic_connection_ex <null> (quicapitest+0x14aead) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7187665Z openssl#1 qtest_create_quic_connection <null> (quicapitest+0x14b220) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7188567Z openssl#2 test_quic_write_read quicapitest.c (quicapitest+0x150ee2) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7189561Z openssl#3 run_tests <null> (quicapitest+0x2237ab) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7190294Z openssl#4 main <null> (quicapitest+0x223d2b) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7190720Z 2023-11-03T16:27:23.7190902Z Previous write of size 8 at 0x558f6f9fe970 by thread T1: 2023-11-03T16:27:23.7191607Z #0 qtest_create_quic_connection_ex <null> (quicapitest+0x14aecf) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7192505Z openssl#1 run_server_thread quictestlib.c (quicapitest+0x14b1d6) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7193361Z openssl#2 thread_run quictestlib.c (quicapitest+0x14cadf) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7193848Z 2023-11-03T16:27:23.7194220Z Location is global 'fake_now.0' of size 8 at 0x558f6f9fe970 (quicapitest+0x1af4970) 2023-11-03T16:27:23.7194636Z 2023-11-03T16:27:23.7194816Z Thread T1 (tid=18760, running) created by main thread at: 2023-11-03T16:27:23.7195465Z #0 pthread_create <null> (quicapitest+0xca12d) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7196317Z openssl#1 qtest_create_quic_connection_ex <null> (quicapitest+0x14adcb) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7197214Z openssl#2 qtest_create_quic_connection <null> (quicapitest+0x14b220) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7198111Z openssl#3 test_quic_write_read quicapitest.c (quicapitest+0x150ee2) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7198940Z openssl#4 run_tests <null> (quicapitest+0x2237ab) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7199661Z openssl#5 main <null> (quicapitest+0x223d2b) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7200083Z 2023-11-03T16:27:23.7200862Z SUMMARY: ThreadSanitizer: data race (/home/runner/work/openssl/openssl/test/quicapitest+0x14aead) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) in qtest_create_quic_connection_ex Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#22616)
Sometimes the error handling returns an ASN1_STRING
object in *out although that was not passed in by the
caller, and sometimes the error handling deletes the
ASN1_STRING but forgets to clear the *out parameter.
Therefore the caller has no chance to know, if the leaked
object in *out shall be deleted or not.
This may cause a use-after-free error e.g. in asn1_str2type:
==63312==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000073280 at pc 0x7f2652e93b08 bp 0x7ffe0e1951c0 sp 0x7ffe0e1951b0
READ of size 8 at 0x603000073280 thread T0
#0 0x7f2652e93b07 in asn1_string_embed_free crypto/asn1/asn1_lib.c:354
openssl#1 0x7f2652eb521a in asn1_primitive_free crypto/asn1/tasn_fre.c:204
openssl#2 0x7f2652eb50a9 in asn1_primitive_free crypto/asn1/tasn_fre.c:199
openssl#3 0x7f2652eb5b67 in ASN1_item_free crypto/asn1/tasn_fre.c:20
openssl#4 0x7f2652e8e13b in asn1_str2type crypto/asn1/asn1_gen.c:740
openssl#5 0x7f2652e8e13b in generate_v3 crypto/asn1/asn1_gen.c:137
openssl#6 0x7f2652e9166c in ASN1_generate_v3 crypto/asn1/asn1_gen.c:92
openssl#7 0x7f2653307b9b in do_othername crypto/x509v3/v3_alt.c:577
openssl#8 0x7f2653307b9b in a2i_GENERAL_NAME crypto/x509v3/v3_alt.c:492
openssl#9 0x7f26533087c2 in v2i_subject_alt crypto/x509v3/v3_alt.c:327
openssl#10 0x7f26533107fc in do_ext_nconf crypto/x509v3/v3_conf.c:100
openssl#11 0x7f2653310f33 in X509V3_EXT_nconf crypto/x509v3/v3_conf.c:45
openssl#12 0x7f2653311426 in X509V3_EXT_add_nconf_sk crypto/x509v3/v3_conf.c:312
openssl#13 0x7f265331170c in X509V3_EXT_REQ_add_nconf crypto/x509v3/v3_conf.c:360
openssl#14 0x564ed19d5f25 in req_main apps/req.c:806
openssl#15 0x564ed19b8de0 in do_cmd apps/openssl.c:564
openssl#16 0x564ed1985165 in main apps/openssl.c:183
openssl#17 0x7f2651c4a082 in __libc_start_main ../csu/libc-start.c:308
openssl#18 0x564ed1985acd in _start (/home/ed/OPCToolboxV5/Source/Core/OpenSSL/openssl/apps/openssl+0x139acd)
0x603000073280 is located 16 bytes inside of 24-byte region [0x603000073270,0x603000073288)
freed by thread T0 here:
#0 0x7f265413440f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:122
openssl#1 0x7f265315a429 in CRYPTO_free crypto/mem.c:311
openssl#2 0x7f265315a429 in CRYPTO_free crypto/mem.c:300
openssl#3 0x7f2652e757b9 in ASN1_mbstring_ncopy crypto/asn1/a_mbstr.c:191
openssl#4 0x7f2652e75ec5 in ASN1_mbstring_copy crypto/asn1/a_mbstr.c:38
openssl#5 0x7f2652e8e227 in asn1_str2type crypto/asn1/asn1_gen.c:681
openssl#6 0x7f2652e8e227 in generate_v3 crypto/asn1/asn1_gen.c:137
openssl#7 0x7f2652e9166c in ASN1_generate_v3 crypto/asn1/asn1_gen.c:92
openssl#8 0x7f2653307b9b in do_othername crypto/x509v3/v3_alt.c:577
openssl#9 0x7f2653307b9b in a2i_GENERAL_NAME crypto/x509v3/v3_alt.c:492
openssl#10 0x7f26533087c2 in v2i_subject_alt crypto/x509v3/v3_alt.c:327
openssl#11 0x7f26533107fc in do_ext_nconf crypto/x509v3/v3_conf.c:100
openssl#12 0x7f2653310f33 in X509V3_EXT_nconf crypto/x509v3/v3_conf.c:45
openssl#13 0x7f2653311426 in X509V3_EXT_add_nconf_sk crypto/x509v3/v3_conf.c:312
openssl#14 0x7f265331170c in X509V3_EXT_REQ_add_nconf crypto/x509v3/v3_conf.c:360
openssl#15 0x564ed19d5f25 in req_main apps/req.c:806
openssl#16 0x564ed19b8de0 in do_cmd apps/openssl.c:564
openssl#17 0x564ed1985165 in main apps/openssl.c:183
openssl#18 0x7f2651c4a082 in __libc_start_main ../csu/libc-start.c:308
previously allocated by thread T0 here:
#0 0x7f2654134808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
openssl#1 0x7f265315a4fd in CRYPTO_malloc crypto/mem.c:221
openssl#2 0x7f265315a4fd in CRYPTO_malloc crypto/mem.c:198
openssl#3 0x7f265315a945 in CRYPTO_zalloc crypto/mem.c:236
openssl#4 0x7f2652e939a4 in ASN1_STRING_type_new crypto/asn1/asn1_lib.c:341
openssl#5 0x7f2652e74e51 in ASN1_mbstring_ncopy crypto/asn1/a_mbstr.c:150
openssl#6 0x7f2652e75ec5 in ASN1_mbstring_copy crypto/asn1/a_mbstr.c:38
openssl#7 0x7f2652e8e227 in asn1_str2type crypto/asn1/asn1_gen.c:681
openssl#8 0x7f2652e8e227 in generate_v3 crypto/asn1/asn1_gen.c:137
openssl#9 0x7f2652e9166c in ASN1_generate_v3 crypto/asn1/asn1_gen.c:92
openssl#10 0x7f2653307b9b in do_othername crypto/x509v3/v3_alt.c:577
openssl#11 0x7f2653307b9b in a2i_GENERAL_NAME crypto/x509v3/v3_alt.c:492
openssl#12 0x7f26533087c2 in v2i_subject_alt crypto/x509v3/v3_alt.c:327
openssl#13 0x7f26533107fc in do_ext_nconf crypto/x509v3/v3_conf.c:100
openssl#14 0x7f2653310f33 in X509V3_EXT_nconf crypto/x509v3/v3_conf.c:45
openssl#15 0x7f2653311426 in X509V3_EXT_add_nconf_sk crypto/x509v3/v3_conf.c:312
openssl#16 0x7f265331170c in X509V3_EXT_REQ_add_nconf crypto/x509v3/v3_conf.c:360
openssl#17 0x564ed19d5f25 in req_main apps/req.c:806
openssl#18 0x564ed19b8de0 in do_cmd apps/openssl.c:564
openssl#19 0x564ed1985165 in main apps/openssl.c:183
openssl#20 0x7f2651c4a082 in __libc_start_main ../csu/libc-start.c:308
Reviewed-by: Paul Yang <kaishen.yy@antfin.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#23138)
ubsan on clang17 has started warning about the following undefined
behavior:
crypto/lhash/lhash.c:299:12: runtime error: call to function err_string_data_hash through pointer to incorrect function type 'unsigned long (*)(const void *)'
[...]/crypto/err/err.c:184: note: err_string_data_hash defined here
#0 0x7fa569e3a434 in getrn [...]/crypto/lhash/lhash.c:299:12
openssl#1 0x7fa569e39a46 in OPENSSL_LH_insert [...]/crypto/lhash/lhash.c:119:10
openssl#2 0x7fa569d866ee in err_load_strings [...]/crypto/err/err.c:280:15
[...]
The issue occurs because, the generic hash functions (OPENSSL_LH_*) will
occasionaly call back to the type specific registered functions for hash
generation/comparison/free/etc, using functions of the (example)
prototype:
[return value] <hash|cmp|free> (void *, [void *], ...)
While the functions implementing hash|cmp|free|etc are defined as
[return value] <fnname> (TYPE *, [TYPE *], ...)
The compiler, not knowing the type signature of the function pointed to
by the implementation, performs no type conversion on the function
arguments
While the C language specification allows for pointers to data of one
type to be converted to pointers of another type, it does not
allow for pointers to functions with one signature to be called
while pointing to functions of another signature. Compilers often allow
this behavior, but strictly speaking it results in undefined behavior
As such, ubsan warns us about this issue
This is an potential fix for the issue, implemented using, in effect,
thunking macros. For each hash type, an additional set of wrapper
funtions is created (currently for compare and hash, but more will be
added for free/doall/etc). The corresponding thunking macros for each
type cases the actuall corresponding callback to a function pointer of
the proper type, and then calls that with the parameters appropriately
cast, avoiding the ubsan warning
This approach is adventageous as it maintains a level of type safety,
but comes at the cost of having to implement several additional
functions per hash table type.
Related to openssl#22896
Reviewed-by: Sasa Nedvedicky <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl#23192)
if the private key is output to stdout using the HARNESS_OSSL_PREFIX,
out is a stack of BIOs and must therefore free'd using BIO_free_all.
Steps to reproduce:
$ HARNESS_OSSL_PREFIX=x OPENSSL_CONF=apps/openssl.cnf util/shlib_wrap.sh apps/openssl req -new -keyout - -passout pass: </dev/null
[...]
Direct leak of 128 byte(s) in 1 object(s) allocated from:
#0 0x7f6f692b89cf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
openssl#1 0x7f6f686eda00 in CRYPTO_malloc crypto/mem.c:202
openssl#2 0x7f6f686edba0 in CRYPTO_zalloc crypto/mem.c:222
openssl#3 0x7f6f68471bdf in BIO_new_ex crypto/bio/bio_lib.c:83
openssl#4 0x7f6f68491a8f in BIO_new_fp crypto/bio/bss_file.c:95
openssl#5 0x555c5f58b378 in dup_bio_out apps/lib/apps.c:3014
openssl#6 0x555c5f58f9ac in bio_open_default_ apps/lib/apps.c:3175
openssl#7 0x555c5f58f9ac in bio_open_default apps/lib/apps.c:3203
openssl#8 0x555c5f528537 in req_main apps/req.c:683
openssl#9 0x555c5f50e315 in do_cmd apps/openssl.c:426
openssl#10 0x555c5f4c5575 in main apps/openssl.c:307
openssl#11 0x7f6f680461c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
SUMMARY: AddressSanitizer: 128 byte(s) leaked in 1 allocation(s).
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#23365)
This merges backports that are already part of ubuntu.
You may want to "linearize" the history, and possibly re-cherrypick these from scratch from master yourself. All of my comments are annotated with "cherry picked from commit "