Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asan finding in VMAC on i686 in inline asm #860

Closed
noloader opened this issue Jul 5, 2019 · 2 comments
Closed

Asan finding in VMAC on i686 in inline asm #860

noloader opened this issue Jul 5, 2019 · 2 comments
Labels

Comments

@noloader
Copy link
Collaborator

noloader commented Jul 5, 2019

This issue has been nagging us for about 4 years, since we started testing with Asan. Also see Issue 304.

Originally we thought (hoped?) it may be a false positive since the finding pointed to an address in our stack frame. We also were not able to duplicate with Valgrind, so it became a low priority item. However, the finding has not cleared itself in several versions of Asan.

Here is what it looks like on a 32-bit Intel machine with ASM enabled:

$ ./cryptest.exe tv vmac
Using seed: 1562310506

Testing MAC algorithm VMAC(AES)-64.
.ASAN:DEADLYSIGNAL
=================================================================
==30299==ERROR: AddressSanitizer: SEGV on unknown address 0xffffffd0 (pc 0x00cb1d05 bp 0xbfd41708 sp 0xbfd41630 T0)
    #0 0xcb1d04 in CryptoPP::VMAC_Base::VHASH_Update_SSE2(unsigned long long const*, unsigned int, int) /home/jwalton/cryptopp/vmac.cpp:427
    #1 0xcb275a in CryptoPP::VMAC_Base::VHASH_Update(unsigned long long const*, unsigned int) /home/jwalton/cryptopp/vmac.cpp:756
    ...

Stepping it under GDB:

(gdb) r tv vmac
...
(gdb) disass
   ...
   0x00c0dcf9 <+955>:   jne    0xc0dabc <VMAC_Base::VHASH_Update_SSE2(unsigned long long const*, unsigned int, int)+382>
   0x00c0dcff <+961>:   add    $0xc,%esp
   0x00c0dd02 <+964>:   pop    %ebp
   0x00c0dd03 <+965>:   emms
=> 0x00c0dd05 <+967>:   mov    -0x40(%ebx),%ebx
   0x00c0dd08 <+970>:   mov    -0xbc(%ebp),%ebx
   0x00c0dd0e <+976>:   cmp    %ebx,-0xc8(%ebp)
   ...

The mov -0x40(%ebx),%ebx is due to mov %0, %%ebx in vmac.cpp. Obviously, that won't work. ebx was blown away since it was used in VHASH_Update_SSE2, so we can't use something relative to ebx to restore ebx. We need to find a better pattern than below due to code generation:

	AS_POP_IF86(	bp)
	AS1(	emms)
#ifdef __GNUC__
	ATT_PREFIX
	AS2(	mov	%0, %%ebx)
		: "=m" (temp)
		: "m" (L1KeyLength), "c" (blocksRemainingInWord64), "S" (data), 
		  "D" (nhK+tagPart*2), "d" (m_isFirstBlock), "a" (polyS+tagPart*4)
		: "memory", "cc"
	);
#endif
noloader added a commit that referenced this issue Jul 5, 2019
This one has been nagging us for a while. Tested OK under i686 and x86_64.
@noloader noloader closed this as completed Jul 5, 2019
noloader added a commit that referenced this issue Jul 5, 2019
cryptest.sh showed it broke at -O3
@noloader noloader reopened this Jul 5, 2019
noloader added a commit that referenced this issue Jul 5, 2019
Second try. The first try cleared the Asan finding but broke at -O3. Eventually we will skin this cat.
@noloader
Copy link
Collaborator Author

noloader commented Jul 9, 2019

We had to revert the previous check-in because it failed under testing at -O3. At -O3 GCC was generating code relative to ESP, which was also blown away in the function.

Cleared at Commit ad99fc5b05e1.

Leaving this report open until we can test the change back to GCC 3 and Fedora 1.

@noloader
Copy link
Collaborator Author

Tested OK on early Ubuntu and Fedora. Closing issue.

@noloader noloader added the Bug label Dec 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant