Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 11 Jul 2011 20:43:59 +0000 (UTC)
From:      Marius Strobl <marius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r223938 - head/sys/boot/common
Message-ID:  <201107112043.p6BKhxOX008571@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: marius
Date: Mon Jul 11 20:43:59 2011
New Revision: 223938
URL: http://svn.freebsd.org/changeset/base/223938

Log:
  Since r219452 the alignment of __dmadat has changed, revealing that fsread()
  bogusly casts its contents around causing alignment faults on sparc64 and
  most likely also on at least powerpc. Fix this by copying the contents
  bytewise instead as partly already done here. Solving this the right way
  costs some space, i.e. 148 bytes with GCC and 16 bytes with clang on x86
  there are still some bytes left there though, and an acceptable hack which
  tricks the compiler into only using a 2-byte alignment instead of the native
  one when accessing the contents turned out to even take up more space that.

Modified:
  head/sys/boot/common/ufsread.c

Modified: head/sys/boot/common/ufsread.c
==============================================================================
--- head/sys/boot/common/ufsread.c	Mon Jul 11 16:51:29 2011	(r223937)
+++ head/sys/boot/common/ufsread.c	Mon Jul 11 20:43:59 2011	(r223938)
@@ -49,6 +49,7 @@ __FBSDID("$FreeBSD$");
 #include <ufs/ufs/dinode.h>
 #include <ufs/ufs/dir.h>
 #include <ufs/ffs/fs.h>
+
 #ifdef UFS_SMALL_CGBASE
 /* XXX: Revert to old (broken for over 1.5Tb filesystems) version of cgbase
    (see sys/ufs/ffs/fs.h rev 1.39) so that small boot loaders (e.g. boot2) can
@@ -90,7 +91,7 @@ static ssize_t fsread(ino_t, void *, siz
 static uint8_t ls, dsk_meta;
 static uint32_t fs_off;
 
-static __inline int
+static __inline uint8_t
 fsfind(const char *name, ino_t * ino)
 {
 	char buf[DEV_BSIZE];
@@ -158,7 +159,7 @@ static int sblock_try[] = SBLOCKSEARCH;
 #elif defined(UFS1_ONLY)
 #define DIP(field) dp1.field
 #else
-#define DIP(field) fs->fs_magic == FS_UFS1_MAGIC ? dp1.field : dp2.field
+#define DIP(field) fs.fs_magic == FS_UFS1_MAGIC ? dp1.field : dp2.field
 #endif
 
 static ssize_t
@@ -173,7 +174,7 @@ fsread(ino_t inode, void *buf, size_t nb
 	static ino_t inomap;
 	char *blkbuf;
 	void *indbuf;
-	struct fs *fs;
+	struct fs fs;
 	char *s;
 	size_t n, nb, size, off, vboff;
 	ufs_lbn_t lbn;
@@ -181,30 +182,29 @@ fsread(ino_t inode, void *buf, size_t nb
 	static ufs2_daddr_t blkmap, indmap;
 	u_int u;
 
-
 	blkbuf = dmadat->blkbuf;
 	indbuf = dmadat->indbuf;
-	fs = (struct fs *)dmadat->sbbuf;
 	if (!dsk_meta) {
 		inomap = 0;
 		for (n = 0; sblock_try[n] != -1; n++) {
-			if (dskread(fs, sblock_try[n] / DEV_BSIZE,
+			if (dskread(dmadat->sbbuf, sblock_try[n] / DEV_BSIZE,
 			    SBLOCKSIZE / DEV_BSIZE))
 				return -1;
+			memcpy(&fs, dmadat->sbbuf, sizeof(struct fs));
 			if ((
 #if defined(UFS1_ONLY)
-			     fs->fs_magic == FS_UFS1_MAGIC
+			    fs.fs_magic == FS_UFS1_MAGIC
 #elif defined(UFS2_ONLY)
-			    (fs->fs_magic == FS_UFS2_MAGIC &&
-			    fs->fs_sblockloc == sblock_try[n])
+			    (fs.fs_magic == FS_UFS2_MAGIC &&
+			    fs.fs_sblockloc == sblock_try[n])
 #else
-			     fs->fs_magic == FS_UFS1_MAGIC ||
-			    (fs->fs_magic == FS_UFS2_MAGIC &&
-			    fs->fs_sblockloc == sblock_try[n])
+			    fs.fs_magic == FS_UFS1_MAGIC ||
+			    (fs.fs_magic == FS_UFS2_MAGIC &&
+			    fs.fs_sblockloc == sblock_try[n])
 #endif
 			    ) &&
-			    fs->fs_bsize <= MAXBSIZE &&
-			    fs->fs_bsize >= sizeof(struct fs))
+			    fs.fs_bsize <= MAXBSIZE &&
+			    fs.fs_bsize >= sizeof(struct fs))
 				break;
 		}
 		if (sblock_try[n] == -1) {
@@ -212,12 +212,13 @@ fsread(ino_t inode, void *buf, size_t nb
 			return -1;
 		}
 		dsk_meta++;
-	}
+	} else
+		memcpy(&fs, dmadat->sbbuf, sizeof(struct fs));
 	if (!inode)
 		return 0;
 	if (inomap != inode) {
-		n = IPERVBLK(fs);
-		if (dskread(blkbuf, INO_TO_VBA(fs, n, inode), DBPERVBLK))
+		n = IPERVBLK(&fs);
+		if (dskread(blkbuf, INO_TO_VBA(&fs, n, inode), DBPERVBLK))
 			return -1;
 		n = INO_TO_VBO(n, inode);
 #if defined(UFS1_ONLY)
@@ -227,13 +228,12 @@ fsread(ino_t inode, void *buf, size_t nb
 		memcpy(&dp2, (struct ufs2_dinode *)blkbuf + n,
 		    sizeof(struct ufs2_dinode));
 #else
-		if (fs->fs_magic == FS_UFS1_MAGIC)
+		if (fs.fs_magic == FS_UFS1_MAGIC)
 			memcpy(&dp1, (struct ufs1_dinode *)blkbuf + n,
 			    sizeof(struct ufs1_dinode));
 		else
 			memcpy(&dp2, (struct ufs2_dinode *)blkbuf + n,
 			    sizeof(struct ufs2_dinode));
-
 #endif
 		inomap = inode;
 		fs_off = 0;
@@ -246,15 +246,15 @@ fsread(ino_t inode, void *buf, size_t nb
 		nbyte = n;
 	nb = nbyte;
 	while (nb) {
-		lbn = lblkno(fs, fs_off);
-		off = blkoff(fs, fs_off);
+		lbn = lblkno(&fs, fs_off);
+		off = blkoff(&fs, fs_off);
 		if (lbn < NDADDR) {
 			addr = DIP(di_db[lbn]);
-		} else if (lbn < NDADDR + NINDIR(fs)) {
-			n = INDIRPERVBLK(fs);
+		} else if (lbn < NDADDR + NINDIR(&fs)) {
+			n = INDIRPERVBLK(&fs);
 			addr = DIP(di_ib[0]);
 			u = (u_int)(lbn - NDADDR) / n * DBPERVBLK;
-			vbaddr = fsbtodb(fs, addr) + u;
+			vbaddr = fsbtodb(&fs, addr) + u;
 			if (indmap != vbaddr) {
 				if (dskread(indbuf, vbaddr, DBPERVBLK))
 					return -1;
@@ -262,21 +262,25 @@ fsread(ino_t inode, void *buf, size_t nb
 			}
 			n = (lbn - NDADDR) & (n - 1);
 #if defined(UFS1_ONLY)
-			addr = ((ufs1_daddr_t *)indbuf)[n];
+			memcpy(&addr, (ufs1_daddr_t *)indbuf + n,
+			    sizeof(ufs1_daddr_t));
 #elif defined(UFS2_ONLY)
-			addr = ((ufs2_daddr_t *)indbuf)[n];
+			memcpy(&addr, (ufs2_daddr_t *)indbuf + n,
+			    sizeof(ufs2_daddr_t));
 #else
-			if (fs->fs_magic == FS_UFS1_MAGIC)
-				addr = ((ufs1_daddr_t *)indbuf)[n];
+			if (fs.fs_magic == FS_UFS1_MAGIC)
+				memcpy(&addr, (ufs1_daddr_t *)indbuf + n,
+				    sizeof(ufs1_daddr_t));
 			else
-				addr = ((ufs2_daddr_t *)indbuf)[n];
+				memcpy(&addr, (ufs2_daddr_t *)indbuf + n,
+				    sizeof(ufs2_daddr_t));
 #endif
 		} else {
 			return -1;
 		}
-		vbaddr = fsbtodb(fs, addr) + (off >> VBLKSHIFT) * DBPERVBLK;
+		vbaddr = fsbtodb(&fs, addr) + (off >> VBLKSHIFT) * DBPERVBLK;
 		vboff = off & VBLKMASK;
-		n = sblksize(fs, size, lbn) - (off & ~VBLKMASK);
+		n = sblksize(&fs, size, lbn) - (off & ~VBLKMASK);
 		if (n > VBLKSIZE)
 			n = VBLKSIZE;
 		if (blkmap != vbaddr) {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201107112043.p6BKhxOX008571>