diff mbox

[COMMITTED] test-skeleton: Support temporary files without memory leaks [BZ#18333]

Message ID CAMe9rOpdNDsXxcwRcpi2MxuoRhA0H85HHWrPXbUAv=3sv2BTzA@mail.gmail.com
State Committed
Delegated to: David Miller
Headers show

Commit Message

H.J. Lu April 27, 2015, 5:01 p.m. UTC
On Mon, Apr 27, 2015 at 7:25 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 04/26/2015 03:50 PM, H.J. Lu wrote:
>> On Fri, Apr 24, 2015 at 8:36 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 02/18/2015 11:46 AM, Florian Weimer wrote:
>>>> On 02/16/2015 11:51 PM, Paul Eggert wrote:
>>>>> Florian Weimer wrote:
>>>>>> So I'm not sure what to do here.  Get rid of the alloca?  That's going
>>>>>> to be more difficult to review.
>>>>>
>>>>> I haven't read the code carefully, but if the only reason for the alloca
>>>>> is to have a temporary string that one can munge by storing '\0' bytes
>>>>> at strategic locations, then I presume that one could rewrite the code
>>>>> to avoid the need to make a temporary copy,
>>>>
>>>> Indeed.  I introduced __tzstring_len to avoid the need for the copy, and
>>>> broke down __tzset_parse_tz into several smaller functions.  Hopefully,
>>>> the control flow is more transparent.
>>>
>>> I've committed this now, with a few whitespace fixes.
>>>
>>
>> This caused:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=18333
>
> Fixed thusly.  I didn't see the crash in my original testing.

The bug is exposed by x32 with

 if (sizeof (time_t) == 8 && trans_width == 8)
    {

I am testing this patch and will check it in after testing on x32.

Comments

Florian Weimer April 27, 2015, 5:43 p.m. UTC | #1
On 04/27/2015 07:01 PM, H.J. Lu wrote:
> On Mon, Apr 27, 2015 at 7:25 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 04/26/2015 03:50 PM, H.J. Lu wrote:
>>> On Fri, Apr 24, 2015 at 8:36 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>>> On 02/18/2015 11:46 AM, Florian Weimer wrote:
>>>>> On 02/16/2015 11:51 PM, Paul Eggert wrote:
>>>>>> Florian Weimer wrote:
>>>>>>> So I'm not sure what to do here.  Get rid of the alloca?  That's going
>>>>>>> to be more difficult to review.
>>>>>>
>>>>>> I haven't read the code carefully, but if the only reason for the alloca
>>>>>> is to have a temporary string that one can munge by storing '\0' bytes
>>>>>> at strategic locations, then I presume that one could rewrite the code
>>>>>> to avoid the need to make a temporary copy,
>>>>>
>>>>> Indeed.  I introduced __tzstring_len to avoid the need for the copy, and
>>>>> broke down __tzset_parse_tz into several smaller functions.  Hopefully,
>>>>> the control flow is more transparent.
>>>>
>>>> I've committed this now, with a few whitespace fixes.
>>>>
>>>
>>> This caused:
>>>
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=18333
>>
>> Fixed thusly.  I didn't see the crash in my original testing.
> 
> The bug is exposed by x32 with
> 
>  if (sizeof (time_t) == 8 && trans_width == 8)
>     {
> 
> I am testing this patch and will check it in after testing on x32.

Thanks for taking care of this issue.
diff mbox

Patch

diff --git a/time/tzfile.c b/time/tzfile.c
index 46d4fc7..57abbb8 100644
--- a/time/tzfile.c
+++ b/time/tzfile.c
@@ -270,7 +270,8 @@  __tzfile_read (const char *file, size_t extra, char **extrap
)
       if (__glibc_unlikely (tzspec_len == 0 || tzspec_len - 1 < num_isgmt))
  goto lose;
       tzspec_len -= num_isgmt + 1;
-      if (__glibc_unlikely (SIZE_MAX - total_size < tzspec_len))
+      if (__glibc_unlikely (tzspec_len == 0
+    || SIZE_MAX - total_size < tzspec_len))
  goto lose;
     }