Bug 812 - mktime does not return -1
Summary: mktime does not return -1
Status: RESOLVED INVALID
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: 2.3.4
: P2 normal
Target Milestone: ---
Assignee: Uttam Pawar
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-04-01 03:45 UTC by Uttam Pawar
Modified: 2018-04-19 14:56 UTC (History)
2 users (show)

See Also:
Host: powerpc
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
The attachment contain the localized fix (250 bytes, patch)
2005-04-01 16:37 UTC, Uttam Pawar
Details | Diff
Program to demonstrate that mktime() does not always return -1 for errors (350 bytes, application/octet-stream)
2005-09-28 08:00 UTC, Matthew Fischer
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Uttam Pawar 2005-04-01 03:45:51 UTC
mktime function does not (time_t) -1 when the input is invalid.
Here is a test case.

#include <time.h>
#include <stdio.h>
                                                                               
                  
main(int argc, char** argv)
{
    time_t t1=0;
    struct tm t2;
                                                                               
                  
/* 02/29/1889 is an invalid date, mktime() should fail.          */
    t2.tm_year  = 1889 - 1900;
    t2.tm_mon   = 2 - 1 ;
    t2.tm_mday  = 29;
    t2.tm_hour  = 0;
    t2.tm_min   = 0;
    t2.tm_sec   = 1;
    t2.tm_isdst  = -1;
    t1 = mktime(&t2);
    printf("tm_year = [%d], tm_isdst = [%d], T1=[%d]\n", t2.tm_year,
t2.tm_isdst,t1);
    if (t1 != (time_t) -1) {
        printf("ERROR...mktime not returning -1 when error occurs.\n");
        return 1;
    }
    return 0;
}

Steps to Reproduce:
1. gcc version 3.4.3 20041212 (Red Hat 3.4.3-9.EL4)
2. glibc-2.3.4-2.ppc
2. Compile: gcc t.c
3. Run: a.out

Actual Results:
ERROR...mktime not returning -1 when error occurs.

Expected Results:
Return 0.

All though it has been tested on PowerPC platform, I think it's generic bug in
glibc-2.3.4.

I've also, created a fix for the above problem as shown below,
diff -Naru glibc-20041219T2331/time/mktime.c glibc-20041219T2331.mine/time/mktime.c
--- glibc-20041219T2331/time/mktime.c   2004-12-02 14:16:28.000000000 -0800
+++ glibc-20041219T2331.mine/time/mktime.c      2005-03-31 18:53:01.036619040 -0800
@@ -463,7 +463,7 @@
       t2 = t1 + sec_adjustment;
       if (((t1 < t) != (sec_requested < 0))
          | ((t2 < t1) != (sec_adjustment < 0))
-         | ! (*convert) (&t2, &tm))
+         | ! (*convert) (&t2, &tm) | (t2 < 0))
        return -1;
       t = t2;
     }
~

As it makes no sense to return the negative value other than -1 in case of
error. This patch has been tested and above change seems to fix the problem.

TESTING before the change:
Tesing with tm_year=0
tm_year = [0], tm_isdst = [0], T1=[-1]
tm_year = [1], tm_isdst = [0], T1=[-2147483648]
ERROR...mktime not returning -1 when error occurs.
tm_year = [1], tm_isdst = [0], T1=[-2147483648]
ERROR...mktime not returning -1 when error occurs.
Tesing with tm_year=1
tm_year = [1], tm_isdst = [0], T1=[-1]
tm_year = [1], tm_isdst = [0], T1=[-2147483648]
ERROR...mktime not returning -1 when error occurs.
tm_year = [1], tm_isdst = [0], T1=[-2147483648]
ERROR...mktime not returning -1 when error occurs.
Tesing with tm_year=-1
tm_year = [-1], tm_isdst = [0], T1=[-1]
tm_year = [1], tm_isdst = [0], T1=[-2147483648]
ERROR...mktime not returning -1 when error occurs.
tm_year = [1], tm_isdst = [0], T1=[-2147483648]
ERROR...mktime not returning -1 when error occurs.
Tesing with tm_year=-11
tm_year = [-11], tm_isdst = [0], T1=[-1]
tm_year = [1], tm_isdst = [0], T1=[-2147483648]
ERROR...mktime not returning -1 when error occurs.
tm_year = [1], tm_isdst = [0], T1=[-2147483648]
ERROR...mktime not returning -1 when error occurs.

TESTIG after the change:
Tesing with tm_year=0
tm_year = [0], tm_isdst = [0], T1=[-1]
tm_year = [0], tm_isdst = [1], T1=[-1]
tm_year = [0], tm_isdst = [-1], T1=[-1]
Tesing with tm_year=1
tm_year = [1], tm_isdst = [0], T1=[-1]
tm_year = [1], tm_isdst = [1], T1=[-1]
tm_year = [1], tm_isdst = [-1], T1=[-1]
Tesing with tm_year=-1
tm_year = [-1], tm_isdst = [0], T1=[-1]
tm_year = [-1], tm_isdst = [1], T1=[-1]
tm_year = [-1], tm_isdst = [-1], T1=[-1]
Tesing with tm_year=-11
tm_year = [-11], tm_isdst = [0], T1=[-1]
tm_year = [-11], tm_isdst = [1], T1=[-1]
tm_year = [-11], tm_isdst = [-1], T1=[-1]


Thanks,

Uttam
Comment 1 Uttam Pawar 2005-04-01 16:37:44 UTC
Created attachment 450 [details]
The attachment contain the localized fix

The proposed patch contains the localized fix.
Comment 2 Andreas Schwab 2005-04-01 17:49:08 UTC
A negative value as the number of seconds since the Epoch is perfectly valid.  
Comment 3 Uttam Pawar 2005-04-01 19:54:06 UTC
Looking at different documents, I can assume that, the negative number is not
totally wrong but it's representation of time prior to 1970. Is this assumption
correct? If the value it's returning is valid but negative due to year prior to
1970 then it's not an error? If this is true then this does not look like a
valid bug?
Comment 4 Uttam Pawar 2005-04-04 21:31:28 UTC
(In reply to comment #2)
> A negative value as the number of seconds since the Epoch is perfectly valid.  

Yes, but the value returned by mktime (between -2147483648 and 2147483647) is
the maximum number of seconds can be represent using time_t where tm_year ranges
between (1-138) i.e. tm_year > 0. Can't we just add check to see if the input
tm_year is valid?

Comment 5 Matthew Fischer 2005-04-10 09:33:15 UTC
It looks like mktime() will return INT_MIN if the value would be less than
time_t's minimum, but not by more than 20 years, and it will return INT_MAX if
the value would be greater than time_t's maximum, but not by more than 20 years.
If the value exceeds time_t's range by more than 20 years, then mktime() will
return -1. It should, however, return -1 in all three cases.
Comment 6 Uttam Pawar 2005-04-11 17:39:43 UTC
(In reply to comment #5)
> It looks like mktime() will return INT_MIN if the value would be less than
> time_t's minimum, but not by more than 20 years, and it will return INT_MAX if
> the value would be greater than time_t's maximum, but not by more than 20 years.
> If the value exceeds time_t's range by more than 20 years, then mktime() will
> return -1. It should, however, return -1 in all three cases.

I think it should return -1 for the 2 cases you mentioned (if the value is less
than time_t's minimum or greter than time_t's maximum because if the value is
smaller or greater than those two threshold then it's going to wrap around to
give you wrong results). This is due to the size of time_t. On a 64-bit system
where size of size of time_t is 8 byes, this will hold valid seconds for many
years past and future of since epoch.
But for now it can represent only seconds for (tm_year) 138 years (since 1901
till 2038).
Comment 7 Danny.Aizer 2005-06-01 11:34:30 UTC
If mktime can return negative values for dates/times before 1970, it also means
that the value (time_t)-1 which is returned by mktime() can either mean:

1. Invalid tm structure (see the man page under RETURN VALUE)

2. Dec 31 23:59:59 UTC 1969 (assuming the local TZ variable is set to UTC)

3. Some other date and time in the +/-24 hours around the EPOCH, if the TZ
variable is set to some other non-UTC timezone.

This makes it almost impossible to guess whether the mktime() failed due to
invalid tm, or just gave a valid calculated negative answer.
Comment 8 Ulrich Drepper 2005-09-23 20:17:28 UTC
There is nothing wrong with the code.  Negative results are possible and maybe
even used.  Any change would mean possible binary incompatiblity.

And of course this doesn't introduce any ambiguities: (time_t) -1 means error,
one way or another.  Everybody who cannot see this is not fit enough to comment
here.
Comment 9 Matthew Fischer 2005-09-28 07:56:43 UTC
Of course it is true that negative values are valid, but the problem that I see
is that -1 is not always returned for an error. If I call mktime() for January
1st, 1880 with a 32-bit time_t, I get -1 (correct). If I call mktime() for
January 1st, 1890, I get -2147483648 (INT_MIN), which is not correct. If I call
mktime() for January 1st, 1900, I also get -2147483648. Both of those values
should have been -1, since the parameters to mktime() could not be represented
in a time_t.
Comment 10 Matthew Fischer 2005-09-28 08:00:47 UTC
Created attachment 678 [details]
Program to demonstrate that mktime() does not always return -1 for errors