bin/47528: eeprom(8) dumps core after 64 bit time_t changes

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

>Number:         47528
>Category:       bin
>Synopsis:       eeprom(8) dumps core after 64 bit time_t changes
>Confidential:   no
>Severity:       critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Feb 03 12:00:01 +0000 2013
>Originator:     Izumi Tsutsui
>Release:        NetBSD 6.0.1
>Organization:
>Environment:
System: NetBSD 6.0.1 sun3 on 3/80 
Architecture: m68k
Machine: sun3
>Description:
eeprom(8) command dumps core on NetBSD/sun3 6.0.1:

---
# eeprom
Segmentation fault (core dumped)
# gdb eeprom eeprom.core 
GNU gdb (GDB) 7.3.1
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "m68k--netbsdelf".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /usr/sbin/eeprom...(no debugging symbols found)...done.
[New process 1]
Core was generated by `eeprom'.
Program terminated with signal 11, Segmentation fault.
#0  0x080c5a76 in strrchr () from /usr/lib/libc.so.12
(gdb) bt
#0  0x080c5a76 in strrchr () from /usr/lib/libc.so.12
#1  0x00003e74 in ee_hwupdate ()
#2  0x00003f20 in ee_dump ()
#3  0x00002f84 in main ()
(gdb) 

---
This means:
 - ee_hwupdate() command uses sizeof(time_t) to read/write hwupdate value
   from/to EEPROM/NVRAM via doio()
 - on the hardware memory it's always fixed 4 byte
   so doio() returns bogus time_t value
 - with bogus (too large) time_t value ctime(3) could return NULL
   (on BE machines first 4 bytes in int64_t are MSBytes)
 - NULL is passed to strrchr() and it causes coredump

>How-To-Repeat:
Just exec eeprom on sun3.
(it looks sparc doesn't have hwupdate value in NVRAM)

>Fix:
The following patch fixes the problem by:
 - (ab)use uint32_t to read/write EEPROM/NVRAM via doio()
 - check return value of ctime(3)
 - don't print date format in output if ctime(3) returns NULL

In the perfect world, we should consider how
we can handle all possible hardware/on-disk 32bit time_t,
but using uint32_t might be easier.

Index: eehandlers.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/eeprom/eehandlers.c,v
retrieving revision 1.15
diff -u -p -r1.15 eehandlers.c
--- eehandlers.c        30 Apr 2009 07:45:28 -0000      1.15
+++ eehandlers.c        3 Feb 2013 11:38:13 -0000
@@ -40,6 +40,7 @@
 #include <time.h>
 #include <unistd.h>
 #include <util.h>
+#include <sys/inttypes.h>
 
 #include <machine/eeprom.h>
 #ifdef __sparc__
@@ -140,6 +141,7 @@ ee_hwupdate(ktent, arg)
        struct keytabent *ktent;
        char *arg;
 {
+       uint32_t hwtime;
        time_t t;
        char *cp, *cp2;
 
@@ -154,18 +156,24 @@ ee_hwupdate(ktent, arg)
                } else
                        if ((t = parsedate(arg, NULL, NULL)) == (time_t)(-1))
                                BARF(ktent);
+               hwtime = (uint32_t)t;   /* XXX 32 bit time_t on hardware */
 
-               if (doio(ktent, (u_char *)&t, sizeof(t), IO_WRITE))
+               if (doio(ktent, (u_char *)&hwtime, sizeof(hwtime), IO_WRITE))
                        FAILEDWRITE(ktent);
-       } else
-               if (doio(ktent, (u_char *)&t, sizeof(t), IO_READ))
+       } else {
+               if (doio(ktent, (u_char *)&hwtime, sizeof(hwtime), IO_READ))
                        FAILEDREAD(ktent);
+               t = (time_t)hwtime;     /* XXX 32 bit time_t on hardware */
+       }
 
        cp = ctime(&t);
-       if ((cp2 = strrchr(cp, '\n')) != NULL)
+       if (cp != NULL && (cp2 = strrchr(cp, '\n')) != NULL)
                *cp2 = '\0';
 
-       printf("%s=%ld (%s)\n", ktent->kt_keyword, (long)t, cp);
+       printf("%s=%" PRIu32, ktent->kt_keyword, (uint32_t)t);
+       if (cp != NULL)
+               printf(" (%s)", cp);
+       printf("\n");
 }
 
 void