[PATCH] fs: isofs: Fix bug in the way to check if the year is a leap year

[PATCH] fs: isofs: Fix bug in the way to check if the year is a leap year

[Oscar Forner Martinez]

The way to check if the year is a leap year was incomplete.
It was checking only if the year can be divided by 4, that is
necessary but not a sufficient condition. It has to check as well
if the year can not be divided by 100 or if the year can be
divided by 400.

Signed-off-by: Oscar Forner Martinez <oscar.forner.martinez@gmail.com>
---
 fs/isofs/util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/isofs/util.c b/fs/isofs/util.c
index 01e1ee7..d2dd602 100644
--- a/fs/isofs/util.c
+++ b/fs/isofs/util.c
@@ -38,7 +38,7 @@ int iso_date(char * p, int flag)
 			days += (year+1) / 4;
 		for (i = 1; i < month; i++)
 			days += monlen[i-1];
-		if (((year+2) % 4) == 0 && month > 2)
+		if (((year+2) % 4) == 0 && month > 2 && ((((year+2) % 100) != 0) || (((year+2) % 400) == 0)))
 			days++;
 		days += day - 1;
 		crtime = ((((days * 24) + hour) * 60 + minute) * 60)
-- 
2.2.1

Re: [PATCH] fs: isofs: Fix bug in the way to check if the year is a leap year

[Al Viro]

On Fri, Jan 02, 2015 at 04:08:48PM +0000, Oscar Forner Martinez wrote:
> The way to check if the year is a leap year was incomplete.
> It was checking only if the year can be divided by 4, that is
> necessary but not a sufficient condition. It has to check as well
> if the year can not be divided by 100 or if the year can be
> divided by 400.

... which is utterly pointless since the possible values of 'year' are
quite limited here.

> Signed-off-by: Oscar Forner Martinez <oscar.forner.martinez@gmail.com>
> ---
>  fs/isofs/util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/isofs/util.c b/fs/isofs/util.c
> index 01e1ee7..d2dd602 100644
> --- a/fs/isofs/util.c
> +++ b/fs/isofs/util.c
> @@ -38,7 +38,7 @@ int iso_date(char * p, int flag)
>  			days += (year+1) / 4;
>  		for (i = 1; i < month; i++)
>  			days += monlen[i-1];
> -		if (((year+2) % 4) == 0 && month > 2)
> +		if (((year+2) % 4) == 0 && month > 2 && ((((year+2) % 100) != 0) || (((year+2) % 400) == 0)))

... *and* completely wrong, since 'year' at that point is "years since
the epoch (1970)".  So your extra condition would first trigger in 2068.
What do you think that "+ 2" was about?  Black magic?

FWIW, ISO9660 (9.1.5) says that the date is stored as series of 8bit values,
interpreted as
	* year - 1900
	* month (1 to 12)
	* day of month (1 to 31)
	* hour (0 to 23)
	* minute (0 to 59)
	* second (0 to 59)
	* offset from GMT in quarters of hour (-48 to 52)

So the only relevant years are 1900 and 2100.  It refuses to accept anything
prior to 1970 anyway and the damn thing returns signed 32bit (seconds since
Jan 1, 1970).  Remember what's the maximal date that can be represented by
that?  Right, it's in 2038...

Incidentally, iso_date() is broken - it trusts the month to be within 1..12,
which might not be true for a corrupted image.  And while we are at it,
having that monlen[12] array on stack means initializing it every sodding
time.  And I really, really doubt that we don't have that sucker already
done better... <greps>

Aha:
/*
 * mktime64 - Converts date to seconds.
 * Converts Gregorian date to seconds since 1970-01-01 00:00:00.
 * Assumes input in normal date format, i.e. 1980-12-31 23:59:59
 * => year=1980, mon=12, day=31, hour=23, min=59, sec=59.
 *
 * [For the Julian calendar (which was used in Russia before 1917,
 * Britain & colonies before 1752, anywhere else before 1582,
 * and is still in use by some communities) leave out the
 * -year/100+year/400 terms, and add 10.]
 *
 * This algorithm was first published by Gauss (I think).
 */

So this
                int monlen[12] = {31,28,31,30,31,30,31,31,30,31,30,31};

                days = year * 365;
                if (year > 2)
                        days += (year+1) / 4;
                for (i = 1; i < month; i++)
                        days += monlen[i-1];
                if (((year+2) % 4) == 0 && month > 2)
                        days++;
                days += day - 1;
                crtime = ((((days * 24) + hour) * 60 + minute) * 60)
                        + second;
should become simply
		crtime = mktime64(year+1970, month, day, hour, minute, second);

Re: [PATCH] fs: isofs: Fix bug in the way to check if the year is a leap year

[Al Viro]

On Fri, Jan 02, 2015 at 09:26:35PM +0000, Oscar Forner Martinez wrote:

> I did not know that function.

Neither did I ;-)  What I'd done was

; git grep -n -w date.*seconds
drivers/rtc/rtc-lib.c:117: * Convert Gregorian date to seconds since 01-01-1970 00:00:00.
fs/fat/misc.c:192:/* Convert a FAT time/date pair to a UNIX date (seconds since 1 1 70). */
fs/ncpfs/dir.c:1193:/* Convert a MS-DOS time/date pair to a UNIX date (seconds since 1 1 70). */
fs/nfs/nfs4proc.c:6868:                 clp->cl_implid->date.seconds,
fs/nfs/nfs4proc.c:6869:                 clp->cl_implid->date.nseconds);
fs/nfs/nfs4xdr.c:5532:          p = xdr_decode_hyper(p, &res->impl_id->date.seconds);
fs/nfs/nfs4xdr.c:5533:          res->impl_id->date.nseconds = be32_to_cpup(p);
fs/nfs/super.c:762:                        impl_id->date.seconds, impl_id->date.nseconds);
kernel/time/time.c:308: * mktime64 - Converts date to seconds.
kernel/time/time.c:309: * Converts Gregorian date to seconds since 1970-01-01 00:00:00.
scripts/analyze_suspend.py:165:                 utcoffset = int((datetime.now() - datetime.utcnow()).total_seconds())
;

and that was it.  While we are at it, drivers/rtc hit is a wrapper around
mktime64(), fs/nfs ones are obviously noise from quick and dirty search
pattern and so's scripts/analyze_suspend.py one.  fs/fat/misc.c and
fs/ncpfs/dir.c ones, though, are really asking for being converted to
mktime64().

While we are grepping, git grep -n '\<31\>.*\<28\>.*\<31\>' also finds
something interesting -
arch/m68k/bvme6000/rtc.c:34:{0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
arch/m68k/mvme16x/rtc.c:33:{0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
drivers/char/ds1302.c:153:    {0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
drivers/char/genrtc.c:83:{31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
drivers/char/rtc.c:212:{0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
are all about the same thing - open-coded
rtc_month_days(), if not open-coded rtc_valid_tm().  Might be worth looking
into...  And another hit (arch/powerpc/kernel/time.c:to_tm()) is downright
obscene:
        /* Number of months in days left */
        if (leapyear(tm->tm_year))
                days_in_month(FEBRUARY) = 29;
        for (i = 1; day >= days_in_month(i); i++)
                day -= days_in_month(i);
        days_in_month(FEBRUARY) = 28;
(whadday mean, "locking"?)  Almost certainly wants to switch to time_to_tm()...