64bit segfault in cygcheck

[Peter Rosin]

Hi!

I did a "cygcheck -svc", mostly for fun, and got a segfault. A gbd
session puts the blame on cygcheck.cc:671, which currently has this:

670: struct tm *tm = localtime ((const time_t *) &(ed->timestamp));
671: if (tm->tm_year < 60)
672: tm->tm_year += 2000;

The reproducer I have is:

$ cygcheck -v /bin/cygruby191.dll
Segmentation fault
$

So, since I'm not set up to build my own cygwin dll (not
comfortably anyway), I will try a blind patch and leave the
testing of it to someone else.

I haven't researched what's up with that dll, and why localtime()
would fail, but checking the return value is the right thing to do
regardless if there are other issues.

Cheers,
Peter

Index: cygcheck.cc
===================================================================
RCS file: /cvs/src/src/winsup/utils/cygcheck.cc,v
retrieving revision 1.137
diff -u -r1.137 cygcheck.cc
--- cygcheck.cc 21 Jan 2013 16:28:27 -0000 1.137
+++ cygcheck.cc 28 Mar 2013 15:08:29 -0000
@@ -668,16 +668,19 @@
ExpDirectory *ed = (ExpDirectory *) exp;
int ofs = ed->name_rva - export_rva;
struct tm *tm = localtime ((const time_t *) &(ed->timestamp));
- if (tm->tm_year < 60)
+ if (tm && tm->tm_year < 60)
tm->tm_year += 2000;
- if (tm->tm_year < 200)
+ if (tm && tm->tm_year < 200)
tm->tm_year += 1900;
printf ("%*c", lvl + 2, ' ');
- printf ("\"%s\" v%d.%d ts=", exp + ofs,
+ printf ("\"%s\" v%d.%d", exp + ofs,
ed->major_ver, ed->minor_ver);
- printf ("%d/%d/%d %d:%02d\n",
- tm->tm_year, tm->tm_mon + 1, tm->tm_mday,
- tm->tm_hour, tm->tm_min);
+ if (tm)
+ printf (" ts=%d/%d/%d %d:%02d\n",
+ tm->tm_year, tm->tm_mon + 1, tm->tm_mday,
+ tm->tm_hour, tm->tm_min);
+ else
+ printf ("\n");
}
}

[Peter Rosin]

Post by Peter Rosin
Hi!
I did a "cygcheck -svc", mostly for fun, and got a segfault. A gbd
670: struct tm *tm = localtime ((const time_t *) &(ed->timestamp));
671: if (tm->tm_year < 60)
672: tm->tm_year += 2000;
$ cygcheck -v /bin/cygruby191.dll
Segmentation fault
$
So, since I'm not set up to build my own cygwin dll (not
comfortably anyway), I will try a blind patch and leave the
testing of it to someone else.
I haven't researched what's up with that dll, and why localtime()
would fail, but checking the return value is the right thing to do
regardless if there are other issues.

Sorry for replying to self, but right after hitting send it occurred
to me that it might be type size issue (duh!), and I believe this is
a better patch...

int is still 32 bits, while time_t is now 64 bits, right?

Cheers,
Peter

Index: cygcheck.cc
===================================================================
RCS file: /cvs/src/src/winsup/utils/cygcheck.cc,v
retrieving revision 1.137
diff -u -r1.137 cygcheck.cc
--- cygcheck.cc 21 Jan 2013 16:28:27 -0000 1.137
+++ cygcheck.cc 28 Mar 2013 15:26:57 -0000
@@ -667,17 +667,21 @@

ExpDirectory *ed = (ExpDirectory *) exp;
int ofs = ed->name_rva - export_rva;
- struct tm *tm = localtime ((const time_t *) &(ed->timestamp));
- if (tm->tm_year < 60)
+ time_t ts = ed->timestamp;
+ struct tm *tm = localtime (&ts);
+ if (tm && tm->tm_year < 60)
tm->tm_year += 2000;
- if (tm->tm_year < 200)
+ if (tm && tm->tm_year < 200)
tm->tm_year += 1900;
printf ("%*c", lvl + 2, ' ');
- printf ("\"%s\" v%d.%d ts=", exp + ofs,
+ printf ("\"%s\" v%d.%d", exp + ofs,
ed->major_ver, ed->minor_ver);
- printf ("%d/%d/%d %d:%02d\n",
- tm->tm_year, tm->tm_mon + 1, tm->tm_mday,
- tm->tm_hour, tm->tm_min);
+ if (tm)
+ printf (" ts=%d/%d/%d %d:%02d\n",
+ tm->tm_year, tm->tm_mon + 1, tm->tm_mday,
+ tm->tm_hour, tm->tm_min);
+ else
+ printf ("\n");
}
}

[Corinna Vinschen]

Post by Peter Rosin

Post by Peter Rosin
Hi!
I did a "cygcheck -svc", mostly for fun, and got a segfault. A gbd
670: struct tm *tm = localtime ((const time_t *) &(ed->timestamp));
671: if (tm->tm_year < 60)
672: tm->tm_year += 2000;
$ cygcheck -v /bin/cygruby191.dll
Segmentation fault
$
So, since I'm not set up to build my own cygwin dll (not
comfortably anyway), I will try a blind patch and leave the
testing of it to someone else.
I haven't researched what's up with that dll, and why localtime()
would fail, but checking the return value is the right thing to do
regardless if there are other issues.

Sorry for replying to self, but right after hitting send it occurred
to me that it might be type size issue (duh!), and I believe this is
a better patch...
int is still 32 bits, while time_t is now 64 bits, right?

That's right. Thanks a lot for the patch. I'll have a look later today
or tomorrow, since I'm currently hunting down my very own SEGV ;)


Corinna

Post by Peter Rosin
Cheers,
Peter
Index: cygcheck.cc
===================================================================
RCS file: /cvs/src/src/winsup/utils/cygcheck.cc,v
retrieving revision 1.137
diff -u -r1.137 cygcheck.cc
--- cygcheck.cc 21 Jan 2013 16:28:27 -0000 1.137
+++ cygcheck.cc 28 Mar 2013 15:26:57 -0000
@@ -667,17 +667,21 @@
ExpDirectory *ed = (ExpDirectory *) exp;
int ofs = ed->name_rva - export_rva;
- struct tm *tm = localtime ((const time_t *) &(ed->timestamp));
- if (tm->tm_year < 60)
+ time_t ts = ed->timestamp;
+ struct tm *tm = localtime (&ts);
+ if (tm && tm->tm_year < 60)
tm->tm_year += 2000;
- if (tm->tm_year < 200)
+ if (tm && tm->tm_year < 200)
tm->tm_year += 1900;
printf ("%*c", lvl + 2, ' ');
- printf ("\"%s\" v%d.%d ts=", exp + ofs,
+ printf ("\"%s\" v%d.%d", exp + ofs,
ed->major_ver, ed->minor_ver);
- printf ("%d/%d/%d %d:%02d\n",
- tm->tm_year, tm->tm_mon + 1, tm->tm_mday,
- tm->tm_hour, tm->tm_min);
+ if (tm)
+ printf (" ts=%d/%d/%d %d:%02d\n",
+ tm->tm_year, tm->tm_mon + 1, tm->tm_mday,
+ tm->tm_hour, tm->tm_min);
+ else
+ printf ("\n");
}
}

[Peter Rosin]

Post by Corinna Vinschen

Post by Peter Rosin

Post by Peter Rosin
Hi!
I did a "cygcheck -svc", mostly for fun, and got a segfault. A gbd
670: struct tm *tm = localtime ((const time_t *) &(ed->timestamp));
671: if (tm->tm_year < 60)
672: tm->tm_year += 2000;
$ cygcheck -v /bin/cygruby191.dll
Segmentation fault
$
So, since I'm not set up to build my own cygwin dll (not
comfortably anyway), I will try a blind patch and leave the
testing of it to someone else.
I haven't researched what's up with that dll, and why localtime()
would fail, but checking the return value is the right thing to do
regardless if there are other issues.

Sorry for replying to self, but right after hitting send it occurred
to me that it might be type size issue (duh!), and I believe this is
a better patch...
int is still 32 bits, while time_t is now 64 bits, right?

That's right. Thanks a lot for the patch. I'll have a look later today
or tomorrow, since I'm currently hunting down my very own SEGV ;)

Meanwhile, after looking at the ExpDirectory definition and seeing
that the next four bytes are version numbers, I checked the version
of the ruby dll and it indeed has a version (1.9) embedded while
most other dlls just has zero. So, it fits. It is in fact the only
cygwin dll with an embedded version number in my Cygwin 32 install
that I can find (only looked in /bin though...)

Cheers,
Peter

[Corinna Vinschen]

Post by Peter Rosin

Post by Corinna Vinschen

Post by Peter Rosin

Post by Peter Rosin
Hi!
I did a "cygcheck -svc", mostly for fun, and got a segfault. A gbd
670: struct tm *tm = localtime ((const time_t *) &(ed->timestamp));
671: if (tm->tm_year < 60)
672: tm->tm_year += 2000;
$ cygcheck -v /bin/cygruby191.dll
Segmentation fault
$
So, since I'm not set up to build my own cygwin dll (not
comfortably anyway), I will try a blind patch and leave the
testing of it to someone else.
I haven't researched what's up with that dll, and why localtime()
would fail, but checking the return value is the right thing to do
regardless if there are other issues.

Sorry for replying to self, but right after hitting send it occurred
to me that it might be type size issue (duh!), and I believe this is
a better patch...
int is still 32 bits, while time_t is now 64 bits, right?

That's right. Thanks a lot for the patch. I'll have a look later today
or tomorrow, since I'm currently hunting down my very own SEGV ;)

Meanwhile, after looking at the ExpDirectory definition and seeing
that the next four bytes are version numbers, I checked the version
of the ruby dll and it indeed has a version (1.9) embedded while
most other dlls just has zero. So, it fits. It is in fact the only
cygwin dll with an embedded version number in my Cygwin 32 install
that I can find (only looked in /bin though...)

I applied your patch, plus a change to use a more modern date format.


Thanks,
Corinna