[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aad2942dedb6797db72ec0f0ae3319e6@smtp.hushmail.com>
Date: Wed, 17 Apr 2013 01:08:01 +0200
From: magnum <john.magnum@...hmail.com>
To: john-dev@...ts.openwall.com
Subject: Re: Segfaults probably caused by DEBUG code in memory.c
On 17 Apr, 2013, at 0:23 , Frank Dittrich <frank_dittrich@...mail.com> wrote:
> On 04/17/2013 12:14 AM, magnum wrote:
>> On 16 Apr, 2013, at 23:37 , Frank Dittrich <frank_dittrich@...mail.com> wrote:
>>> On 04/16/2013 09:56 PM, jfoug wrote:
>>>> In hindsight, in debug mode, you might have gotten the same results by doing
>>>> this:
>>>>
>>>> #if DEBUG
>>>> #undef MEM_ALLOC_SIZE
>>>> #define MEM_ALLOC_SIZE 0
>>>> #endif
>>>
>>> I really like this solution, because it leaves mem_alloc_tiny (which is
>>> complex enough as it is) unchanged.
>>>
>>> May be even better is to do this in memory.h directly:
>>>
>>> #if DEBUG
>>> /* turn mem_alloc_tiny() into a normal alloc,
>>> * to better track problems.
>>> */
>>> #define MEM_ALLOC_SIZE 0
>>> #else
>>> #define MEM_ALLOC_SIZE 0x10000
>>
>> I bought your arguments and did this, even commited it without testing (silly me). But it segfaults for me in unstable, and hangs in bleeding, both in dynamic_1.
>>
>> I reverted it. You can try for yourself, unstable is 3b53da3 and bleeding is 83a9e10.
>
> My fault:
>
> dynamic_fmt.c:1800: pSaltDataBuf = pNextSaltDataBuf =
> mem_alloc_tiny(MEM_ALLOC_SIZE*6, MEM_ALIGN_NON
> dynamic_fmt.c:1801: nSaltDataBuf = MEM_ALLOC_SIZE*6;
>
> I should have checked the usage of MEM_ALLOC_SIZE before suggesting the
> change.
Aha... I thought it was some worse issue within mem_alloc_tiny and the remainder of last alloc. In bleeding (they do differ) it really is. Here's the bleeding version (actually the bleeding core version):
31 void *mem_alloc_tiny(size_t size, size_t align)
32 {
33 static char *buffer = NULL;
34 static size_t bufree = 0;
35 size_t mask;
36 char *p;
37
38 #if ARCH_ALLOWS_UNALIGNED
39 if (mem_saving_level > 2)
40 align = MEM_ALIGN_NONE;
41 #endif
42
43 mask = align - 1;
44
45 do {
46 if (buffer) {
47 size_t need =
48 size + mask - (((size_t)buffer + mask) & mask);
49 if (bufree >= need) {
50 p = buffer;
51 p += mask;
52 p -= (size_t)p & mask;
53 bufree -= need;
54 buffer = p + size;
55 return p;
56 }
57 }
58
59 if (size + mask > MEM_ALLOC_SIZE ||
60 bufree > MEM_ALLOC_MAX_WASTE)
61 break;
62
63 buffer = mem_alloc(MEM_ALLOC_SIZE);
64 bufree = MEM_ALLOC_SIZE;
65 } while (1);
66
67 p = mem_alloc(size + mask);
68 p += mask;
69 p -= (size_t)p & mask;
70 return p;
71 }
Now, if MEM_ALLOC_SIZE is 0 we will allocate 0 bytes in line 63, and never get out of that loop.
In unstable this does not happen but there is another problem (even if not doing the #ifdef in memory.h): If we allocate something with a large alignment like 64 but malloc() happens to return an already aligned buffer, we'll end up having bufree==64. At next call and providing we ask for <= 64 bytes we will get memory from that. This contradicts the point with my -DDEBUG behaviour.
I suggest we keep the current -DDEBUG code as-is for now.
magnum
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
