Page MenuHome GnuPG

libgcrypt: bug in _gcry_poly1305_armv7_neon_init_ext
Closed, ResolvedPublic

Description

There is a bug in _gcry_poly1305_armv7_neon_init_ext that appears only in some occasions depending on the code which calls the function.

Please see the https://bugzilla.redhat.com/show_bug.cgi?id=1794488#c5 for comprehensive description of the issue.

Details

Version
libgcrypt-1.8.5

Event Timeline

This is a patch to fix the issue. I am no ARMv7 assembly expert, but all the tests pass with this applied.

werner triaged this task as High priority.Jan 31 2020, 11:39 AM
werner added a project: libgcrypt.

Thanks for reporting this this. Your patch is correct.

Original function from https://github.com/floodyberry/poly1305-opt/blob/master/app/extensions/poly1305/poly1305_neon-32.inc took three parameters. When I integrated implementation to libgcrypt, I forgot to change assembly for that third parameter (not used in libgcrypt).

That third parameter should be fixed to zero for libgcrypt use-case. This is what is done for amd64/sse/avx2 poly1305 implementations. So setting r2 to zero at function entry would fix the issue, but there is another problem which prevent that fix from working. Code in poly1305_init_ext_neon tries to copy value of r2 to r14 and if r2 is zero, set r14 to -1. However, even the original gets 'r2==0' check wrong as 'and r2,r2,r2' does not set the flags. For setting cc flags, 'ands' instruction should had been used.

I prepared slightly different patch, with 'and r2,r2,r2' instruction removed as it is no longer needed.

werner claimed this task.

The commit which fixes this is rC761a1a0d30

Note: This does not apply to1.9 because we use a new implementation for Poly1305