Re: Fix sha functions on sparc

[Jim Meyering]

"Tom \"spot\" Callaway" <address@hidden> wrote:
> In porting Fedora to SPARC, we've noticed that the SHA functions in
> coreutils SIGBUS on sparc. Attached is a patch which resolves the
> failure on sparc. I've tested this patch on sparc, sparc64, and x86_64,
> with no test regressions as a result.

Hi Tom,

Thanks for the report and patch!
Out of curiosity, did coreutils' own tests fail with SIGBUS?

I can't use your patch as-is, since it would change the API, e.g.,

  diff -up coreutils-6.10/lib/sha256.h.BAD coreutils-6.10/lib/sha256.h
  ...
  -  uint32_t buffer[32];
  +  char buffer[128];

so I tried to accomplish the same thing, but with no API change.
This patch should do the job, but I haven't tested it.
Would you please let me know if it passes your tests?

[ By the way, this patch is relative to gnulib, because
  I moved the sha256 and sha512 modules from coreutils into
  gnulib just a couple weeks ago. ]

>From e341e7caaea9a3fd5f311195b023af8d280944fa Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 26 May 2008 16:25:28 +0200
Subject: [PATCH] avoid unaligned access errors, e.g., on sparc

* lib/sha512.c (sha512_conclude_ctx): Use set_uint64 rather than
direct access through a possibly-unaligned uint64* pointer.
* lib/sha256.c (sha256_conclude_ctx): Use set_uint32 rather than
direct access through a possibly-unaligned uint32* pointer.
Prompted by this patch from Tom "spot" Callaway:
http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/13638
---
 ChangeLog    |    8 ++++++++
 lib/sha256.c |   10 +++++++---
 lib/sha512.c |   12 ++++++++----
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 32655cb..9aea4e6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2008-05-26  Jim Meyering  <address@hidden>

+       avoid unaligned access errors, e.g., on sparc
+       * lib/sha512.c (sha512_conclude_ctx): Use set_uint64 rather than
+       direct access through a possibly-unaligned uint64* pointer.
+       * lib/sha256.c (sha256_conclude_ctx): Use set_uint32 rather than
+       direct access through a possibly-unaligned uint32* pointer.
+       Prompted by this patch from Tom "spot" Callaway:
+       http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/13638
+
        sha512.c: fix typo in comment
        * lib/sha512.c (sha512_conclude_ctx): Length is 128-bit, not 64-bit.

diff --git a/lib/sha256.c b/lib/sha256.c
index 4a632c9..a1362ca 100644
--- a/lib/sha256.c
+++ b/lib/sha256.c
@@ -134,9 +134,13 @@ sha256_conclude_ctx (struct sha256_ctx *ctx)
   if (ctx->total[0] < bytes)
     ++ctx->total[1];

-  /* Put the 64-bit file length in *bits* at the end of the buffer.  */
-  ctx->buffer[size - 2] = SWAP ((ctx->total[1] << 3) | (ctx->total[0] >> 29));
-  ctx->buffer[size - 1] = SWAP (ctx->total[0] << 3);
+  /* Put the 64-bit file length in *bits* at the end of the buffer.
+     Use set_uint32 rather than a simple assignment, to avoid risk of
+     unaligned access.  */
+  set_uint32 ((char *) &ctx->buffer[size - 2],
+             SWAP ((ctx->total[1] << 3) | (ctx->total[0] >> 29)));
+  set_uint32 ((char *) &ctx->buffer[size - 1],
+             SWAP (ctx->total[0] << 3));

   memcpy (&((char *) ctx->buffer)[bytes], fillbuf, (size - 2) * 4 - bytes);

diff --git a/lib/sha512.c b/lib/sha512.c
index 66cfaa9..261a7bb 100644
--- a/lib/sha512.c
+++ b/lib/sha512.c
@@ -141,10 +141,14 @@ sha512_conclude_ctx (struct sha512_ctx *ctx)
   if (u64lt (ctx->total[0], u64lo (bytes)))
     ctx->total[1] = u64plus (ctx->total[1], u64lo (1));

-  /* Put the 128-bit file length in *bits* at the end of the buffer.  */
-  ctx->buffer[size - 2] = SWAP (u64or (u64shl (ctx->total[1], 3),
-                                      u64shr (ctx->total[0], 61)));
-  ctx->buffer[size - 1] = SWAP (u64shl (ctx->total[0], 3));
+  /* Put the 128-bit file length in *bits* at the end of the buffer.
+     Use set_uint64 rather than a simple assignment, to avoid risk of
+     unaligned access.  */
+  set_uint64 ((char *) &ctx->buffer[size - 2],
+             SWAP (u64or (u64shl (ctx->total[1], 3),
+                          u64shr (ctx->total[0], 61))));
+  set_uint64 ((char *) &ctx->buffer[size - 1],
+             SWAP (u64shl (ctx->total[0], 3)));

   memcpy (&((char *) ctx->buffer)[bytes], fillbuf, (size - 2) * 8 - bytes);

--
1.5.5.1.383.g8078b