[PATCH 0/5 V2] fix (mostly) minor nits spotted by gcc sanitization

[PATCH 0/5 V2] fix (mostly) minor nits spotted by gcc sanitization

[Eric Sandeen]

newer gcc supports -fsanitize=undefined, which spits out
errors when the code does something ... undefined.

These are a handful of the (IMHO) obvious and unobtrusive
fixes that knock down the noise.

One more patch than last time, found a couple things in
metadump as well.

Thanks,
-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/5] libxfs: avoid negative (and full-width) shifts in radix-tree.c

[Eric Sandeen]

Pull this commit over from the kernel, as libubsan spotted a
bad shift at runtime:

    commit 430d275a399175c7c0673459738979287ec1fd22
    Author: Peter Lund <firefly@vax64.dk>
    Date:   Tue Oct 16 23:29:35 2007 -0700

    avoid negative (and full-width) shifts in radix-tree.c

    Negative shifts are not allowed in C (the result is undefined).  Same thing
    with full-width shifts.

    It works on most platforms but not on the VAX with gcc 4.0.1 (it results in an
    "operand reserved" fault).

    Shifting by more than the width of the value on the left is also not
    allowed.  I think the extra '>> 1' tacked on at the end in the original
    code was an attempt to work around that.  Getting rid of that is an extra
    feature of this patch.

    Here's the chapter and verse, taken from the final draft of the C99
    standard ("6.5.7 Bitwise shift operators", paragraph 3):

      "The integer promotions are performed on each of the operands. The
      type of the result is that of the promoted left operand. If the
      value of the right operand is negative or is greater than or equal
      to the width of the promoted left operand, the behavior is
      undefined."

    Thank you to Jan-Benedict Glaw, Christoph Hellwig, Maciej Rozycki, Pekka
    Enberg, Andreas Schwab, and Christoph Lameter for review.  Special thanks
    to Andreas for spotting that my fix only removed half the undefined
    behaviour.

    Signed-off-by: Peter Lund <firefly@vax64.dk>

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 libxfs/radix-tree.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/libxfs/radix-tree.c b/libxfs/radix-tree.c
index 4d44ab4..eef9c36 100644
--- a/libxfs/radix-tree.c
+++ b/libxfs/radix-tree.c
@@ -784,12 +784,14 @@ int radix_tree_tagged(struct radix_tree_root *root, unsigned int tag)
 
 static unsigned long __maxindex(unsigned int height)
 {
-	unsigned int tmp = height * RADIX_TREE_MAP_SHIFT;
-	unsigned long index = (~0UL >> (RADIX_TREE_INDEX_BITS - tmp - 1)) >> 1;
-
-	if (tmp >= RADIX_TREE_INDEX_BITS)
-		index = ~0UL;
-	return index;
+	unsigned int width = height * RADIX_TREE_MAP_SHIFT;
+	int shift = RADIX_TREE_INDEX_BITS - width;
+
+	if (shift < 0)
+		return ~0UL;
+	if (shift >= BITS_PER_LONG)
+		return 0UL;
+	return ~0UL >> shift;
 }
 
 static void radix_tree_init_maxindex(void)
-- 
2.6.1


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/5] xfs_repair: fix unaligned accesses

[Eric Sandeen]

This fixes some unaligned accesses spotted by libubsan in repair.

See Documentation/unaligned-memory-access.txt in the kernel
tree for why these can be a problem.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: 
Add note about why ...
Add another in libxfs_bmbt_disk_get_all
Fix mistaken double-swap in dinode.c in original patch

 include/libxfs.h  |  4 ++--
 repair/dinode.c   | 47 ++++++++++++++++++++++++-----------------------
 repair/prefetch.c |  4 ++--
 3 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/include/libxfs.h b/include/libxfs.h
index b1604e2..52fb483 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -206,8 +206,8 @@ libxfs_bmbt_disk_get_all(
 {
 	struct xfs_bmbt_rec_host hrec;
 
-	hrec.l0 = be64_to_cpu(rp->l0);
-	hrec.l1 = be64_to_cpu(rp->l1);
+	hrec.l0 = get_unaligned_be64(&rp->l0);
+	hrec.l1 = get_unaligned_be64(&rp->l1);
 	libxfs_bmbt_get_all(&hrec, irec);
 }
 
diff --git a/repair/dinode.c b/repair/dinode.c
index f78f907..f99cba3 100644
--- a/repair/dinode.c
+++ b/repair/dinode.c
@@ -960,15 +960,17 @@ _("bad numrecs 0 in inode %" PRIu64 " bmap btree root block\n"),
 		 * btree, we'd do it right here.  For now, if there's a
 		 * problem, we'll bail out and presumably clear the inode.
 		 */
-		if (!verify_dfsbno(mp, be64_to_cpu(pp[i])))  {
-			do_warn(_("bad bmap btree ptr 0x%llx in ino %" PRIu64 "\n"),
-			       (unsigned long long) be64_to_cpu(pp[i]), lino);
+		if (!verify_dfsbno(mp, get_unaligned_be64(&pp[i])))  {
+			do_warn(
+("bad bmap btree ptr 0x%" PRIx64 " in ino %" PRIu64 "\n"),
+				get_unaligned_be64(&pp[i]), lino);
 			return(1);
 		}
 
-		if (scan_lbtree(be64_to_cpu(pp[i]), level, scan_bmapbt, type,
-				whichfork, lino, tot, nex, blkmapp, &cursor,
-				1, check_dups, magic, &xfs_bmbt_buf_ops))
+		if (scan_lbtree(get_unaligned_be64(&pp[i]), level, scan_bmapbt,
+				type, whichfork, lino, tot, nex, blkmapp,
+				&cursor, 1, check_dups, magic,
+				&xfs_bmbt_buf_ops))
 			return(1);
 		/*
 		 * fix key (offset) mismatches between the keys in root
@@ -977,28 +979,27 @@ _("bad numrecs 0 in inode %" PRIu64 " bmap btree root block\n"),
 		 * blocks but the parent hasn't been updated
 		 */
 		if (!check_dups && cursor.level[level-1].first_key !=
-					be64_to_cpu(pkey[i].br_startoff))  {
+				   get_unaligned_be64(&pkey[i].br_startoff)) {
 			if (!no_modify)  {
 				do_warn(
-	_("correcting key in bmbt root (was %llu, now %" PRIu64") in inode "
-	  "%" PRIu64" %s fork\n"),
-				       (unsigned long long)
-					       be64_to_cpu(pkey[i].br_startoff),
-					cursor.level[level-1].first_key,
-					XFS_AGINO_TO_INO(mp, agno, ino),
-					forkname);
+_("correcting key in bmbt root (was %" PRIu64 ", now %" PRIu64") in inode "
+  "%" PRIu64" %s fork\n"),
+				       get_unaligned_be64(&pkey[i].br_startoff),
+				       cursor.level[level-1].first_key,
+				       XFS_AGINO_TO_INO(mp, agno, ino),
+				       forkname);
 				*dirty = 1;
-				pkey[i].br_startoff = cpu_to_be64(
-					cursor.level[level-1].first_key);
+				put_unaligned_be64(
+					cursor.level[level-1].first_key,
+					&pkey[i].br_startoff);
 			} else  {
 				do_warn(
-	_("bad key in bmbt root (is %llu, would reset to %" PRIu64 ") in inode "
-	  "%" PRIu64 " %s fork\n"),
-				       (unsigned long long)
-					       be64_to_cpu(pkey[i].br_startoff),
-					cursor.level[level-1].first_key,
-					XFS_AGINO_TO_INO(mp, agno, ino),
-					forkname);
+_("bad key in bmbt root (is %" PRIu64 ", would reset to %" PRIu64 ") in inode "
+  "%" PRIu64 " %s fork\n"),
+				       get_unaligned_be64(&pkey[i].br_startoff),
+				       cursor.level[level-1].first_key,
+				       XFS_AGINO_TO_INO(mp, agno, ino),
+				       forkname);
 			}
 		}
 		/*
diff --git a/repair/prefetch.c b/repair/prefetch.c
index 32ec55e..52238ca 100644
--- a/repair/prefetch.c
+++ b/repair/prefetch.c
@@ -330,7 +330,7 @@ pf_scanfunc_bmap(
 	pp = XFS_BMBT_PTR_ADDR(mp, block, 1, mp->m_bmap_dmxr[1]);
 
 	for (i = 0; i < numrecs; i++) {
-		dbno = be64_to_cpu(pp[i]);
+		dbno = get_unaligned_be64(&pp[i]);
 		if (!verify_dfsbno(mp, dbno))
 			return 0;
 		if (!pf_scan_lbtree(dbno, level, isadir, args, pf_scanfunc_bmap))
@@ -372,7 +372,7 @@ pf_read_btinode(
 	pp = XFS_BMDR_PTR_ADDR(dib, 1, xfs_bmdr_maxrecs(dsize, 0));
 
 	for (i = 0; i < numrecs; i++) {
-		dbno = be64_to_cpu(pp[i]);
+		dbno = get_unaligned_be64(&pp[i]);
 		if (!verify_dfsbno(mp, dbno))
 			break;
 		if (!pf_scan_lbtree(dbno, level, isadir, args, pf_scanfunc_bmap))
-- 
2.6.1


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/5] xfs_logprint: fix some unaligned accesses

[Eric Sandeen]

This routine had a fair bit of gyration to avoid unaligned
accesses, but didn't fix them all.
Fix some more spotted at runtime by libubsan.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: Change variable scope, remove extraneous hunk

 logprint/log_misc.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/logprint/log_misc.c b/logprint/log_misc.c
index d76145c..4cdcbec 100644
--- a/logprint/log_misc.c
+++ b/logprint/log_misc.c
@@ -243,9 +243,6 @@ int
 xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
 {
     xfs_buf_log_format_t *f;
-    xfs_agi_t		 *agi;
-    xfs_agf_t		 *agf;
-    xfs_disk_dquot_t	 *dq;
     xlog_op_header_t	 *head = NULL;
     int			 num, skip;
     int			 super_block = 0;
@@ -325,7 +322,11 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
 		}
 		super_block = 0;
 	} else if (be32_to_cpu(*(__be32 *)(*ptr)) == XFS_AGI_MAGIC) {
-		agi = (xfs_agi_t *)(*ptr);
+  	  	struct xfs_agi	*agi, agi_s;
+
+		/* memmove because *ptr may not be 8-byte aligned */
+		agi = &agi_s;
+		memmove(agi, *ptr, sizeof(struct xfs_agi));
 		printf(_("AGI Buffer: XAGI  "));
 		/*
 		 * v4 filesystems only contain the fields before the uuid.
@@ -375,7 +376,11 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
 			}
 		}
 	} else if (be32_to_cpu(*(__be32 *)(*ptr)) == XFS_AGF_MAGIC) {
-		agf = (xfs_agf_t *)(*ptr);
+    		struct xfs_agf	*agf, agf_s;
+
+		/* memmove because *ptr may not be 8-byte aligned */
+		agf = &agf_s;
+		memmove(agf, *ptr, sizeof(struct xfs_agf));
 		printf(_("AGF Buffer: XAGF  "));
 		/*
 		 * v4 filesystems only contain the fields before the uuid.
@@ -408,7 +413,11 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
 				be32_to_cpu(agf->agf_longest));
 		}
 	} else if (be32_to_cpu(*(__be32 *)(*ptr)) == XFS_DQUOT_MAGIC) {
-		dq = (xfs_disk_dquot_t *)(*ptr);
+		struct xfs_disk_dquot *dq, dq_s;
+
+		/* memmove because *ptr may not be 8-byte aligned */
+		dq = &dq_s;
+		memmove(dq, *ptr, sizeof(struct xfs_disk_dquot));
 		printf(_("DQUOT Buffer: DQ  "));
 		if (be32_to_cpu(head->oh_len) <
 				sizeof(xfs_disk_dquot_t)) {
-- 
2.6.1


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 4/5] xfs_metadump: Fix unaligned accesses

[Eric Sandeen]

This fixes some unaligned accesses spotted by libubsan in
xfs_metadump.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 db/metadump.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/db/metadump.c b/db/metadump.c
index af96e12..39f893d 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -1872,8 +1872,8 @@ scanfunc_bmap(
 		xfs_agnumber_t	ag;
 		xfs_agblock_t	bno;
 
-		ag = XFS_FSB_TO_AGNO(mp, be64_to_cpu(pp[i]));
-		bno = XFS_FSB_TO_AGBNO(mp, be64_to_cpu(pp[i]));
+		ag = XFS_FSB_TO_AGNO(mp, get_unaligned_be64(&pp[i]));
+		bno = XFS_FSB_TO_AGBNO(mp, get_unaligned_be64(&pp[i]));
 
 		if (bno == 0 || bno > mp->m_sb.sb_agblocks ||
 				ag > mp->m_sb.sb_agcount) {
@@ -1938,8 +1938,8 @@ process_btinode(
 		xfs_agnumber_t	ag;
 		xfs_agblock_t	bno;
 
-		ag = XFS_FSB_TO_AGNO(mp, be64_to_cpu(pp[i]));
-		bno = XFS_FSB_TO_AGBNO(mp, be64_to_cpu(pp[i]));
+		ag = XFS_FSB_TO_AGNO(mp, get_unaligned_be64(&pp[i]));
+		bno = XFS_FSB_TO_AGBNO(mp, get_unaligned_be64(&pp[i]));
 
 		if (bno == 0 || bno > mp->m_sb.sb_agblocks ||
 				ag > mp->m_sb.sb_agcount) {
-- 
2.6.1


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 5/5] xfs_repair: fix left-shift overflows

[Eric Sandeen]

pmask in struct parent_list is a __uint64_t, but in some places
we populated it with "1LL << shift" where shift could be up
to 63; this really needs to be a 1ULL type for this to be correct.

Also spotted by libubsan...

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 repair/incore_ino.c | 14 +++++++-------
 repair/prefetch.c   |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/repair/incore_ino.c b/repair/incore_ino.c
index 32d7678..1898257 100644
--- a/repair/incore_ino.c
+++ b/repair/incore_ino.c
@@ -625,7 +625,7 @@ set_inode_parent(
 		else
 			irec->ino_un.plist = ptbl;
 
-		ptbl->pmask = 1LL << offset;
+		ptbl->pmask = 1ULL << offset;
 		ptbl->pentries = (xfs_ino_t*)memalign(sizeof(xfs_ino_t),
 							sizeof(xfs_ino_t));
 		if (!ptbl->pentries)
@@ -638,8 +638,8 @@ set_inode_parent(
 		return;
 	}
 
-	if (ptbl->pmask & (1LL << offset))  {
-		bitmask = 1LL;
+	if (ptbl->pmask & (1ULL << offset))  {
+		bitmask = 1ULL;
 		target = 0;
 
 		for (i = 0; i < offset; i++)  {
@@ -655,7 +655,7 @@ set_inode_parent(
 		return;
 	}
 
-	bitmask = 1LL;
+	bitmask = 1ULL;
 	cnt = target = 0;
 
 	for (i = 0; i < XFS_INODES_PER_CHUNK; i++)  {
@@ -691,7 +691,7 @@ set_inode_parent(
 	ptbl->cnt++;
 #endif
 	ptbl->pentries[target] = parent;
-	ptbl->pmask |= (1LL << offset);
+	ptbl->pmask |= (1ULL << offset);
 }
 
 xfs_ino_t
@@ -707,8 +707,8 @@ get_inode_parent(ino_tree_node_t *irec, int offset)
 	else
 		ptbl = irec->ino_un.plist;
 
-	if (ptbl->pmask & (1LL << offset))  {
-		bitmask = 1LL;
+	if (ptbl->pmask & (1ULL << offset))  {
+		bitmask = 1ULL;
 		target = 0;
 
 		for (i = 0; i < offset; i++)  {
diff --git a/repair/prefetch.c b/repair/prefetch.c
index 52238ca..b11dcb3 100644
--- a/repair/prefetch.c
+++ b/repair/prefetch.c
@@ -762,7 +762,7 @@ pf_queuing_worker(
 			 * sparse state in cluster sized chunks as cluster size
 			 * is the min. granularity of sparse irec regions.
 			 */
-			if ((sparse & ((1 << inodes_per_cluster) - 1)) == 0)
+			if ((sparse & ((1ULL << inodes_per_cluster) - 1)) == 0)
 				pf_queue_io(args, &map, 1,
 					    (cur_irec->ino_isa_dir != 0) ?
 					     B_DIR_INODE : B_INODE);
-- 
2.6.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/5] xfs_repair: fix unaligned accesses

[Brian Foster]

On Fri, Oct 09, 2015 at 02:51:09PM -0500, Eric Sandeen wrote:
> This fixes some unaligned accesses spotted by libubsan in repair.
> 
> See Documentation/unaligned-memory-access.txt in the kernel
> tree for why these can be a problem.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

> 
> V2: 
> Add note about why ...
> Add another in libxfs_bmbt_disk_get_all
> Fix mistaken double-swap in dinode.c in original patch
> 
>  include/libxfs.h  |  4 ++--
>  repair/dinode.c   | 47 ++++++++++++++++++++++++-----------------------
>  repair/prefetch.c |  4 ++--
>  3 files changed, 28 insertions(+), 27 deletions(-)
> 
> diff --git a/include/libxfs.h b/include/libxfs.h
> index b1604e2..52fb483 100644
> --- a/include/libxfs.h
> +++ b/include/libxfs.h
> @@ -206,8 +206,8 @@ libxfs_bmbt_disk_get_all(
>  {
>  	struct xfs_bmbt_rec_host hrec;
>  
> -	hrec.l0 = be64_to_cpu(rp->l0);
> -	hrec.l1 = be64_to_cpu(rp->l1);
> +	hrec.l0 = get_unaligned_be64(&rp->l0);
> +	hrec.l1 = get_unaligned_be64(&rp->l1);
>  	libxfs_bmbt_get_all(&hrec, irec);
>  }
>  
> diff --git a/repair/dinode.c b/repair/dinode.c
> index f78f907..f99cba3 100644
> --- a/repair/dinode.c
> +++ b/repair/dinode.c
> @@ -960,15 +960,17 @@ _("bad numrecs 0 in inode %" PRIu64 " bmap btree root block\n"),
>  		 * btree, we'd do it right here.  For now, if there's a
>  		 * problem, we'll bail out and presumably clear the inode.
>  		 */
> -		if (!verify_dfsbno(mp, be64_to_cpu(pp[i])))  {
> -			do_warn(_("bad bmap btree ptr 0x%llx in ino %" PRIu64 "\n"),
> -			       (unsigned long long) be64_to_cpu(pp[i]), lino);
> +		if (!verify_dfsbno(mp, get_unaligned_be64(&pp[i])))  {
> +			do_warn(
> +("bad bmap btree ptr 0x%" PRIx64 " in ino %" PRIu64 "\n"),
> +				get_unaligned_be64(&pp[i]), lino);
>  			return(1);
>  		}
>  
> -		if (scan_lbtree(be64_to_cpu(pp[i]), level, scan_bmapbt, type,
> -				whichfork, lino, tot, nex, blkmapp, &cursor,
> -				1, check_dups, magic, &xfs_bmbt_buf_ops))
> +		if (scan_lbtree(get_unaligned_be64(&pp[i]), level, scan_bmapbt,
> +				type, whichfork, lino, tot, nex, blkmapp,
> +				&cursor, 1, check_dups, magic,
> +				&xfs_bmbt_buf_ops))
>  			return(1);
>  		/*
>  		 * fix key (offset) mismatches between the keys in root
> @@ -977,28 +979,27 @@ _("bad numrecs 0 in inode %" PRIu64 " bmap btree root block\n"),
>  		 * blocks but the parent hasn't been updated
>  		 */
>  		if (!check_dups && cursor.level[level-1].first_key !=
> -					be64_to_cpu(pkey[i].br_startoff))  {
> +				   get_unaligned_be64(&pkey[i].br_startoff)) {
>  			if (!no_modify)  {
>  				do_warn(
> -	_("correcting key in bmbt root (was %llu, now %" PRIu64") in inode "
> -	  "%" PRIu64" %s fork\n"),
> -				       (unsigned long long)
> -					       be64_to_cpu(pkey[i].br_startoff),
> -					cursor.level[level-1].first_key,
> -					XFS_AGINO_TO_INO(mp, agno, ino),
> -					forkname);
> +_("correcting key in bmbt root (was %" PRIu64 ", now %" PRIu64") in inode "
> +  "%" PRIu64" %s fork\n"),
> +				       get_unaligned_be64(&pkey[i].br_startoff),
> +				       cursor.level[level-1].first_key,
> +				       XFS_AGINO_TO_INO(mp, agno, ino),
> +				       forkname);
>  				*dirty = 1;
> -				pkey[i].br_startoff = cpu_to_be64(
> -					cursor.level[level-1].first_key);
> +				put_unaligned_be64(
> +					cursor.level[level-1].first_key,
> +					&pkey[i].br_startoff);
>  			} else  {
>  				do_warn(
> -	_("bad key in bmbt root (is %llu, would reset to %" PRIu64 ") in inode "
> -	  "%" PRIu64 " %s fork\n"),
> -				       (unsigned long long)
> -					       be64_to_cpu(pkey[i].br_startoff),
> -					cursor.level[level-1].first_key,
> -					XFS_AGINO_TO_INO(mp, agno, ino),
> -					forkname);
> +_("bad key in bmbt root (is %" PRIu64 ", would reset to %" PRIu64 ") in inode "
> +  "%" PRIu64 " %s fork\n"),
> +				       get_unaligned_be64(&pkey[i].br_startoff),
> +				       cursor.level[level-1].first_key,
> +				       XFS_AGINO_TO_INO(mp, agno, ino),
> +				       forkname);
>  			}
>  		}
>  		/*
> diff --git a/repair/prefetch.c b/repair/prefetch.c
> index 32ec55e..52238ca 100644
> --- a/repair/prefetch.c
> +++ b/repair/prefetch.c
> @@ -330,7 +330,7 @@ pf_scanfunc_bmap(
>  	pp = XFS_BMBT_PTR_ADDR(mp, block, 1, mp->m_bmap_dmxr[1]);
>  
>  	for (i = 0; i < numrecs; i++) {
> -		dbno = be64_to_cpu(pp[i]);
> +		dbno = get_unaligned_be64(&pp[i]);
>  		if (!verify_dfsbno(mp, dbno))
>  			return 0;
>  		if (!pf_scan_lbtree(dbno, level, isadir, args, pf_scanfunc_bmap))
> @@ -372,7 +372,7 @@ pf_read_btinode(
>  	pp = XFS_BMDR_PTR_ADDR(dib, 1, xfs_bmdr_maxrecs(dsize, 0));
>  
>  	for (i = 0; i < numrecs; i++) {
> -		dbno = be64_to_cpu(pp[i]);
> +		dbno = get_unaligned_be64(&pp[i]);
>  		if (!verify_dfsbno(mp, dbno))
>  			break;
>  		if (!pf_scan_lbtree(dbno, level, isadir, args, pf_scanfunc_bmap))
> -- 
> 2.6.1
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/5] xfs_logprint: fix some unaligned accesses

[Brian Foster]

On Fri, Oct 09, 2015 at 02:52:16PM -0500, Eric Sandeen wrote:
> This routine had a fair bit of gyration to avoid unaligned
> accesses, but didn't fix them all.
> Fix some more spotted at runtime by libubsan.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> V2: Change variable scope, remove extraneous hunk
> 

Thanks!

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  logprint/log_misc.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/logprint/log_misc.c b/logprint/log_misc.c
> index d76145c..4cdcbec 100644
> --- a/logprint/log_misc.c
> +++ b/logprint/log_misc.c
> @@ -243,9 +243,6 @@ int
>  xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
>  {
>      xfs_buf_log_format_t *f;
> -    xfs_agi_t		 *agi;
> -    xfs_agf_t		 *agf;
> -    xfs_disk_dquot_t	 *dq;
>      xlog_op_header_t	 *head = NULL;
>      int			 num, skip;
>      int			 super_block = 0;
> @@ -325,7 +322,11 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
>  		}
>  		super_block = 0;
>  	} else if (be32_to_cpu(*(__be32 *)(*ptr)) == XFS_AGI_MAGIC) {
> -		agi = (xfs_agi_t *)(*ptr);
> +  	  	struct xfs_agi	*agi, agi_s;
> +
> +		/* memmove because *ptr may not be 8-byte aligned */
> +		agi = &agi_s;
> +		memmove(agi, *ptr, sizeof(struct xfs_agi));
>  		printf(_("AGI Buffer: XAGI  "));
>  		/*
>  		 * v4 filesystems only contain the fields before the uuid.
> @@ -375,7 +376,11 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
>  			}
>  		}
>  	} else if (be32_to_cpu(*(__be32 *)(*ptr)) == XFS_AGF_MAGIC) {
> -		agf = (xfs_agf_t *)(*ptr);
> +    		struct xfs_agf	*agf, agf_s;
> +
> +		/* memmove because *ptr may not be 8-byte aligned */
> +		agf = &agf_s;
> +		memmove(agf, *ptr, sizeof(struct xfs_agf));
>  		printf(_("AGF Buffer: XAGF  "));
>  		/*
>  		 * v4 filesystems only contain the fields before the uuid.
> @@ -408,7 +413,11 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
>  				be32_to_cpu(agf->agf_longest));
>  		}
>  	} else if (be32_to_cpu(*(__be32 *)(*ptr)) == XFS_DQUOT_MAGIC) {
> -		dq = (xfs_disk_dquot_t *)(*ptr);
> +		struct xfs_disk_dquot *dq, dq_s;
> +
> +		/* memmove because *ptr may not be 8-byte aligned */
> +		dq = &dq_s;
> +		memmove(dq, *ptr, sizeof(struct xfs_disk_dquot));
>  		printf(_("DQUOT Buffer: DQ  "));
>  		if (be32_to_cpu(head->oh_len) <
>  				sizeof(xfs_disk_dquot_t)) {
> -- 
> 2.6.1
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 4/5] xfs_metadump: Fix unaligned accesses

[Brian Foster]

On Fri, Oct 09, 2015 at 02:53:00PM -0500, Eric Sandeen wrote:
> This fixes some unaligned accesses spotted by libubsan in
> xfs_metadump.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Looks good:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  db/metadump.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/db/metadump.c b/db/metadump.c
> index af96e12..39f893d 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -1872,8 +1872,8 @@ scanfunc_bmap(
>  		xfs_agnumber_t	ag;
>  		xfs_agblock_t	bno;
>  
> -		ag = XFS_FSB_TO_AGNO(mp, be64_to_cpu(pp[i]));
> -		bno = XFS_FSB_TO_AGBNO(mp, be64_to_cpu(pp[i]));
> +		ag = XFS_FSB_TO_AGNO(mp, get_unaligned_be64(&pp[i]));
> +		bno = XFS_FSB_TO_AGBNO(mp, get_unaligned_be64(&pp[i]));
>  
>  		if (bno == 0 || bno > mp->m_sb.sb_agblocks ||
>  				ag > mp->m_sb.sb_agcount) {
> @@ -1938,8 +1938,8 @@ process_btinode(
>  		xfs_agnumber_t	ag;
>  		xfs_agblock_t	bno;
>  
> -		ag = XFS_FSB_TO_AGNO(mp, be64_to_cpu(pp[i]));
> -		bno = XFS_FSB_TO_AGBNO(mp, be64_to_cpu(pp[i]));
> +		ag = XFS_FSB_TO_AGNO(mp, get_unaligned_be64(&pp[i]));
> +		bno = XFS_FSB_TO_AGBNO(mp, get_unaligned_be64(&pp[i]));
>  
>  		if (bno == 0 || bno > mp->m_sb.sb_agblocks ||
>  				ag > mp->m_sb.sb_agcount) {
> -- 
> 2.6.1
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/5] xfs_repair: fix unaligned accesses

[Dave Chinner]

On Fri, Oct 09, 2015 at 04:08:22PM -0400, Brian Foster wrote:
> On Fri, Oct 09, 2015 at 02:51:09PM -0500, Eric Sandeen wrote:
> > This fixes some unaligned accesses spotted by libubsan in repair.
> > 
> > See Documentation/unaligned-memory-access.txt in the kernel
> > tree for why these can be a problem.
> > 
> > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > ---
> 
> Reviewed-by: Brian Foster <bfoster@redhat.com>
....
> > @@ -960,15 +960,17 @@ _("bad numrecs 0 in inode %" PRIu64 " bmap btree root block\n"),
> >  		 * btree, we'd do it right here.  For now, if there's a
> >  		 * problem, we'll bail out and presumably clear the inode.
> >  		 */
> > -		if (!verify_dfsbno(mp, be64_to_cpu(pp[i])))  {
> > -			do_warn(_("bad bmap btree ptr 0x%llx in ino %" PRIu64 "\n"),
> > -			       (unsigned long long) be64_to_cpu(pp[i]), lino);
> > +		if (!verify_dfsbno(mp, get_unaligned_be64(&pp[i])))  {
> > +			do_warn(
> > +("bad bmap btree ptr 0x%" PRIx64 " in ino %" PRIu64 "\n"),
> > +				get_unaligned_be64(&pp[i]), lino);

drops the "_" from the translation string. I'll fix it on commit.

/me can't help but think that a local variable or two would make
this code so much more readable....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs