libgcrypt: bug in _gcry_poly1305_armv7_neon_init_ext
Open, HighPublic

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
t8m created this task.Thu, Jan 30, 5:31 PM
t8m added a comment.Thu, Jan 30, 5:40 PM

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.

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.

jukivili added a comment.EditedSat, Feb 1, 6:39 PM

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