libgcrypt: bug in _gcry_poly1305_armv7_neon_init_ext
Closed, ResolvedPublic


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 for comprehensive description of the issue.


t8m created this task.Jan 30 2020, 5:31 PM
t8m added a comment.Jan 30 2020, 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 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.EditedFeb 1 2020, 6:39 PM

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

werner closed this task as Resolved.Jul 6 2020, 10:54 AM
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