Hi Laszlo, > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Tuesday, October 22, 2019 12:29 AM > To: devel@edk2.groups.io; Zhang, Shenglei <shenglei.zh...@intel.com> > Cc: Wang, Jian J <jian.j.w...@intel.com>; Lu, XiaoyuX > <xiaoyux...@intel.com>; David Woodhouse <dw...@infradead.org> > Subject: Re: [edk2-devel] [PATCH] CryptoPkg: Upgrade OpenSSL to 1.1.1d > > On 10/21/19 10:06, Zhang, Shenglei wrote: > > Update openssl from 1.1.1b to 1.1.1d. > > Something needs to be noticed is that, there is a bug existing in the > > released 1_1_1d version(894da2fb7ed5d314ee5c2fc9fd2d9b8b74111596), > > which causes build failure. So we switch the code base to a usable > > version, which is 2 commits later than the stable tag. > > Now we use the version c3656cc594daac8167721dde7220f0e59ae146fc. > > This log is to fix the build failure. > > https://bugzilla.tianocore.org/show_bug.cgi?id=2226 > > > > Cc: Jian J Wang <jian.j.w...@intel.com> > > Cc: Xiaoyu Lu <xiaoyux...@intel.com> > > Signed-off-by: Shenglei Zhang <shenglei.zh...@intel.com> > > --- > > CryptoPkg/Library/OpensslLib/OpensslLib.inf | 57 ------------------- > > .../Library/OpensslLib/OpensslLibCrypto.inf | 49 ---------------- > > CryptoPkg/Library/OpensslLib/openssl | 2 +- > > 3 files changed, 1 insertion(+), 107 deletions(-) > > When I try to apply this patch manually, on top of current master > (91f98c908627), then "git am" fails. > > However, if I try to reproduce this patch myself (advancing the > submodule to c3656cc594da, and then running "process_files.pl"), then > the result ("git diff") matches the code changes in the patch -- not > counting CRLF vs. LF, anyway. > > (It seems like the "git am" failure is due to mixed line-endings within > the patch -- the submodule reference hunk uses LFs, not CRLFs. I can > live with that.) > > Having to use openssl at c3656cc594da is unfortunate, but I think it's > justified. > > Unfortunately, with this update, the following build command fails for > me (it may fail for other OVMF builds as well, this was simply my first > attempt): > > $ nice build \ > -a IA32 \ > -p OvmfPkg/OvmfPkgIa32.dsc \ > -t GCC48 \ > -b DEBUG \ > -D SMM_REQUIRE \ > -D SECURE_BOOT_ENABLE \ > -D NETWORK_IP6_ENABLE \ > -D NETWORK_TLS_ENABLE \ > -D NETWORK_HTTP_BOOT_ENABLE \ > -D E1000_ENABLE \ > -n 4 \ > --report-file=$HOME/tmp/build.ovmf.32.report \ > --log=$HOME/tmp/build.ovmf.32.log \ > --cmd-len=65536 \ > --genfds-multi-thread > > The directly failing command is: > > "gcc" \ > -o > Build/OvmfIa32/DEBUG_GCC48/IA32/MdeModulePkg/Universal/Variable/R > untimeDxe/VariableSmm/DEBUG/VariableSmm.dll \ > -nostdlib \ > -Wl,-n,-q,--gc-sections \ > -z common-page-size=0x20 \ > -Wl,--entry,_ModuleEntryPoint \ > -u _ModuleEntryPoint \ > -Wl,- > Map,Build/OvmfIa32/DEBUG_GCC48/IA32/MdeModulePkg/Universal/Varia > ble/RuntimeDxe/VariableSmm/DEBUG/VariableSmm.map,--whole-archive \ > -Wl,-m,elf_i386,--oformat=elf32-i386 \ > -z common-page-size=0x1000 \ > -Wl,--start- > group,@Build/OvmfIa32/DEBUG_GCC48/IA32/MdeModulePkg/Universal/Va > riable/RuntimeDxe/VariableSmm/OUTPUT/static_library_files.lst,--end- > group \ > -g \ > -fshort-wchar \ > -fno-builtin \ > -fno-strict-aliasing \ > -Wall \ > -Werror \ > -Wno-array-bounds \ > -ffunction-sections \ > -fdata-sections \ > -include AutoGen.h \ > -fno-common \ > -DSTRING_ARRAY_NAME=VariableSmmStrings \ > -m32 \ > -march=i586 \ > -malign-double \ > -fno-stack-protector \ > -D EFI32 \ > -fno-asynchronous-unwind-tables \ > -Wno-address \ > -Os \ > -mno-mmx \ > -mno-sse \ > -D DISABLE_NEW_DEPRECATED_INTERFACES \ > -Wl,--defsym=PECOFF_HEADER_SIZE=0x220 \ > -Wl,--script=BaseTools/Scripts/GccBase.lds > > And the error message: > > > > Build/OvmfIa32/DEBUG_GCC48/IA32/CryptoPkg/Library/OpensslLib/Openssl > Lib/OUTPUT/OpensslLib.lib(dso_lib.obj): In function `DSO_new_method': > > CryptoPkg/Library/OpensslLib/openssl/crypto/dso/dso_lib.c:25: undefined > reference to `DSO_METHOD_openssl' > > > Build/OvmfIa32/DEBUG_GCC48/IA32/CryptoPkg/Library/OpensslLib/Openssl > Lib/OUTPUT/OpensslLib.lib(dso_lib.obj): In function `DSO_pathbyaddr': > > CryptoPkg/Library/OpensslLib/openssl/crypto/dso/dso_lib.c:314: > undefined reference to `DSO_METHOD_openssl' > > This is strange, because the missing function is provided by > "crypto/dso/dso_openssl.c", which is listed in the INF files. > > Hmmm. I ran the following command too: > > $ build \ > -p CryptoPkg/CryptoPkg.dsc \ > -a IA32 \ > -b NOOPT \ > -t GCC48 \ > -m CryptoPkg/Library/OpensslLib/OpensslLib.inf > > This compiles OK. The last commands are: > > > rm -f > Build/CryptoPkg/NOOPT_GCC48/IA32/CryptoPkg/Library/OpensslLib/Opens > slLib/OUTPUT/OpensslLib.lib > > > > "ar" cr \ > > > Build/CryptoPkg/NOOPT_GCC48/IA32/CryptoPkg/Library/OpensslLib/Opens > slLib/OUTPUT/OpensslLib.lib \ > > > @Build/CryptoPkg/NOOPT_GCC48/IA32/CryptoPkg/Library/OpensslLib/Ope > nsslLib/OUTPUT/object_files.lst > > If I check the file > > > Build/CryptoPkg/NOOPT_GCC48/IA32/CryptoPkg/Library/OpensslLib/Opens > slLib/OUTPUT/object_files.lst > > I definitely see > > > Build/CryptoPkg/NOOPT_GCC48/IA32/CryptoPkg/Library/OpensslLib/Opens > slLib/OUTPUT/openssl/crypto/dso/dso_openssl.obj > > there. However, if I run > > nm > Build/CryptoPkg/NOOPT_GCC48/IA32/CryptoPkg/Library/OpensslLib/Opens > slLib/OUTPUT/OpensslLib.lib > > then the DSO_METHOD_openssl() symbol is reported as undefined: > > > dso_lib.obj: > > U DSO_METHOD_openssl > > In fact, if I run the "nm" command on "dso_openssl.obj" itself, the > output is totally empty! (No symbols in the object file.) > > Ahh, I know what's up. See the source code in > "crypto/dso/dso_openssl.c": > > > #ifdef DSO_NONE > > > > static DSO_METHOD dso_meth_null = { > > "NULL shared library method" > > }; > > > > DSO_METHOD *DSO_METHOD_openssl(void) > > { > > return &dso_meth_null; > > } > > #endif > > The #ifdef comes from OpenSSL commit 5fba3afad017 ("Rework DSO API > conditions and configuration option", 2019-04-10), which is part of > OpenSSL_1_1_1c. See the change (excerpt): > > $ git show 5fba3afad017 -- \ > crypto/dso/dso_openssl.c \ > crypto/include/internal/dso_conf.h.in > > > commit 5fba3afad01707f4a8856a35500de007a8a256ec > > Author: Richard Levitte <levi...@openssl.org> > > Date: Mon Apr 1 06:40:33 2019 +0200 > > > > Rework DSO API conditions and configuration option > > > > 'no-dso' is meaningless, as it doesn't get any macro defined. > > Therefore, we remove all checks of OPENSSL_NO_DSO. However, there > may > > be some odd platforms with no DSO scheme. For those, we generate > the > > internal macro DSO_NONE aand use it. > > > > Reviewed-by: Paul Dale <paul.d...@oracle.com> > > (Merged from https://github.com/openssl/openssl/pull/8622) > > > > diff --git a/crypto/dso/dso_openssl.c b/crypto/dso/dso_openssl.c > > index 6626331e9256..eeebd98087b4 100644 > > --- a/crypto/dso/dso_openssl.c > > +++ b/crypto/dso/dso_openssl.c > > @@ -9,7 +9,7 @@ > > > > #include "dso_locl.h" > > > > -#if !defined(DSO_VMS) && !defined(DSO_DLCFN) && !defined(DSO_DL) > && !defined(DSO_WIN32) && !defined(DSO_DLFCN) > > +#ifdef DSO_NONE > > > > static DSO_METHOD dso_meth_null = { > > "NULL shared library method" > > diff --git a/crypto/include/internal/dso_conf.h.in > b/crypto/include/internal/dso_conf.h.in > > index d6e9d1b1baae..17fae7d8023a 100644 > > --- a/crypto/include/internal/dso_conf.h.in > > +++ b/crypto/include/internal/dso_conf.h.in > > @@ -10,7 +10,6 @@ > > > > #ifndef HEADER_DSO_CONF_H > > # define HEADER_DSO_CONF_H > > -{- output_off() if $disabled{dso} -} > > {- # The DSO code currently always implements all functions so that no > > # applications will have to worry about that from a compilation point > > # of view. However, the "method"s may return zero unless that platform > > @@ -18,6 +17,9 @@ > > # by a define "DSO_<name>" ... we translate the "dso_scheme" config > > # string entry into using the following logic; > > my $scheme = uc $target{dso_scheme}; > > + if (!$scheme) { > > + $scheme = "NONE"; > > + } > > my @macros = ( "DSO_$scheme" ); > > if ($scheme eq 'DLFCN') { > > @macros = ( "DSO_DLFCN", "HAVE_DLFCN_H" ); > > @@ -26,5 +28,4 @@ > > } > > join("\n", map { "# define $_" } @macros); -} > > # define DSO_EXTENSION "{- $target{dso_extension} -}" > > -{- output_on() if $disabled{dso} -} > > #endif > > Sure enough, "build.info" invokes the generator on this template file > too: > > > DEPEND[include/openssl/opensslconf.h]=configdata.pm > > > GENERATE[include/openssl/opensslconf.h]=include/openssl/opensslconf.h.i > n > > DEPEND[crypto/include/internal/bn_conf.h]=configdata.pm > > > GENERATE[crypto/include/internal/bn_conf.h]=crypto/include/internal/bn_ > conf.h.in > > DEPEND[crypto/include/internal/dso_conf.h]=configdata.pm > > > GENERATE[crypto/include/internal/dso_conf.h]=crypto/include/internal/dso > _conf.h.in > > Unfortunately, it seems like the DSO_NONE internal macro is *not* > generated, in our case. > > ... Ah. Our "process_files.pl" script manually generates > "opensslconf.h", from the configuration data. But, we have never done > the same for "dso_conf.h". > > Thus far, we've gotten away with it, because the *absence* of all > DSO_xxx flags happened to do the right thing for us. But now, the "right > thing for us" actually depends on a new macro, DSO_NONE, and for getting > that, we need to invoke the generator on "dso_conf.h.in" too. Otherwise, > "crypto/dso/dso_openssl.c" gets pre-processed to an empty source file, > as a result of openssl commit 5fba3afad017. > > Therefore, the bug is in "process_files.pl". Please invoke the generator > on "dso_conf.h.in" too, similarly to "opensslconf.h.in". > > ... And now I'm left with the question: did you test this patch at all, > with any real platform?
Actually... I didn’t. Because I treated it as a small change with little risk. I found there is no change except the auto-generated things in OpensslLib.inf and OpensslLibCrypto.inf. With the update in "process_files.pl" according to your comments, things can be generated into dso_conf.h, where DSO_NONE is not absent now. And, I test this change on a real platform, which can boot to Shell with this patch. I 'll send a new patch for the change in "process_files.pl". Thanks, Shenglei > > Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#49386): https://edk2.groups.io/g/devel/message/49386 Mute This Topic: https://groups.io/mt/36311456/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
