xfs
[Top] [All Lists]

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

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH 2/5] xfs_repair: fix unaligned accesses
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Fri, 9 Oct 2015 16:08:22 -0400
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <56181AAD.9080505@xxxxxxxxxxx>
References: <56181A17.9080503@xxxxxxxxxxx> <56181AAD.9080505@xxxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
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@xxxxxxxxxx>
> ---

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

> 
> 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@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>