Bug 2031019

Summary: zlib: Unnecessary IFUNC resolver for crc32_z
Product: Red Hat Enterprise Linux 9 Reporter: Florian Weimer <fweimer>
Component: zlibAssignee: Lukas Javorsky <ljavorsk>
Status: CLOSED ERRATA QA Contact: Dita Stehlikova <dstehlik>
Severity: low Docs Contact:
Priority: unspecified    
Version: 9.0CC: bugproxy, databases-maint, hhorak, jkejda, ljavorsk, mmuzila, pkubat, rhel-cs-apps-subsystem-qe, zmiklank
Target Milestone: rcKeywords: Triaged
Target Release: ---Flags: pm-rhel: mirror+
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: zlib-1.2.11-38.el9 Doc Type: No Doc Update
Doc Text:
Story Points: ---
Clone Of: 2031015 Environment:
Last Closed: 2023-05-09 08:25:05 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 2031015    
Bug Blocks:    
Attachments:
Description Flags
Patch [1/3] to add support to zlib 1.2.11
none
Patch [3/3] to add support to zlib 1.2.11
none
Patch [2/3] to add support to zlib 1.2.11 none

Description Florian Weimer 2021-12-10 10:22:09 UTC
+++ This bug was initially created as a clone of Bug #2031015 +++

The IFUNC resolver for crc32_z just returns a constant result on x86_64:

(gdb) disassemble/m crc32_z
Dump of assembler code for function crc32_z_ifunc:
261	{
   0x0000000000003280 <+0>:	endbr64 

262	#if _ARCH_PWR8==1
263	#if defined(__BUILTIN_CPU_SUPPORTS__)
264	    if (__builtin_cpu_supports("arch_2_07"))
265	        return crc32_vpmsum;
266	#else
267	    if (getauxval(AT_HWCAP2) & PPC_FEATURE2_ARCH_2_07)
268	        return crc32_vpmsum;
269	#endif
270	#endif /* _ARCH_PWR8 */
271	
272	/* return a function pointer for optimized arches here */
273	
274	#ifdef DYNAMIC_CRC_TABLE
275	    if (crc_table_empty)
276	        make_crc_table();
277	#endif /* DYNAMIC_CRC_TABLE */
278	
279	#ifdef BYFOUR
280	    if (sizeof(void *) == sizeof(ptrdiff_t)) {

281	        z_crc_t endian;

282	
283	        endian = 1;

284	        if (*((unsigned char *)(&endian)))

285	            return crc32_little;
   0x0000000000003284 <+4>:	lea    -0x4eb(%rip),%rax        # 0x2da0 <crc32_little>
   0x000000000000328b <+11>:	retq   
   0x000000000000328c <+0>:	nopl   0x0(%rax)

End of assembler dump.

I assume the IFUNC resolver was added for the s390x optimizations. On x86-64, it is definitely unnecessary, likewise for AArch64. I'll check with IBM regarding the POWER8 check and file a separate bug for that if necessary.

The new IFUNC resolver causes problems with symbol interposition (zlib functions are frequently bundled with other software). ELF dependency tracking in the dynamic loader does not work with interposition, and crashes can be the result. The fix is certainly to hide/rename the other symbols (so that the bundled zlib does not interpose the system implementation), but avoiding the IFUNC resolver where possible prevents these crashes and improves backwards compatibility.

Comment 2 Tulio Magno Quites Machado Filho 2021-12-13 14:47:39 UTC
> I'll check with IBM regarding the POWER8 check and file a separate bug for that if necessary.

This is important for Power8. Actually, we added the code.
Would you like to mirror this issue to IBM?

Comment 3 Florian Weimer 2021-12-13 16:23:06 UTC
(In reply to Tulio Magno Quites Machado Filho from comment #2)
> > I'll check with IBM regarding the POWER8 check and file a separate bug for that if necessary.
> 
> This is important for Power8. Actually, we added the code.
> Would you like to mirror this issue to IBM?

Sure, please go ahead. Thanks.

Comment 4 IBM Bug Proxy 2021-12-16 18:10:27 UTC
------- Comment From mscastanho 2021-12-16 13:02 EDT-------
As mentioned before, this was added by a patch to provide an optimized version of crc32 for POWER8. There is actually a revised version of that patch waiting for review upstream [1][2]. That one explicitly checks the architecture to make sure it only adds the resolver for powerpc, so should avoid the problem on x86.

Could we use that patch instead?

[1] https://github.com/madler/zlib/pull/478
[2] https://patch-diff.githubusercontent.com/raw/madler/zlib/pull/478.patch

Comment 7 Honza Horak 2022-02-15 09:29:34 UTC
There were some issues with Power improvements before (BZ#1741266) and these enhancements are not included in Fedora yet. Since the end of the development phase for RHEL-9.0.0 is getting close, and this enhanced patch is not yet tested in Fedora either, I don't feel good about including this at this point into 9.0.0.

zlib is used by many other components, so in order to test it properly, I'd rather suggest to apply the patch in Fedora now and once shortly after also for CentOS Stream 9, which will turn to RHEL-9.1.0. That way it will be properly tested by other components and available for testing in CentOS Stream 9 quite soon as well.

Would that be acceptable plan for IBM?

Comment 8 IBM Bug Proxy 2022-02-15 14:20:43 UTC
------- Comment From tulioqm.com 2022-02-15 09:18 EDT-------
> Would that be acceptable plan for IBM?

I think there are 2 possible interpretations for your previous comment and it's unclear to me which one you're proposing:

1. Do not apply the new version of the patch in RHEL 9.0, but keep distributing in RHEL 9.0 the patch from RH BZ #1666798 that has been distributed since RHEL 8.2.

2. Remove all CRC32 optimizations for ppc64le, even those distributed since RHEL 8.2.

I do agree with option 1. It's unclear if this could impact other architectures as reported by Florian.  We might have to restrict the patch to ppc64le only.

However, option 2 would cause a 100x slowdown in CRC32 calculation on POWER, which is not acceptable.

With that said, could we adopt option 1, please?

It doesn't hurt to clarify that the patch in this bugzilla is an updated version of the patch in RH BZ #1666798. Anyway, I agree with you that all new changes to zlib should be tested carefully before including in a new RHEL version.

Comment 9 Honza Horak 2022-02-16 12:07:06 UTC
I definitely meant #1, if we miss some patch from RHEL-8, then I'd consider it an issue to fix, but I believe that should not be the case now.

Bwt. the sources are visible as part of CentOS Stream 9: https://gitlab.com/redhat/centos-stream/rpms/zlib (which is currently the same as RHEL-9).

Comment 10 IBM Bug Proxy 2022-02-16 18:00:25 UTC
------- Comment From tulioqm.com 2022-02-16 12:59 EDT-------
> I definitely meant #1, if we miss some patch from RHEL-8, then I'd consider
> it an issue to fix, but I believe that should not be the case now.

Great! So I do agree with delaying this bug to RHEL 9.1.
Please, let us know if there is anything we can do in order to help with the inclusion of this patch in Fedora and CentOS Stream 9.

> Bwt. the sources are visible as part of CentOS Stream 9:
> https://gitlab.com/redhat/centos-stream/rpms/zlib (which is currently the
> same as RHEL-9).

Ack. And that patch looks good to me.

Thank you!

Comment 11 Honza Horak 2022-02-21 10:16:01 UTC
Moving to 9.1.0 per discussion above.

Comment 13 Lukas Javorsky 2022-11-16 12:08:07 UTC
This is still waiting for the IBM patch support same as BZ#2031015.

Once we have the patch for the 1.2.11 version of zlib, we can backport it to the RHEL zlib packages.

Comment 14 IBM Bug Proxy 2022-11-30 07:40:57 UTC
------- Comment From Manjunath.Matti 2022-11-30 02:32 EDT-------
I have uploaded the patches on bug https://bugzilla.linux.ibm.com/show_bug.cgi?id=195660. Bug #195660 also deals with the same version of zlib 1.2.11 but on RHEL 8.5. I have tested these patches on RHEL 9.0 as well.

Comment 15 IBM Bug Proxy 2022-11-30 13:50:42 UTC
------- Comment From Manjunath.Matti 2022-11-30 08:48 EDT-------
I have attached the 3 patches, which can be applied to the zlib 1.2.11 sources.

The patches are back ported from the below link.

[1] https://github.com/madler/zlib/pull/478

I have built and just run make test. Looks Ok to me.

Comment 16 IBM Bug Proxy 2022-11-30 13:50:44 UTC
Created attachment 1928740 [details]
Patch [1/3] to add support to zlib 1.2.11

Comment 17 IBM Bug Proxy 2022-11-30 13:50:45 UTC
Created attachment 1928741 [details]
Patch [3/3] to add support to zlib 1.2.11

Comment 18 IBM Bug Proxy 2022-11-30 13:50:47 UTC
Created attachment 1928742 [details]
Patch [2/3] to add support to zlib 1.2.11

Comment 22 Lukas Javorsky 2023-02-07 15:29:26 UTC
After a quick response from IBM, and internal decision process, we decided to fix it in 9.2.0

Comment 31 errata-xmlrpc 2023-05-09 08:25:05 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (zlib bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2023:2563