Re: [BZ #14090] Fix md5/sha512 with large block sizes

[Andreas Jaeger <aj at suse dot com>]

On 08/15/2012 08:00 PM, Roland McGrath wrote:
> > +// MD5 sum values of zero-filled blocks of specified sizes.
>
> We don't use // comments.
>
> > +static struct test_data_s
>
> Add const.
>
> > +  else
> > +      __md5_process_bytes (buf, len, &ctx);
>
> Indentation looks wrong.

Fixed all, thanks for the review.

> > +  /* This test will not work with 32-bit size_t, so let it succeed there.  */
> > +  if (sizeof (void *) == 4)
> > +    return 0;
> > +
> > +  buf = mmap (0, CONST_10G, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>
> Does this compile without warning on 32-bit machines?

The new appended version does ;)

> > --- a/crypt/sha512.c
> > +++ b/crypt/sha512.c
>
> There's no test for the sha512 change.  I don't recall, was it determined
> that there is no test that runs in a reasonable time?

Yes, exactly. Paul said in this thread "It's not practical to test for that.
It'd need a machine where size_t is wider than 64 bits,
along with a single buffer with at least 16 EiB."


Andreas

commit a69d6ba7f54ba76810ba1f66f0422aaddb711f7e
Author: Andreas Jaeger <aj@suse.de>
Date:   Wed Aug 15 19:40:55 2012 +0200

    Fix BZ#14090 - md5/sha512 with large sizes

    2012-08-15  Andreas Jaeger  <aj@suse.de>

    	[BZ #14090]
    	* crypt/md5test-giant.c: New test, based on test supplied by
    	* Serge
    	Belyshev <belyshev@depni.sinp.msu.ru>.
    	* crypt/Makefile (xtests): Add md5test-giant.
    	* crypt/Makefile ($(objpfx)md5test-giant): Add.

    2012-08-15 Paul Eggert <eggert@cs.ucla.edu>

	[BZ #14090]
        * crypt/md5.c (md5_process_block): Don't assume the buffer
        length is less than 2**32.
    	* crypt/sha512.c (sha512_process_block): Don't assume the buffer
        length is less than 2**64.

diff --git a/crypt/Makefile b/crypt/Makefile
index d276092..ccca688 100644
--- a/crypt/Makefile
+++ b/crypt/Makefile
@@ -46,7 +46,12 @@ libcrypt-routines += md5 sha256 sha512

 tests += md5test sha256test sha512test

+# The test md5test-giant uses up to 400 MB of RSS and runs on a fast
+# machine over a minute.
+xtests = md5test-giant
+
 $(objpfx)md5test: $(objpfx)md5.o
+$(objpfx)md5test-giant: $(objpfx)md5.o
 $(objpfx)sha256test: $(objpfx)sha256.o
 $(objpfx)sha512test: $(objpfx)sha512.o
 endif
diff --git a/crypt/md5.c b/crypt/md5.c
index 292bee1..3d2e79b 100644
--- a/crypt/md5.c
+++ b/crypt/md5.c
@@ -1,7 +1,6 @@
 /* Functions to compute MD5 message digest of files or memory blocks.
    according to the definition of MD5 in RFC 1321 from April 1992.
-   Copyright (C) 1995,1996,1997,1999,2000,2001,2005,2011
-	Free Software Foundation, Inc.
+   Copyright (C) 1995-2012 Free Software Foundation, Inc.
    This file is part of the GNU C Library.

    The GNU C Library is free software; you can redistribute it and/or
@@ -312,13 +311,13 @@ md5_process_block (buffer, len, ctx)
   md5_uint32 B = ctx->B;
   md5_uint32 C = ctx->C;
   md5_uint32 D = ctx->D;
+  md5_uint32 lolen = len;

   /* First increment the byte count.  RFC 1321 specifies the possible
      length of the file up to 2^64 bits.  Here we only compute the
      number of bytes.  Do a double word increment.  */
-  ctx->total[0] += len;
-  if (ctx->total[0] < len)
-    ++ctx->total[1];
+  ctx->total[0] += lolen;
+  ctx->total[1] += (len >> 31 >> 1) + (ctx->total[0] < lolen);

   /* Process all bytes in the buffer with 64 bytes in each round of
      the loop.  */
diff --git a/crypt/md5test-giant.c b/crypt/md5test-giant.c
new file mode 100644
index 0000000..c73fca7
--- /dev/null
+++ b/crypt/md5test-giant.c
@@ -0,0 +1,135 @@
+/* Testcase for http://sourceware.org/bugzilla/show_bug.cgi?id=14090.
+   Copyright (C) 2012 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published
+   by the Free Software Foundation; version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+
+#include "md5.h"
+
+  /* This test will not work with 32-bit size_t, so let it succeed 
there.  */
+#if __WORDSIZE == 32
+static int
+do_test (void)
+{
+  return 0;
+}
+#else
+
+# define CONST_2G  0x080000000
+# define CONST_10G 0x280000000
+
+/* MD5 sum values of zero-filled blocks of specified sizes.  */
+static const struct test_data_s
+{
+  const char ref[16];
+  size_t len;
+} test_data[] =
+  {
+    { "\xd4\x1d\x8c\xd9\x8f\x00\xb2\x04\xe9\x80\x09\x98\xec\xf8\x42\x7e",
+      0x000000000 },
+    { "\xa9\x81\x13\x0c\xf2\xb7\xe0\x9f\x46\x86\xdc\x27\x3c\xf7\x18\x7e",
+      0x080000000 },
+    { "\xc9\xa5\xa6\x87\x8d\x97\xb4\x8c\xc9\x65\xc1\xe4\x18\x59\xf0\x34",
+      0x100000000 },
+    { "\x58\xcf\x63\x8a\x73\x3f\x91\x90\x07\xb4\x28\x7c\xf5\x39\x6d\x0c",
+      0x180000000 },
+    { "\xb7\x70\x35\x1f\xad\xae\x5a\x96\xbb\xaf\x97\x02\xed\x97\xd2\x8d",
+      0x200000000 },
+    { "\x2d\xd2\x6c\x4d\x47\x99\xeb\xd2\x9f\xa3\x1e\x48\xd4\x9e\x8e\x53",
+      0x280000000 },
+};
+
+static int
+report (const char *id, const char *md5, size_t len, const char *ref)
+{
+  if (memcmp (md5, ref, 16))
+    {
+      printf ("test %s with size %zd failed\n", id, len);
+      return 1;
+    }
+  return 0;
+}
+
+/* Test md5 in a single md5_process_bytes call.  */
+static int
+test_single (void *buf, size_t len, const char *ref)
+{
+  char sum[16];
+  struct md5_ctx ctx;
+
+  __md5_init_ctx (&ctx);
+  __md5_process_bytes (buf, len, &ctx);
+  __md5_finish_ctx (&ctx, sum);
+
+  return report ("single", sum, len, ref);
+}
+
+/* Test md5 with two md5_process_bytes calls to trigger a
+   different path in md5_process_block for sizes > 2 GB.  */
+static int
+test_double (void *buf, size_t len, const char *ref)
+{
+  char sum[16];
+  struct md5_ctx ctx;
+
+  __md5_init_ctx (&ctx);
+  if (len >= CONST_2G)
+    {
+      __md5_process_bytes (buf, CONST_2G, &ctx);
+      __md5_process_bytes (buf + CONST_2G, len - CONST_2G, &ctx);
+    }
+  else
+    __md5_process_bytes (buf, len, &ctx);
+
+  __md5_finish_ctx (&ctx, sum);
+
+  return report ("double", sum, len, ref);
+}
+
+
+static int
+do_test (void)
+{
+  void *buf;
+  unsigned int j;
+  int result = 0;
+
+  buf = mmap64 (0, CONST_10G, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, 
-1, 0);
+  if (buf == MAP_FAILED)
+    {
+      puts ("Could not allocate 10 GB via mmap, skipping test.");
+      return 0;
+    }
+
+  for (j = 0; j < sizeof (test_data) / sizeof (struct test_data_s); j++)
+    {
+      if (test_single (buf, test_data[j].len, test_data[j].ref))
+	result = 1;
+      if (test_double (buf, test_data[j].len, test_data[j].ref))
+	result = 1;
+    }
+
+  return result;
+}
+#endif
+
+/* This needs on a fast machine 90s.  */
+#define TIMEOUT 180
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/crypt/sha512.c b/crypt/sha512.c
index 6e531c5..bec7bb3 100644
--- a/crypt/sha512.c
+++ b/crypt/sha512.c
@@ -1,6 +1,6 @@
 /* Functions to compute SHA512 message digest of files or memory blocks.
    according to the definition of SHA512 in FIPS 180-2.
-   Copyright (C) 2007, 2011 Free Software Foundation, Inc.
+   Copyright (C) 2007-2012 Free Software Foundation, Inc.
    This file is part of the GNU C Library.

    The GNU C Library is free software; you can redistribute it and/or
@@ -123,9 +123,10 @@ sha512_process_block (const void *buffer, size_t 
len, struct sha512_ctx *ctx)
 #ifdef USE_TOTAL128
   ctx->total128 += len;
 #else
-  ctx->total[TOTAL128_low] += len;
-  if (ctx->total[TOTAL128_low] < len)
-    ++ctx->total[TOTAL128_high];
+  uint64_t lolen = len;
+  ctx->total[TOTAL128_low] += lolen;
+  ctx->total[TOTAL128_high] += ((len >> 63 >> 1)
+				+ (ctx->total[TOTAL128_low] < lolen));
 #endif

   /* Process all bytes in the buffer with 128 bytes in each round of

--
 Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
  SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
    GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126