NetBSD Problem Report #47528

From tsutsui@ceres.dti.ne.jp  Sun Feb  3 11:55:51 2013
Return-Path: <tsutsui@ceres.dti.ne.jp>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	by www.NetBSD.org (Postfix) with ESMTP id 0D3C363D7B3
	for <gnats-bugs@gnats.NetBSD.org>; Sun,  3 Feb 2013 11:55:50 +0000 (UTC)
Message-Id: <201302031155.r13Btlih002511@mirage.localdomain>
Date: Sun, 3 Feb 2013 20:55:47 +0900 (JST)
From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
Reply-To: tsutsui@ceres.dti.ne.jp
To: gnats-bugs@gnats.NetBSD.org
Cc: tsutsui@ceres.dti.ne.jp
Subject: eeprom(8) dumps core after 64 bit time_t changes
X-Send-Pr-Version: 3.95

>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:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Feb 03 12:00:01 +0000 2013
>Closed-Date:    Sun Oct 20 21:09:26 +0000 2013
>Last-Modified:  Sun Oct 20 21:09:26 +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

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/47528 CVS commit: src/usr.sbin/eeprom
Date: Sun, 3 Feb 2013 10:30:05 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Sun Feb  3 15:30:04 UTC 2013

 Modified Files:
 	src/usr.sbin/eeprom: eehandlers.c

 Log Message:
 PR/47528: Izumi Tsutsui: eeprom(8) dumps core after 64 bit time_t changes


 To generate a diff of this commit:
 cvs rdiff -u -r1.15 -r1.16 src/usr.sbin/eeprom/eehandlers.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->feedback
State-Changed-By: jakllsch@NetBSD.org
State-Changed-When: Sun, 06 Oct 2013 16:15:20 +0000
State-Changed-Why:
Possible fix committed.


From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
To: gnats-bugs@NetBSD.org
Cc: tsutsui@ceres.dti.ne.jp
Subject: Re: bin/47528 (eeprom(8) dumps core after 64 bit time_t changes)
Date: Mon, 7 Oct 2013 08:25:24 +0900

 I'm afraid all netbsd-6 branches are also affected.
 ---

State-Changed-From-To: feedback->pending-pullups
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sun, 06 Oct 2013 23:50:21 +0000
State-Changed-Why:
pullup-6 #964
(I take it that the commit does fix the problem?)
(fully fix, that is)


From: "Manuel Bouyer" <bouyer@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/47528 CVS commit: [netbsd-6] src/usr.sbin/eeprom
Date: Sun, 20 Oct 2013 13:25:02 +0000

 Module Name:	src
 Committed By:	bouyer
 Date:		Sun Oct 20 13:25:02 UTC 2013

 Modified Files:
 	src/usr.sbin/eeprom [netbsd-6]: eehandlers.c

 Log Message:
 Pull up following revision(s) (requested by dholland in ticket #964):
 	usr.sbin/eeprom/eehandlers.c: revision 1.16
 PR/47528: Izumi Tsutsui: eeprom(8) dumps core after 64 bit time_t changes


 To generate a diff of this commit:
 cvs rdiff -u -r1.15 -r1.15.8.1 src/usr.sbin/eeprom/eehandlers.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: bouyer@NetBSD.org
State-Changed-When: Sun, 20 Oct 2013 21:09:26 +0000
State-Changed-Why:
fix pulled up to netbsd-6


>Unformatted:

Home	PR Database Search