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

Hang in Rijndael's Enc::AdvancedProcessBlocks and AliasedWithTable #132

Closed
noloader opened this issue Feb 6, 2016 · 2 comments
Closed
Labels
Bug clang-3.6 Clang compiler version 3.6 gcc-4 GCC compiler version 4

Comments

@noloader
Copy link
Collaborator

noloader commented Feb 6, 2016

The cleanup of CRYPTOPP_GENERATE_X64_MASM (see commits d55d7fe and 58ed2fc) revealed Rijndael::Enc::AdvancedProcessBlocks experiences a hang with AliasedWithTable. The issue is present on Linux, OS X and Windows with SSE2 ASM using aligned data. It can be duplicated with #define CRYPTOPP_NO_UNALIGNED_DATA_ACCESS and #define CRYPTOPP_DISABLE_SSSE3.

From rijndael.cpp (near line 1205):

 1230         const byte* zeros = (byte *)(Te+256);
 1231         byte *space;
 1232 
 1233         do {
 1234             // https://msdn.microsoft.com/en-us/library/5471dc8s.aspx
 1235 #if (CRYPTOPP_MSC_VERSION >= 1400)
 1236             space = (byte *)_malloca(255+sizeof(Locals));
 1237             space += (256-(size_t)space%256)%256;
 1238 #else
 1239             space = (byte *)alloca(255+sizeof(Locals));
 1240             space += (256-(size_t)space%256)%256;
 1241 #endif
 1242         }
 1243         while (AliasedWithTable(space, space+sizeof(Locals)));

These gyrations appear to align the pointer on a 0x100 boundary:

 1239             space = (byte *)alloca(255+sizeof(Locals));
 1240             space += (256-(size_t)space%256)%256;

When misbehaving the function AliasedWithTable never returns false (near line 1015):

 1016 static inline bool AliasedWithTable(const byte *begin, const byte *end)
 1017 {
 1018     size_t s0 = size_t(begin)%4096, s1 = size_t(end)%4096;
 1019     size_t t0 = size_t(Te)%4096, t1 = (size_t(Te)+sizeof(Te))%4096;
 1020     if (t1 > t0)
 1021         return (s0 >= t0 && s0 < t1) || (s1 > t0 && s1 <= t1);
 1022     else
 1023         return (s0 < t1 || s1 <= t1) || (s0 >= t0 || s1 > t0);
 1024 }

A test against the Crypto++ 5.6.2 shows the issue is present there, also. To duplicate under 5.6.2, uncomment CRYPTOPP_ALLOW_UNALIGNED_DATA_ACCESS in config.h and add the code from Commit c8de423a.


Here's a quick trace with some instrumentation to dump the variables. Its using CRYPTOPP_NO_UNALIGNED_DATA_ACCESS.

space: 0x7fff5f526f40, space: 0x7fff5f527000, locals: 312
  s0: 0, s1: 138
  t0: 3b0, t1: 3b0
space: 0x7fff5f526d00, space: 0x7fff5f526d00, locals: 312
  s0: d00, s1: e38
  t0: 3b0, t1: 3b0
space: 0x7fff5f526ac0, space: 0x7fff5f526b00, locals: 312
  s0: b00, s1: c38
  t0: 3b0, t1: 3b0
space: 0x7fff5f526880, space: 0x7fff5f526900, locals: 312
  s0: 900, s1: a38
  t0: 3b0, t1: 3b0
space: 0x7fff5f526640, space: 0x7fff5f526700, locals: 312
  s0: 700, s1: 838
  t0: 3b0, t1: 3b0
space: 0x7fff5f526400, space: 0x7fff5f526400, locals: 312
  s0: 400, s1: 538
  t0: 3b0, t1: 3b0
space: 0x7fff5f5261c0, space: 0x7fff5f526200, locals: 312
  s0: 200, s1: 338
  t0: 3b0, t1: 3b0
space: 0x7fff5f525f80, space: 0x7fff5f526000, locals: 312
  s0: 0, s1: 138
  t0: 3b0, t1: 3b0
space: 0x7fff5f525d40, space: 0x7fff5f525e00, locals: 312
  s0: e00, s1: f38
  t0: 3b0, t1: 3b0
space: 0x7fff5f525b00, space: 0x7fff5f525b00, locals: 312
  s0: b00, s1: c38
  t0: 3b0, t1: 3b0
...
space: 0x7fff5ed2b1c0, space: 0x7fff5ed2b200, locals: 312
  s0: 200, s1: 338
  t0: 3b0, t1: 3b0
space: 0x7fff5ed2af80, space: 0x7fff5ed2b000, locals: 312
  s0: 0, s1: 138
  t0: 3b0, t1: 3b0
space: 0x7fff5ed2ad40, space: 0x7fff5ed2ae00, locals: 312
  s0: e00, s1: f38
  t0: 3b0, t1: 3b0
space: 0x7fff5ed2ab00, space: 0x7fff5ed2ab00, locals: 312
  s0: b00, s1: c38
  t0: 3b0, t1: 3b0
Segmentation fault: 11
@noloader noloader added Bug gcc-4 GCC compiler version 4 clang-3.6 Clang compiler version 3.6 labels Feb 6, 2016
@noloader
Copy link
Collaborator Author

noloader commented Feb 6, 2016

A temporary fix was re-added at Commit b773052b3d580214. It reincorporates the original fix from Issue 42. It does not use the methods discussed below.

@noloader
Copy link
Collaborator Author

We can no longer duplicate this issue. There was likely something going on, but it got cleared when we tightened the code around aligned/unaligned data access.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug clang-3.6 Clang compiler version 3.6 gcc-4 GCC compiler version 4
Projects
None yet
Development

No branches or pull requests

1 participant