* misbehaviour with md5_process_bytes and maybe in optimization
@ 2011-09-23 14:32 Pierre Vittet
2011-09-23 15:13 ` Ian Lance Taylor
0 siblings, 1 reply; 4+ messages in thread
From: Pierre Vittet @ 2011-09-23 14:32 UTC (permalink / raw)
To: gcc; +Cc: Basile Starynkevitch, gcc-melt, Alexandre Lissy, sje
[-- Attachment #1: Type: text/plain, Size: 2007 bytes --]
Hello,
I recently asked for some help as I got a problem when using
md5_process_bytes (in libiberty/md5.c):
http://gcc.gnu.org/ml/gcc-help/2011-09/msg00126.html,
http://gcc.gnu.org/ml/gcc-help/2011-09/msg00127.html and it appears that
there is a bug in md5_process_bytes.
The bug can conduct to a miscomputed md5 result.
It tooks time to me to make the bug reproducible but I was finally able
to do so. The fact is that it only appears in very particular situation.
I have written a small gcc plugin, allowing to reproduce it (see
attachment).
The bad news is that the bug only appears when use libiberty compiled in
-g -O0 (it works well with -O2). It is quite sad, because It could means
another bug in an optimization function.
I have attached a README which detail how to use the plugin and how to
explain the bug. I have tried to explain as good as possible (and I
apologize for my very bad english).
The bug appears when:
1) We use libiberty compiled with -O0
2) We first call md5_process_bytes with a less than 64 bits buffer (we
call his size len1).
3) We make a new call of md5_process_bytes with a buffer which has a
size len2 such as:
len2 > 127 + 65 (so test in line 228 of md5.C will be true)
128 -len1 != Mulint with Mulint % __alignof__ (md5_uint32) != 0 (so
condition on line 238 is true)
len2 - (128 - len1) = Mul64 and Mul64 such as Mul %64=0 (so the loop of
line 239 is broken with len = 64, this leads to the bug as, line 249,
(len & ~63) = 64 and we shift the buffer without processing the data).
Please, can you reproduce the bug? Is there any useful informations I
can add? Must I contact somebody from libiberty (I don't know the status
of this library (is this part of gcc or from another project?)).
I already sent a patch correcting this issue (it does not correct the
fact that we don't get the bug with an optimised libiberty):
http://gcc.gnu.org/ml/gcc-patches/2011-09/msg01098.html. It has not been
reviewed, could someone reviews this?
Thanks!
Pierre Vittet
[-- Attachment #2: md5sum_plugin.tar.gz --]
[-- Type: application/gzip, Size: 2548 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: misbehaviour with md5_process_bytes and maybe in optimization
2011-09-23 14:32 misbehaviour with md5_process_bytes and maybe in optimization Pierre Vittet
@ 2011-09-23 15:13 ` Ian Lance Taylor
2011-09-23 18:17 ` Pierre Vittet
0 siblings, 1 reply; 4+ messages in thread
From: Ian Lance Taylor @ 2011-09-23 15:13 UTC (permalink / raw)
To: Pierre Vittet; +Cc: gcc, Basile Starynkevitch, gcc-melt, Alexandre Lissy, sje
Pierre Vittet <piervit@pvittet.com> writes:
> The bug appears when:
> 1) We use libiberty compiled with -O0
> 2) We first call md5_process_bytes with a less than 64 bits buffer (we
> call his size len1).
> 3) We make a new call of md5_process_bytes with a buffer which has a
> size len2 such as:
> len2 > 127 + 65 (so test in line 228 of md5.C will be true)
> 128 -len1 != Mulint with Mulint % __alignof__ (md5_uint32) != 0 (so
> condition on line 238 is true)
> len2 - (128 - len1) = Mul64 and Mul64 such as Mul %64=0 (so the loop of
> line 239 is broken with len = 64, this leads to the bug as, line 249,
> (len & ~63) = 64 and we shift the buffer without processing the data).
The line numbers you mention do not correspond to any version of
libiberty/md5.c that I can see. Can you list the exact line for each
line number you mention, so that your explanation is easier to follow?
Thanks.
Ian
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: misbehaviour with md5_process_bytes and maybe in optimization
2011-09-23 15:13 ` Ian Lance Taylor
@ 2011-09-23 18:17 ` Pierre Vittet
2011-09-23 19:20 ` Ian Lance Taylor
0 siblings, 1 reply; 4+ messages in thread
From: Pierre Vittet @ 2011-09-23 18:17 UTC (permalink / raw)
To: Ian Lance Taylor
Cc: gcc, Basile Starynkevitch, gcc-melt, Alexandre Lissy, sje
Thanks for your interest,
I just checked revision 179127 of GCC. Last revision is 177700, it has
not been change for 6 weeks.
My file is the same as this one:
http://gcc.gnu.org/viewcvs/trunk/libiberty/md5.c?revision=177700&view=markup
in libiberty/md5.c, function md5_process_bytes start line 203.
On 23/09/2011 17:13, Ian Lance Taylor wrote:
> Pierre Vittet <piervit@pvittet.com> writes:
>
>> The bug appears when:
>> 1) We use libiberty compiled with -O0
>> 2) We first call md5_process_bytes with a less than 64 bits buffer (we
>> call his size len1).
>> 3) We make a new call of md5_process_bytes with a buffer which has a
>> size len2 such as:
>> len2 > 127 + 65 (so test in line 228 of md5.C will be true)
line 228 is the following: if (len > 64)
>> 128 -len1 != Mulint with Mulint % __alignof__ (md5_uint32) != 0 (so
>> condition on line 238 is true)
line 238 is the following: if (UNALIGNED_P (buffer))
>> len2 - (128 - len1) = Mul64 and Mul64 such as Mul %64=0 (so the loop of
>> line 239 is broken with len = 64, this leads to the bug as, line 249,
>> (len & ~63) = 64 and we shift the buffer without processing the data).
line 239 is the following: while (len > 64)
line 249: buffer = (const void *) ((const char *) buffer + (len & ~63));
>
> The line numbers you mention do not correspond to any version of
> libiberty/md5.c that I can see. Can you list the exact line for each
> line number you mention, so that your explanation is easier to follow?
> Thanks.
I give about the same explanation in the README (which is in the
attached archive of my previous mail) but I does not use line number but
direct quote of the code. It mights be more easy to try the plugin with
gdb but it needs to compile libiberty.a with -O0.
>
> Ian
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: misbehaviour with md5_process_bytes and maybe in optimization
2011-09-23 18:17 ` Pierre Vittet
@ 2011-09-23 19:20 ` Ian Lance Taylor
0 siblings, 0 replies; 4+ messages in thread
From: Ian Lance Taylor @ 2011-09-23 19:20 UTC (permalink / raw)
To: Pierre Vittet
Cc: gcc, Basile Starynkevitch, gcc-melt, Alexandre Lissy, sje, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2504 bytes --]
Pierre Vittet <piervit@pvittet.com> writes:
> Thanks for your interest,
>
> I just checked revision 179127 of GCC. Last revision is 177700, it has
> not been change for 6 weeks.
>
> My file is the same as this one:
> http://gcc.gnu.org/viewcvs/trunk/libiberty/md5.c?revision=177700&view=markup
>
> in libiberty/md5.c, function md5_process_bytes start line 203.
>
> On 23/09/2011 17:13, Ian Lance Taylor wrote:
>> Pierre Vittet <piervit@pvittet.com> writes:
>>
>>> The bug appears when:
>>> 1) We use libiberty compiled with -O0
>>> 2) We first call md5_process_bytes with a less than 64 bits buffer (we
>>> call his size len1).
>>> 3) We make a new call of md5_process_bytes with a buffer which has a
>>> size len2 such as:
>>> len2 > 127 + 65 (so test in line 228 of md5.C will be true)
> line 228 is the following: if (len > 64)
>>> 128 -len1 != Mulint with Mulint % __alignof__ (md5_uint32) != 0 (so
>>> condition on line 238 is true)
> line 238 is the following: if (UNALIGNED_P (buffer))
>>> len2 - (128 - len1) = Mul64 and Mul64 such as Mul %64=0 (so the loop of
>>> line 239 is broken with len = 64, this leads to the bug as, line 249,
>>> (len & ~63) = 64 and we shift the buffer without processing the data).
>
> line 239 is the following: while (len > 64)
> line 249: buffer = (const void *) ((const char *) buffer + (len & ~63));
>>
>> The line numbers you mention do not correspond to any version of
>> libiberty/md5.c that I can see. Can you list the exact line for each
>> line number you mention, so that your explanation is easier to follow?
>> Thanks.
>
> I give about the same explanation in the README (which is in the
> attached archive of my previous mail) but I does not use line number but
> direct quote of the code. It mights be more easy to try the plugin with
> gdb but it needs to compile libiberty.a with -O0.
Thanks, I think I have it sorted out now.
It does not happen on x86 glibc-based systems at -O2 because at -O2
<string.h> #defines STRING_ARCH_unaligned, so the problematic code is
not compiled or executed.
The error was introduced by this change:
2005-07-03 Steve Ellcey <sje@cup.hp.com>
PR other/13906
* md5.c (md5_process_bytes): Check alignment.
Thanks for noticing this problem, analyzing it, and reporting it.
I committed this patch to mainline to fix the problem. Bootstrapped on
x86_64-unknown-linux-gnu.
Ian
2011-09-23 Ian Lance Taylor <iant@google.com>
* md5.c (md5_process_bytes): Correct handling of unaligned
buffer.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 951 bytes --]
Index: md5.c
===================================================================
--- md5.c (revision 179127)
+++ md5.c (working copy)
@@ -1,6 +1,6 @@
/* md5.c - 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 Free Software Foundation, Inc.
+ Copyright (C) 1995, 1996, 2011 Free Software Foundation, Inc.
NOTE: This source is derived from an old version taken from the GNU C
Library (glibc).
@@ -245,9 +245,11 @@ md5_process_bytes (const void *buffer, s
}
else
#endif
- md5_process_block (buffer, len & ~63, ctx);
- buffer = (const void *) ((const char *) buffer + (len & ~63));
- len &= 63;
+ {
+ md5_process_block (buffer, len & ~63, ctx);
+ buffer = (const void *) ((const char *) buffer + (len & ~63));
+ len &= 63;
+ }
}
/* Move remaining bytes in internal buffer. */
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-09-23 19:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-23 14:32 misbehaviour with md5_process_bytes and maybe in optimization Pierre Vittet
2011-09-23 15:13 ` Ian Lance Taylor
2011-09-23 18:17 ` Pierre Vittet
2011-09-23 19:20 ` Ian Lance Taylor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).