libcrypt/md5.c: a few statics can be auto variables; strlen("const") is silly
will.newton at gmail.com
Thu Jun 12 10:14:07 UTC 2008
On Thu, Jun 12, 2008 at 8:08 AM, Denys Vlasenko
<vda.linux at googlemail.com> wrote:
> On Wednesday 11 June 2008 17:32, Will Newton wrote:
>> >> This looks like it should be safe. Although it might be worth finding
>> >> the original author and asking why it is done this way, following the
>> >> Debian openssl incident. ;-)
>> > openssl incident? Did I miss some fun? :)
>> This is a fairly good summary:
>> The general lesson being: be very careful when modifying crypto code.
> I'd place the blame on the author. He wrote unreadable code.
> Unfortunately, it's far too common.
>> btw I tested the strlen change and this code:
>> #include <string.h>
>> static const unsigned char __md5__magic = "$1$";
>> int foo(void)
>> return strlen(__md5__magic);
>> compiles to this:
>> Disassembly of section .text:
>> 00000000 <foo>:
>> 0: 55 push %ebp
>> 1: 89 e5 mov %esp,%ebp
>> 3: b8 03 00 00 00 mov $0x3,%eax
>> 8: 5d pop %ebp
>> 9: c3 ret
> Don't place too much faith in gcc, that would be a mistake.
> Piling up a bit more code around, and it will fail to optimize
> it completely.
> This is the patch wich ONLY replaces strlen("const") with
> sizeof("const")-1. Let's check assembly, do we see strlen
> in original version?
What compiler flags were used? It's possible to disable this
optimization in gcc.
Maybe replace strlen with __builtin_strlen?
More information about the uClibc