Re: [PATCH 5/5] log: do not segfault on gmtime errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 24.02.2014 08:49, schrieb Jeff King:
Many code paths assume that show_date and show_ident_date
cannot return NULL. For the most part, we handle missing or
corrupt timestamps by showing the epoch time t=0.

However, we might still return NULL if gmtime rejects the
time_t we feed it, resulting in a segfault. Let's catch this
case and just format t=0.

Signed-off-by: Jeff King <peff@xxxxxxxx>
---
This test is of questionable portability, since we are depending on
gmtime's arbitrary point to decide that our input is crazy and return
NULL. The value is sufficiently large that I'd expect most to do so,
though, so it may be safe.

Just came around to testing on FreeBSD 10 amd64; the new test in t4212 fails there:

--- expect      2014-03-22 08:29:44.000000000 +0000
+++ actual      2014-03-22 08:29:44.000000000 +0000
@@ -1 +1 @@
-Thu Jan 1 00:00:00 1970 +0000
+Sun Jan 0 00:00:00 1900 -0700
not ok 9 - absurdly far-in-future dates produce sentinel
#
#               commit=$(munge_author_date HEAD 999999999999999999) &&
#               echo "Thu Jan 1 00:00:00 1970 +0000" >expect &&
#               git log -1 --format=%ad $commit >actual &&
#               test_cmp expect actual
#

# failed 1 among 9 test(s)

Looks like we get a cleared struct tm instead of a NULL pointer. It seems to be a long-standing bug; http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/145341 was filed in April 2010.

On 32-bit systems, of course, the test does nothing (it is just hitting
the integer overflow code path). But that's OK, since the output is the
same for both cases.

  date.c                 | 6 ++++--
  t/t4212-log-corrupt.sh | 8 ++++++++
  2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/date.c b/date.c
index 90b28f7..e1a2cee 100644
--- a/date.c
+++ b/date.c
@@ -184,8 +184,10 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
  		tz = local_tzoffset(time);

  	tm = time_to_tm(time, tz);
-	if (!tm)
-		return NULL;
+	if (!tm) {

Would it make sense to work around the FreeBSD issue by adding a check like this?

	if (!tm || tm->tm_year < 70) {

+		tm = time_to_tm(0, 0);
+		tz = 0;
+	}

  	strbuf_reset(&timebuf);
  	if (mode == DATE_SHORT)
diff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh
index ba25a2e..3fa1715 100755
--- a/t/t4212-log-corrupt.sh
+++ b/t/t4212-log-corrupt.sh
@@ -81,4 +81,12 @@ test_expect_success 'date parser recognizes time_t overflow' '
  	test_cmp expect actual
  '

+# date is within 2^63-1, but enough to choke glibc's gmtime
+test_expect_success 'absurdly far-in-future dates produce sentinel' '
+	commit=$(munge_author_date HEAD 999999999999999999) &&
+	echo "Thu Jan 1 00:00:00 1970 +0000" >expect &&
+	git log -1 --format=%ad $commit >actual &&
+	test_cmp expect actual
+'
+
  test_done


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]