191703 - partially initialized struct tm leads to segv

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P1)

Description

[Nicholas Maniscalco]

User-Agent:       Mozilla/5.0 (X11; U; NetBSD i386; en-US; rv:1.2.1) Gecko/20021223
Build Identifier: Mozilla/5.0 (X11; U; NetBSD i386; en-US; rv:1.2.1) Gecko/20021223

PR_FormatTime() from nsprpub/pr/src/misc/prtime.c calls strftime() on line 1676
with a partially initialized tm struct.

On NetBSD (and others), struct tm has a char* field calld tm_zone.  If this
field is non-null, strftime() thinks it points to valid data.  If tm_zone is
null, strftime() assumes the local time zone.


Reproducible: Always

Steps to Reproduce:
Because of the nature of this bug it may not be easily reproducable.

I've been reproducing this using phoenix 0.5.  I ...

* click on a link to a file, get prompted on whether/where to save it
* just click save
* seg fault occurs

Actual Results:  
seg fault

Expected Results:  
successfully save file, return to the page I was browsing

If I have assigned this to the wrong component, please tell me how I can
determine the right component.

(gdb) bt
#0  0x48546c95 in strftime () from /usr/lib/libc.so.12
#1  0x48546b13 in strftime () from /usr/lib/libc.so.12
#2  0x48545fba in strftime () from /usr/lib/libc.so.12
#3  0x4822443c in PR_FormatTime (buf=0xbfbfc1c0 "32", buflen=40, 
    fmt=0x4823451c "%Z", tm=0xbfbfc330) at prtime.c:1676
(gdb) frame 3
#3  0x4822443c in PR_FormatTime (buf=0xbfbfc1c0 "32", buflen=40, 
    fmt=0x4823451c "%Z", tm=0xbfbfc330) at prtime.c:1676
1676          return 0;
(gdb) disp a
2: a = {tm_sec = 32, tm_min = 11, tm_hour = 20, tm_mday = 2, tm_mon = 1, 
  tm_year = 103, tm_wday = 0, tm_yday = 32, tm_isdst = 0, 
  tm_gmtoff = 1210282000, 
  tm_zone = 0x2 <Error reading address 0x2: Invalid argument>}
(gdb) 

Notice this value of tm_zone, 0x2.  Simply initializing a to 0 will fix
this bug.  Patch attached.

Comment 1

[Nicholas Maniscalco]

Attachment: patch to prtime.c; created with diff -c

Comment 2

[Boris Zbarsky [:bzbarsky]]

NSPR bug.

Comment 3

[Wan-Teh Chang]

Attachment: wtc's proposed patch

We've run into this problem on other platforms.  This
patch uses our solution on NetBSD.  Could you try this
patch?

I think your solution may also work, but I'd need to
research it.  In any case, what I want to avoid is to
use both solutions.  We should pick one.

Comment 4

[Nicholas Maniscalco]

Wow, nice response time!

When I first ran into this problem I suspected that NetBSD was just left out of
the ifdef, so I added it so that mktime would be called.  In my tests, mktime
fails, returning -1.  That isn't what we want, right?

I really feel that initializing 'a' (a struct tm) is a smart thing to do in any
case.  In other words, is there a valid argument for *not* initializing 'a'?

Comment 5

[Nicholas Maniscalco]

Anything new on this bug?  Can we go with the 'initialized the struct' fix?

Comment 6

[Wan-Teh Chang]

Attachment: wtc's proposed patch v2

Could you try this patch?  This patch sets the tm_zone
field to NULL and the tm_gmtoff field to the right value
if struct tm has these two nonstandard fields.

I am just trying to avoid the overhead of initializing
the structure when we know we are going to set the
individual fields next.

Comment 7

[Nicholas Maniscalco]

Thanks for the update.  I'm rebuilding with this patch now.

Comment 8

[Wan-Teh Chang]

By the way, you don't need to rebuild the whole mozilla client.
You just need to apply the patch to the source tree, cd into
the "nsprpub" directory in your build tree, and say "gmake".

Comment 9

[Nicholas Maniscalco]

> By the way, you don't need to rebuild the whole mozilla client.
> You just need to apply the patch to the source tree, cd into
> the "nsprpub" directory in your build tree, and say "gmake".

Right.  I had some other things changed and didn't really trust my make
environment so I just blew it away and started fresh.

Yes, the second patch appears to fix the problem, phoenix no longer crashes when
I attempt to save a file.

So what's the next step?

Comment 10

[Wan-Teh Chang]

Thank you for verifying that the second patch works.  I've
checked it into the NSPR TIP (NSPR 4.3) and the
NSPRPUB_PRE_4_2_CLIENT_BRANCH (mozilla 1.4alpha).

I don't know how Phoenix is built, so you need to ask the
Phoenix team how to merge this patch into the NSPR they
are using.