>Number:         28471
>Category:       kern
>Synopsis:       Sign extension issue in cd9660
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Nov 29 23:45:01 +0000 2004
>Originator:     Rhialto
>Release:        NetBSD 2.0_BETA
>Organization:
>Environment:
System: NetBSD loelappie.falu.nl 2.0_BETA NetBSD 2.0_BETA (LOELAPPIE) #8: Wed Aug 11 18:22:58 CEST 2004 root@loelappie.falu.nl:/usr/src/sys/arch/i386/compile/LOELAPPIE i386
Architecture: i386
Machine: i386
>Description:
	If you use a version of mkisofs that can create files between
	2G and 4G (version 2.01 and upwards), the file size gets sign-
	extended on 64-bit archs.

	This is because "ip->i_size = isonum_733(isodir->size);" performs
	sign extension since ip->i_size (unsigned long) is bigger than int.

	I checked that the relevant code in 2.0-release is the same.

>How-To-Repeat:
	As above.
>Fix:
	I propose the following patch to bring the code in line with the
	comments. In principle it should be checked that other uses of 
	isonum_733() are not negatively impacted (pun not intended) but
	if the comments accurately reflect the standard, they should not be.

	Furthermore the source used u_int16t and u_int32t in code that
	was apparently never compiled, but these types were not
	defined. I'm fixing that too.

	I compile-tested this on 2.0_BETA/i386 but the principle was
	tested on 1.6.2/alpha.

--- iso.h.orig	2004-11-30 00:19:48.000000000 +0100
+++ iso.h	2004-11-30 00:16:53.000000000 +0100
@@ -170,12 +170,12 @@
 
 static __inline int isonum_711 __P((u_char *)) __attribute__ ((unused));
 static __inline int isonum_712 __P((char *)) __attribute__ ((unused));
-static __inline int isonum_721 __P((u_char *)) __attribute__ ((unused));
-static __inline int isonum_722 __P((u_char *)) __attribute__ ((unused));
-static __inline int isonum_723 __P((u_char *)) __attribute__ ((unused));
-static __inline int isonum_731 __P((u_char *)) __attribute__ ((unused));
-static __inline int isonum_732 __P((u_char *)) __attribute__ ((unused));
-static __inline int isonum_733 __P((u_char *)) __attribute__ ((unused));
+static __inline u_int16_t isonum_721 __P((u_char *)) __attribute__ ((unused));
+static __inline u_int16_t isonum_722 __P((u_char *)) __attribute__ ((unused));
+static __inline u_int16_t isonum_723 __P((u_char *)) __attribute__ ((unused));
+static __inline u_int32_t isonum_731 __P((u_char *)) __attribute__ ((unused));
+static __inline u_int32_t isonum_732 __P((u_char *)) __attribute__ ((unused));
+static __inline u_int32_t isonum_733 __P((u_char *)) __attribute__ ((unused));
 
 /* 7.1.1: unsigned char */
 static __inline int
@@ -202,31 +202,31 @@
 }
 
 /* 7.2.1: unsigned little-endian 16-bit value.  NOT USED IN KERNEL. */
-static __inline int
+static __inline u_int16_t
 isonum_721(p)
 	u_char *p;
 {
 #if defined(UNALIGNED_ACCESS) && (BYTE_ORDER == LITTLE_ENDIAN)
-	return *(u_int16t *)p;
+	return *(u_int16_t *)p;
 #else
 	return *p|((char)p[1] << 8);
 #endif
 }
 
 /* 7.2.2: unsigned big-endian 16-bit value.  NOT USED IN KERNEL. */
-static __inline int     
+static __inline u_int16_t     
 isonum_722(p)
 	unsigned char *p;
 {
 #if defined(UNALIGNED_ACCESS) && (BYTE_ORDER == BIG_ENDIAN)
-	return *(u_int16t *)p;
+	return *(u_int16_t *)p;
 #else
 	return ((char)*p << 8)|p[1];
 #endif
 } 
 
 /* 7.2.3: unsigned both-endian (little, then big) 16-bit value */
-static __inline int
+static __inline u_int16_t
 #if __STDC__
 isonum_723(u_char *p)
 #else
@@ -237,9 +237,9 @@
 #if defined(UNALIGNED_ACCESS) && \
     ((BYTE_ORDER == LITTLE_ENDIAN) || (BYTE_ORDER == BIG_ENDIAN))
 #if BYTE_ORDER == LITTLE_ENDIAN
-	return *(u_int16t *)p;
+	return *(u_int16_t *)p;
 #else
-	return *(u_int16t *)(p + 2);
+	return *(u_int16_t *)(p + 2);
 #endif
 #else /* !UNALIGNED_ACCESS or weird byte order */
 	return *p|(p[1] << 8);
@@ -247,31 +247,31 @@
 }
 
 /* 7.3.1: unsigned little-endian 32-bit value.  NOT USED IN KERNEL. */
-static __inline int
+static __inline u_int32_t
 isonum_731(p)
 	u_char *p;
 {
 #if defined(UNALIGNED_ACCESS) && (BYTE_ORDER == LITTLE_ENDIAN)
-	return *(u_int32t *)p;
+	return *(u_int32_t *)p;
 #else
 	return *p|(p[1] << 8)|(p[2] << 16)|(p[3] << 24);
 #endif
 }
 
 /* 7.3.2: unsigned big-endian 32-bit value.  NOT USED IN KERNEL. */
-static __inline int
+static __inline u_int32_t
 isonum_732(p)
 	unsigned char *p;
 {
 #if defined(UNALIGNED_ACCESS) && (BYTE_ORDER == BIG_ENDIAN)
-	return *(u_int32t *)p;
+	return *(u_int32_t *)p;
 #else
 	return (*p << 24)|(p[1] << 16)|(p[2] << 8)|p[3];
 #endif
 }
 
 /* 7.3.3: unsigned both-endian (little, then big) 32-bit value */
-static __inline int
+static __inline u_int32_t
 #if __STDC__
 isonum_733(u_char *p)
 #else
@@ -282,9 +282,9 @@
 #if defined(UNALIGNED_ACCESS) && \
     ((BYTE_ORDER == LITTLE_ENDIAN) || (BYTE_ORDER == BIG_ENDIAN))
 #if BYTE_ORDER == LITTLE_ENDIAN
-	return *(u_int32t *)p;
+	return *(u_int32_t *)p;
 #else
-	return *(u_int32t *)(p + 4);
+	return *(u_int32_t *)(p + 4);
 #endif
 #else /* !UNALIGNED_ACCESS or weird byte order */
 	return *p|(p[1] << 8)|(p[2] << 16)|(p[3] << 24);

-Olaf.
-- 
                           -- Ceterum censeo "authored[1]" delendum esse.
___ Olaf 'Rhialto' Seibert -- [1] Ugly English neologism[2].
\X/ rhialto/at/xs4all.nl   -- [2] For lawyers whose English/Latin is below par.