* [PATCH 0/5 V2] fix (mostly) minor nits spotted by gcc sanitization
@ 2015-10-09 19:48 Eric Sandeen
2015-10-09 19:49 ` [PATCH 1/5] libxfs: avoid negative (and full-width) shifts in radix-tree.c Eric Sandeen
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Eric Sandeen @ 2015-10-09 19:48 UTC (permalink / raw)
To: xfs
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
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/5] libxfs: avoid negative (and full-width) shifts in radix-tree.c 2015-10-09 19:48 [PATCH 0/5 V2] fix (mostly) minor nits spotted by gcc sanitization Eric Sandeen @ 2015-10-09 19:49 ` Eric Sandeen 2015-10-09 19:51 ` [PATCH 2/5] xfs_repair: fix unaligned accesses Eric Sandeen ` (3 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Eric Sandeen @ 2015-10-09 19:49 UTC (permalink / raw) To: xfs 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] xfs_repair: fix unaligned accesses 2015-10-09 19:48 [PATCH 0/5 V2] fix (mostly) minor nits spotted by gcc sanitization Eric Sandeen 2015-10-09 19:49 ` [PATCH 1/5] libxfs: avoid negative (and full-width) shifts in radix-tree.c Eric Sandeen @ 2015-10-09 19:51 ` Eric Sandeen 2015-10-09 20:08 ` Brian Foster 2015-10-09 19:52 ` [PATCH 3/5] xfs_logprint: fix some " Eric Sandeen ` (2 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Eric Sandeen @ 2015-10-09 19:51 UTC (permalink / raw) To: xfs 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] xfs_repair: fix unaligned accesses 2015-10-09 19:51 ` [PATCH 2/5] xfs_repair: fix unaligned accesses Eric Sandeen @ 2015-10-09 20:08 ` Brian Foster 2015-10-13 0:35 ` Dave Chinner 0 siblings, 1 reply; 10+ messages in thread From: Brian Foster @ 2015-10-09 20:08 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] xfs_repair: fix unaligned accesses 2015-10-09 20:08 ` Brian Foster @ 2015-10-13 0:35 ` Dave Chinner 0 siblings, 0 replies; 10+ messages in thread From: Dave Chinner @ 2015-10-13 0:35 UTC (permalink / raw) To: Brian Foster; +Cc: Eric Sandeen, xfs 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/5] xfs_logprint: fix some unaligned accesses 2015-10-09 19:48 [PATCH 0/5 V2] fix (mostly) minor nits spotted by gcc sanitization Eric Sandeen 2015-10-09 19:49 ` [PATCH 1/5] libxfs: avoid negative (and full-width) shifts in radix-tree.c Eric Sandeen 2015-10-09 19:51 ` [PATCH 2/5] xfs_repair: fix unaligned accesses Eric Sandeen @ 2015-10-09 19:52 ` Eric Sandeen 2015-10-09 20:08 ` Brian Foster 2015-10-09 19:53 ` [PATCH 4/5] xfs_metadump: Fix " Eric Sandeen 2015-10-09 19:53 ` [PATCH 5/5] xfs_repair: fix left-shift overflows Eric Sandeen 4 siblings, 1 reply; 10+ messages in thread From: Eric Sandeen @ 2015-10-09 19:52 UTC (permalink / raw) To: xfs 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/5] xfs_logprint: fix some unaligned accesses 2015-10-09 19:52 ` [PATCH 3/5] xfs_logprint: fix some " Eric Sandeen @ 2015-10-09 20:08 ` Brian Foster 0 siblings, 0 replies; 10+ messages in thread From: Brian Foster @ 2015-10-09 20:08 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/5] xfs_metadump: Fix unaligned accesses 2015-10-09 19:48 [PATCH 0/5 V2] fix (mostly) minor nits spotted by gcc sanitization Eric Sandeen ` (2 preceding siblings ...) 2015-10-09 19:52 ` [PATCH 3/5] xfs_logprint: fix some " Eric Sandeen @ 2015-10-09 19:53 ` Eric Sandeen 2015-10-09 20:08 ` Brian Foster 2015-10-09 19:53 ` [PATCH 5/5] xfs_repair: fix left-shift overflows Eric Sandeen 4 siblings, 1 reply; 10+ messages in thread From: Eric Sandeen @ 2015-10-09 19:53 UTC (permalink / raw) To: xfs 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 4/5] xfs_metadump: Fix unaligned accesses 2015-10-09 19:53 ` [PATCH 4/5] xfs_metadump: Fix " Eric Sandeen @ 2015-10-09 20:08 ` Brian Foster 0 siblings, 0 replies; 10+ messages in thread From: Brian Foster @ 2015-10-09 20:08 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 5/5] xfs_repair: fix left-shift overflows 2015-10-09 19:48 [PATCH 0/5 V2] fix (mostly) minor nits spotted by gcc sanitization Eric Sandeen ` (3 preceding siblings ...) 2015-10-09 19:53 ` [PATCH 4/5] xfs_metadump: Fix " Eric Sandeen @ 2015-10-09 19:53 ` Eric Sandeen 4 siblings, 0 replies; 10+ messages in thread From: Eric Sandeen @ 2015-10-09 19:53 UTC (permalink / raw) To: xfs 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-10-13 0:35 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-09 19:48 [PATCH 0/5 V2] fix (mostly) minor nits spotted by gcc sanitization Eric Sandeen 2015-10-09 19:49 ` [PATCH 1/5] libxfs: avoid negative (and full-width) shifts in radix-tree.c Eric Sandeen 2015-10-09 19:51 ` [PATCH 2/5] xfs_repair: fix unaligned accesses Eric Sandeen 2015-10-09 20:08 ` Brian Foster 2015-10-13 0:35 ` Dave Chinner 2015-10-09 19:52 ` [PATCH 3/5] xfs_logprint: fix some " Eric Sandeen 2015-10-09 20:08 ` Brian Foster 2015-10-09 19:53 ` [PATCH 4/5] xfs_metadump: Fix " Eric Sandeen 2015-10-09 20:08 ` Brian Foster 2015-10-09 19:53 ` [PATCH 5/5] xfs_repair: fix left-shift overflows Eric Sandeen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox