Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

workaround for alignment issue #108

Closed
clinty opened this issue Oct 25, 2016 · 3 comments · Fixed by #174
Closed

workaround for alignment issue #108

clinty opened this issue Oct 25, 2016 · 3 comments · Fixed by #174

Comments

@clinty
Copy link

clinty commented Oct 25, 2016

Author: Steve Langasek <steve.langasek@ubuntu.com>
Description: fix alignment of memory blocks used by SHA3
 SHA3 works in 64-bit chunks, but the incoming data pointer can be at any
 address.  Copy our data to an aligned address, to avoid SIGBUS on certain
 platforms.
 .
 This is not the only alignment issue in the code, but it is the one that
 manifests as SIGBUS on the most architectures.

Index: haskell-cryptonite-0.20/cbits/cryptonite_sha3.c
===================================================================
--- haskell-cryptonite-0.20.orig/cbits/cryptonite_sha3.c
+++ haskell-cryptonite-0.20/cbits/cryptonite_sha3.c
@@ -23,6 +23,7 @@
  */

 #include <stdint.h>
+#include <stdlib.h>
 #include <string.h>
 #include "cryptonite_bitfn.h"
 #include "cryptonite_sha3.h"
@@ -107,6 +108,7 @@ void cryptonite_sha3_init(struct sha3_ct
 void cryptonite_sha3_update(struct sha3_ctx *ctx, const uint8_t *data, uint32_t len)
 {
    uint32_t to_fill;
+   uint64_t *data_aligned = NULL;

    to_fill = ctx->bufsz - ctx->bufindex;

@@ -124,6 +126,13 @@ void cryptonite_sha3_update(struct sha3_
            ctx->bufindex = 0;
    }

+   /* fix up alignment if necessary */
+   if (len && (unsigned long) data & 7) {
+           data_aligned = malloc(len);
+           memcpy(data_aligned, data, len);
+           data = (uint8_t *) data_aligned;
+   }
+
    /* process as much ctx->bufsz-block */
    for (; len >= ctx->bufsz; len -= ctx->bufsz, data += ctx->bufsz)
            sha3_do_chunk(ctx->state, (uint64_t *) data, ctx->bufsz / 8);
@@ -133,6 +142,7 @@ void cryptonite_sha3_update(struct sha3_
            memcpy(ctx->buf + ctx->bufindex, data, len);
            ctx->bufindex += len;
    }
+   free(data_aligned);
 }

 void cryptonite_sha3_finalize(struct sha3_ctx *ctx, uint32_t hashlen, uint8_t *out)
@vincenthz
Copy link
Member

Not going to apply this:

  • It shouldn't allocate memory dynamically
  • malloc without testing for return success

@vorlonofportland
Copy link
Contributor

Hi Vincent,

The comment about not checking the return of malloc is fair. As far as not using dynamic memory allocation, how do you want this to work, given that bufsz is variable and external to this function? You don't know how large of a static buffer to put on the stack, and I don't see any way to change the code to ensure alignment of the input on the haskell side.

jrtc27 added a commit to jrtc27/cryptonite that referenced this issue Nov 6, 2016
@vincenthz
Copy link
Member

vincenthz commented Nov 7, 2016

I think without having looked too much at this specific case, would choose the maximum size of bufsz on the stack directly, and copy the unaligned data bufsz by bufsz to it, separeting the case where we can loop on aligned data.

vorlonofportland added a commit to vorlonofportland/cryptonite that referenced this issue Jun 24, 2017
Follow-on to commit ba10930, which implemented a trampoline buffer but then
used the unaligned input character array instead.  This commit /actually/
fixes haskell-crypto#108, having been tested on an affected architecture :)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants