Bug 1439 - fixes for crypt/md5.? to permit syncing with gnulib
Summary: fixes for crypt/md5.? to permit syncing with gnulib
Status: RESOLVED INVALID
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: GOTO Masanori
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-08 11:03 UTC by Simon Josefsson
Modified: 2016-05-20 19:54 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Josefsson 2005-10-08 11:03:11 UTC
Hi!  I believe this patch would make it possible to fully sync your md5.?
against gnulib.

2005-10-08  Simon Josefsson  <jas@extundo.com>

	* crypt/md5.h: Synchronize with gnulib: Use stdint.h instead of
	computing local md5_uint32 etc types.  Add guards for
	__attribute__ and __THROW.  Add name space mapping for gnulib.
	Use uint32_t instead of md5_uint.

	* crypt/md5.c: Synchronize with gnulib: Include md5.h first.
	Replace md5_uint32 with uint32_t, and md5_uintptr with uintptr.
	Include C89 headers unconditionally.  Remove obsolete memcpy
	workaround.  Include unlocked-io.h if USE_UNLOCKED_IO, for gnulib.
	Define BLOCKSIZE early, and check the size.  Use C89 function
	prototypes rather than K&R.
	(md5_stream): Retry fread when it return 0 and set errno to EAGAIN
	and EWOULDBLOCK.
	(UNALIGNED_P) [__GNUC__ < 2]: Add better workaround for missing
	__alignof__.
	(md5_process_block): Add comment on how to reproduce values.


Index: md5.h
===================================================================
RCS file: /cvs/glibc/libc/crypt/md5.h,v
retrieving revision 1.6
diff -u -p -u -w -r1.6 md5.h
--- md5.h	5 Oct 2005 20:22:57 -0000	1.6
+++ md5.h	8 Oct 2005 10:55:45 -0000
@@ -23,72 +23,55 @@
 #define _MD5_H 1
 
 #include <stdio.h>
-
-#if defined HAVE_LIMITS_H || _LIBC
-# include <limits.h>
-#endif
-
-#define MD5_DIGEST_SIZE 16
-#define MD5_BLOCK_SIZE 64
-
-/* The following contortions are an attempt to use the C preprocessor
-   to determine an unsigned integral type that is 32 bits wide.  An
-   alternative approach is to use autoconf's AC_CHECK_SIZEOF macro, but
-   doing that would require that the configure script compile and *run*
-   the resulting executable.  Locally running cross-compiled executables
-   is usually not possible.  */
-
-#ifdef _LIBC
 # include <stdint.h>
-typedef uint32_t md5_uint32;
-typedef uintptr_t md5_uintptr;
-#else
-# if defined __STDC__ && __STDC__
-#  define UINT_MAX_32_BITS 4294967295U
+
+#ifndef __GNUC_PREREQ
+# if defined __GNUC__ && defined __GNUC_MINOR__
+#  define __GNUC_PREREQ(maj, min)					\
+  ((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min))
 # else
-#  define UINT_MAX_32_BITS 0xFFFFFFFF
+#  define __GNUC_PREREQ(maj, min) 0
 # endif
-
-/* If UINT_MAX isn't defined, assume it's a 32-bit type.
-   This should be valid for all systems GNU cares about because
-   that doesn't include 16-bit systems, and only modern systems
-   (that certainly have <limits.h>) have 64+-bit integral types.  */
-
-# ifndef UINT_MAX
-#  define UINT_MAX UINT_MAX_32_BITS
 # endif
 
-# if UINT_MAX == UINT_MAX_32_BITS
-   typedef unsigned int md5_uint32;
-# else
-#  if USHRT_MAX == UINT_MAX_32_BITS
-    typedef unsigned short md5_uint32;
-#  else
-#   if ULONG_MAX == UINT_MAX_32_BITS
-     typedef unsigned long md5_uint32;
+#ifndef __THROW
+# if defined __cplusplus && __GNUC_PREREQ (2,8)
+#  define __THROW	throw ()
 #   else
-     /* The following line is intended to evoke an error.
-        Using #error is not portable enough.  */
-     "Cannot determine unsigned 32-bit data type."
+#  define __THROW
 #   endif
 #  endif
+
+#ifndef __attribute__
+# if ! __GNUC_PREREQ (2,8) || __STRICT_ANSI__
+#  define __attribute__(x)
 # endif
-/* We have to make a guess about the integer type equivalent in size
-   to pointers which should always be correct.  */
-typedef unsigned long int md5_uintptr;
 #endif
 
+#ifndef _LIBC
+# define __md5_buffer md5_buffer
+# define __md5_finish_ctx md5_finish_ctx
+# define __md5_init_ctx md5_init_ctx
+# define __md5_process_block md5_process_block
+# define __md5_process_bytes md5_process_bytes
+# define __md5_read_ctx md5_read_ctx
+# define __md5_stream md5_stream
+#endif
+
+#define MD5_DIGEST_SIZE 16
+#define MD5_BLOCK_SIZE 64
+
 /* Structure to save state of computation between the single steps.  */
 struct md5_ctx
 {
-  md5_uint32 A;
-  md5_uint32 B;
-  md5_uint32 C;
-  md5_uint32 D;
-
-  md5_uint32 total[2];
-  md5_uint32 buflen;
-  char buffer[128] __attribute__ ((__aligned__ (__alignof__ (md5_uint32))));
+  uint32_t A;
+  uint32_t B;
+  uint32_t C;
+  uint32_t D;
+
+  uint32_t total[2];
+  uint32_t buflen;
+  char buffer[128] __attribute__ ((__aligned__ (__alignof__ (uint32_t_t))));
 };
 
 /*

--- md5.c	11 Aug 2005 11:57:49 +0200	1.5
+++ md5.c	08 Oct 2005 12:42:18 +0200	
@@ -25,18 +25,16 @@
 # include <config.h>
 #endif
 
-#include <sys/types.h>
+#include "md5.h"
 
-#if STDC_HEADERS || defined _LIBC
+#include <stddef.h>
 # include <stdlib.h>
 # include <string.h>
-#else
-# ifndef HAVE_MEMCPY
-#  define memcpy(d, s, n) (bcopy ((s), (d), (n)), (d))
-# endif
-#endif
+#include <sys/types.h>
 
-#include "md5.h"
+#if USE_UNLOCKED_IO
+# include "unlocked-io.h"
+#endif
 
 #ifdef _LIBC
 # include <endian.h>
@@ -61,6 +59,10 @@
 # define SWAP(n) (n)
 #endif
 
+#define BLOCKSIZE 4096
+#if BLOCKSIZE % 64 != 0
+# error "invalid BLOCKSIZE"
+#endif
 
 /* This array contains the bytes used to pad the buffer to the next
    64-byte boundary.  (RFC 1321, 3.1: Step 1)  */
@@ -70,8 +72,7 @@
 /* Initialize structure containing state of computation.
    (RFC 1321, 3.3: Step 3)  */
 void
-md5_init_ctx (ctx)
-     struct md5_ctx *ctx;
+md5_init_ctx (struct md5_ctx *ctx)
 {
   ctx->A = 0x67452301;
   ctx->B = 0xefcdab89;
@@ -88,14 +89,12 @@
    IMPORTANT: On some systems it is required that RESBUF is correctly
    aligned for a 32 bits value.  */
 void *
-md5_read_ctx (ctx, resbuf)
-     const struct md5_ctx *ctx;
-     void *resbuf;
-{
-  ((md5_uint32 *) resbuf)[0] = SWAP (ctx->A);
-  ((md5_uint32 *) resbuf)[1] = SWAP (ctx->B);
-  ((md5_uint32 *) resbuf)[2] = SWAP (ctx->C);
-  ((md5_uint32 *) resbuf)[3] = SWAP (ctx->D);
+md5_read_ctx (const struct md5_ctx *ctx, void *resbuf)
+{
+  ((uint32_t *) resbuf)[0] = SWAP (ctx->A);
+  ((uint32_t *) resbuf)[1] = SWAP (ctx->B);
+  ((uint32_t *) resbuf)[2] = SWAP (ctx->C);
+  ((uint32_t *) resbuf)[3] = SWAP (ctx->D);
 
   return resbuf;
 }
@@ -106,12 +105,10 @@
    IMPORTANT: On some systems it is required that RESBUF is correctly
    aligned for a 32 bits value.  */
 void *
-md5_finish_ctx (ctx, resbuf)
-     struct md5_ctx *ctx;
-     void *resbuf;
+md5_finish_ctx (struct md5_ctx *ctx, void *resbuf)
 {
   /* Take yet unprocessed bytes into account.  */
-  md5_uint32 bytes = ctx->buflen;
+  uint32_t bytes = ctx->buflen;
   size_t pad;
 
   /* Now count remaining bytes.  */
@@ -123,8 +120,8 @@
   memcpy (&ctx->buffer[bytes], fillbuf, pad);
 
   /* Put the 64-bit file length in *bits* at the end of the buffer.  */
-  *(md5_uint32 *) &ctx->buffer[bytes + pad] = SWAP (ctx->total[0] << 3);
-  *(md5_uint32 *) &ctx->buffer[bytes + pad + 4] = SWAP ((ctx->total[1] << 3) |
+  *(uint32_t *) &ctx->buffer[bytes + pad] = SWAP (ctx->total[0] << 3);
+  *(uint32_t *) &ctx->buffer[bytes + pad + 4] = SWAP ((ctx->total[1] << 3) |
 							(ctx->total[0] >> 29));
 
   /* Process last bytes.  */
@@ -137,12 +134,8 @@
    resulting message digest number will be written into the 16 bytes
    beginning at RESBLOCK.  */
 int
-md5_stream (stream, resblock)
-     FILE *stream;
-     void *resblock;
+md5_stream (FILE *stream, void *resblock)
 {
-  /* Important: BLOCKSIZE must be a multiple of 64.  */
-#define BLOCKSIZE 4096
   struct md5_ctx ctx;
   char buffer[BLOCKSIZE + 72];
   size_t sum;
@@ -160,27 +153,41 @@
       sum = 0;
 
       /* Read block.  Take care for partial reads.  */
-      do
+      while (1)
 	{
 	  n = fread (buffer + sum, 1, BLOCKSIZE - sum, stream);
 
 	  sum += n;
-	}
-      while (sum < BLOCKSIZE && n != 0);
-      if (n == 0 && ferror (stream))
-        return 1;
 
-      /* If end of file is reached, end the loop.  */
-      if (n == 0)
+	  if (sum == BLOCKSIZE)
 	break;
 
+	  if (n == 0)
+	    {
+	      /* Check for the error flag IFF N == 0, so that we don't
+		 exit the loop after a partial read due to e.g., EAGAIN
+		 or EWOULDBLOCK.  */
+	      if (ferror (stream))
+		return 1;
+	      goto process_partial_block;
+	    }
+
+	  /* We've read at least one byte, so ignore errors.  But always
+	     check for EOF, since feof may be true even though N > 0.
+	     Otherwise, we could end up calling fread after EOF.  */
+	  if (feof (stream))
+	    goto process_partial_block;
+	}
+
       /* Process buffer with BLOCKSIZE bytes.  Note that
 			BLOCKSIZE % 64 == 0
        */
       md5_process_block (buffer, BLOCKSIZE, &ctx);
     }
 
-  /* Add the last bytes if necessary.  */
+ process_partial_block:;
+
+  /* Process any remaining bytes.  */
   if (sum > 0)
     md5_process_bytes (buffer, sum, &ctx);
 
@@ -194,10 +201,7 @@
    output yields to the wanted ASCII representation of the message
    digest.  */
 void *
-md5_buffer (buffer, len, resblock)
-     const char *buffer;
-     size_t len;
-     void *resblock;
+md5_buffer (const char *buffer, size_t len, void *resblock)
 {
   struct md5_ctx ctx;
 
@@ -213,10 +217,7 @@
 
 
 void
-md5_process_bytes (buffer, len, ctx)
-     const void *buffer;
-     size_t len;
-     struct md5_ctx *ctx;
+md5_process_bytes (const void *buffer, size_t len, struct md5_ctx *ctx)
 {
   /* When we already have some bits in our internal buffer concatenate
      both inputs first.  */
@@ -249,9 +250,10 @@
 /* To check alignment gcc has an appropriate operator.  Other
    compilers don't.  */
 # if __GNUC__ >= 2
-#  define UNALIGNED_P(p) (((md5_uintptr) p) % __alignof__ (md5_uint32) != 0)
+#  define UNALIGNED_P(p) (((uintptr_t) p) % __alignof__ (uint32_t) != 0)
 # else
-#  define UNALIGNED_P(p) (((md5_uintptr) p) % sizeof (md5_uint32) != 0)
+#  define alignof(type) offsetof (struct { char c; type x; }, x)
+#  define UNALIGNED_P(p) (((size_t) p) % alignof (uint32_t) != 0)
 # endif
       if (UNALIGNED_P (buffer))
 	while (len > 64)
@@ -300,19 +302,16 @@
    It is assumed that LEN % 64 == 0.  */
 
 void
-md5_process_block (buffer, len, ctx)
-     const void *buffer;
-     size_t len;
-     struct md5_ctx *ctx;
-{
-  md5_uint32 correct_words[16];
-  const md5_uint32 *words = buffer;
-  size_t nwords = len / sizeof (md5_uint32);
-  const md5_uint32 *endp = words + nwords;
-  md5_uint32 A = ctx->A;
-  md5_uint32 B = ctx->B;
-  md5_uint32 C = ctx->C;
-  md5_uint32 D = ctx->D;
+md5_process_block (const void *buffer, size_t len, struct md5_ctx *ctx)
+{
+  uint32_t correct_words[16];
+  const uint32_t *words = buffer;
+  size_t nwords = len / sizeof (uint32_t);
+  const uint32_t *endp = words + nwords;
+  uint32_t A = ctx->A;
+  uint32_t B = ctx->B;
+  uint32_t C = ctx->C;
+  uint32_t D = ctx->D;
 
   /* First increment the byte count.  RFC 1321 specifies the possible
      length of the file up to 2^64 bits.  Here we only compute the
@@ -325,11 +324,11 @@
      the loop.  */
   while (words < endp)
     {
-      md5_uint32 *cwp = correct_words;
-      md5_uint32 A_save = A;
-      md5_uint32 B_save = B;
-      md5_uint32 C_save = C;
-      md5_uint32 D_save = D;
+      uint32_t *cwp = correct_words;
+      uint32_t A_save = A;
+      uint32_t B_save = B;
+      uint32_t C_save = C;
+      uint32_t D_save = D;
 
       /* First round: using the given function, the context and a constant
 	 the next context is computed.  Because the algorithms processing
@@ -356,6 +355,10 @@
 	 They are defined in RFC 1321 as
 
 	 T[i] = (int) (4294967296.0 * fabs (sin (i))), i=1..64
+
+	 Here is an equivalent invocation using Perl:
+
+	 perl -e 'foreach(1..64){printf "0x%08x\n", int (4294967296 * abs (sin $_))}'
        */
 
       /* Round 1.  */
Comment 1 Simon Josefsson 2005-10-11 18:41:36 UTC
(In reply to comment #0)
> Hi!  I believe this patch would make it possible to fully sync your md5.?
> against gnulib.

A small mistake, the line:

+  char buffer[128] __attribute__ ((__aligned__ (__alignof__ (uint32_t_t))));

should be:

+  char buffer[128] __attribute__ ((__aligned__ (__alignof__ (uint32_t))));

I.e., remove the final duplicate '_t'.

Thanks!
Comment 2 Simon Josefsson 2005-10-12 00:18:34 UTC
(In reply to comment #0)
> Hi!  I believe this patch would make it possible to fully sync your md5.?
> against gnulib.

Another modification, resulting from bug-gnulib review, this two lines in md5.h:

   IMPORTANT: On some systems it is required that RESBUF is correctly
   aligned for a 32 bits value.  */

should be replaced with:

   IMPORTANT: On some systems, RESBUF must be aligned to a 32-bit boundary. */

Thanks.
Comment 3 Ulrich Drepper 2005-10-14 06:29:22 UTC
And why should these changes be made?  The code is correct as is.
Comment 4 Simon Josefsson 2005-10-14 14:24:07 UTC
(In reply to comment #3)
> And why should these changes be made?  The code is correct as is.

Applying the patch to glibc would make it possible for gnulib to sync against
the glibc copy.  Rather than copy the file into gnulib and make gratitious
changes to it over time, we are trying to sync it with glibc so it will be
stable and to reduce maintainance.  I believe the same procedure is used for
several other files in glibc/gnulib already.

Thanks.
Comment 5 Ulrich Drepper 2005-10-16 08:08:36 UTC
Then change the gnulib code back.  I'm not willing to let the gnulib
"maintainers" decide about the code.  All those changes made for *NO* reason
just cause problems for glibc.  Stop that nonsense.
Comment 6 Ulrich Drepper 2006-04-25 18:23:46 UTC
No response in 6+ months.  Closing.