| Message ID | d380b0b8-b88f-fc96-8515-c5bd97918317@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xfs: add verifier context structure | expand |
On Wed, Dec 05, 2018 at 03:09:48PM -0600, Eric Sandeen wrote: > Modify CRC checking functions to set __this_address into the > verifier context failaddr vc->fa using new macro XFS_BADCRC_RETURN, > and pass that to failure handlers as well. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > fs/xfs/libxfs/xfs_alloc.c | 4 ++-- > fs/xfs/libxfs/xfs_alloc_btree.c | 2 +- > fs/xfs/libxfs/xfs_attr_leaf.c | 2 +- > fs/xfs/libxfs/xfs_bmap_btree.c | 2 +- > fs/xfs/libxfs/xfs_cksum.h | 5 ++++- > fs/xfs/libxfs/xfs_da_btree.c | 3 +-- > fs/xfs/libxfs/xfs_dir2_block.c | 2 +- > fs/xfs/libxfs/xfs_dir2_data.c | 2 +- > fs/xfs/libxfs/xfs_dir2_leaf.c | 2 +- > fs/xfs/libxfs/xfs_dir2_node.c | 2 +- > fs/xfs/libxfs/xfs_ialloc.c | 2 +- > fs/xfs/libxfs/xfs_ialloc_btree.c | 2 +- > fs/xfs/libxfs/xfs_refcount_btree.c | 2 +- > fs/xfs/libxfs/xfs_rmap_btree.c | 2 +- > fs/xfs/libxfs/xfs_symlink_remote.c | 2 +- > fs/xfs/libxfs/xfs_types.h | 12 ++++++++++-- > fs/xfs/xfs_linux.h | 7 ------- > 17 files changed, 29 insertions(+), 26 deletions(-) > ... > diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h > index 29b0d354d9b7..ab045e8dfcb9 100644 > --- a/fs/xfs/libxfs/xfs_types.h > +++ b/fs/xfs/libxfs/xfs_types.h > @@ -45,8 +45,16 @@ struct xfs_vc { > xfs_failaddr_t fa; > }; > > -#define XFS_CORRUPTED_RETURN(vc) ({(vc)->fa = __this_address; false;}) > -#define XFS_VERIFIED_RETURN(vc) ({(vc)->fa = NULL; true;}) > +/* > + * Return the address of a label. Use barrier() so that the optimizer > + * won't reorder code to refactor the error jumpouts into a single > + * return, which throws off the reported address. > + */ > +#define __this_address ({ __label__ __here; __here: barrier(); &&__here; }) > + FYI, minor whitespace damage on the line above. > +#define XFS_CORRUPTED_RETURN(vc) ({(vc)->fa = __this_address; false;}) > +#define XFS_BADCRC_RETURN(vc) ({(vc)->fa = __this_address; false;}) > +#define XFS_VERIFIED_RETURN(vc) ({(vc)->fa = NULL; true;}) > A couple high level comments.. I don't particularly care that much whether we bury function returns in the macro or open-code it, but the macro naming suggests the former (based on precedent of other such macros in XFS) while we implement the latter. If there's objection to a return within a macro, perhaps a reasonable compromise between this and the common pattern of having to return on a separate line is to tweak the macros to never clobber an existing error and update the verifiers to check/return failure state at opportune points. For example: ... if (!uuid_equal(&agfl->agfl_uuid, &mp->m_sb.sb_meta_uuid)) XFS_VC_CORRUPT(vc); if (be32_to_cpu(agfl->agfl_magicnum) != XFS_AGFL_MAGIC) XFS_VC_CORRUPT(vc); if (bp->b_pag && be32_to_cpu(agfl->agfl_seqno) != bp->b_pag->pag_agno) XFS_VC_CORRUPT(vc); ... return vc->fa ? false : true; Of course, that assumes it's safe to keep checking the structure(s) as such in the event of corruption, which perhaps is not ideal. Anyways, we could also just return on a separate line or rename the macros. Just thinking out loud a bit. I'm also a little curious why we have the need for the success macro at all. I've only made a cursory pass at this point, but is there a particular need to set anything in the xfs_vc at the point of a successful return as opposed to just leaving the structure in the initialized state? Brian > /* > * Null values for the types. > diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h > index edbd5a210df2..4141e70bb3fa 100644 > --- a/fs/xfs/xfs_linux.h > +++ b/fs/xfs/xfs_linux.h > @@ -131,13 +131,6 @@ typedef __u32 xfs_nlink_t; > #define SYNCHRONIZE() barrier() > #define __return_address __builtin_return_address(0) > > -/* > - * Return the address of a label. Use barrier() so that the optimizer > - * won't reorder code to refactor the error jumpouts into a single > - * return, which throws off the reported address. > - */ > -#define __this_address ({ __label__ __here; __here: barrier(); &&__here; }) > - > #define XFS_PROJID_DEFAULT 0 > > #define howmany(x, y) (((x)+((y)-1))/(y)) > -- > 2.17.0 > >
On 12/7/18 7:37 AM, Brian Foster wrote: > On Wed, Dec 05, 2018 at 03:09:48PM -0600, Eric Sandeen wrote: >> Modify CRC checking functions to set __this_address into the >> verifier context failaddr vc->fa using new macro XFS_BADCRC_RETURN, >> and pass that to failure handlers as well. >> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com> >> --- >> fs/xfs/libxfs/xfs_alloc.c | 4 ++-- >> fs/xfs/libxfs/xfs_alloc_btree.c | 2 +- >> fs/xfs/libxfs/xfs_attr_leaf.c | 2 +- >> fs/xfs/libxfs/xfs_bmap_btree.c | 2 +- >> fs/xfs/libxfs/xfs_cksum.h | 5 ++++- >> fs/xfs/libxfs/xfs_da_btree.c | 3 +-- >> fs/xfs/libxfs/xfs_dir2_block.c | 2 +- >> fs/xfs/libxfs/xfs_dir2_data.c | 2 +- >> fs/xfs/libxfs/xfs_dir2_leaf.c | 2 +- >> fs/xfs/libxfs/xfs_dir2_node.c | 2 +- >> fs/xfs/libxfs/xfs_ialloc.c | 2 +- >> fs/xfs/libxfs/xfs_ialloc_btree.c | 2 +- >> fs/xfs/libxfs/xfs_refcount_btree.c | 2 +- >> fs/xfs/libxfs/xfs_rmap_btree.c | 2 +- >> fs/xfs/libxfs/xfs_symlink_remote.c | 2 +- >> fs/xfs/libxfs/xfs_types.h | 12 ++++++++++-- >> fs/xfs/xfs_linux.h | 7 ------- >> 17 files changed, 29 insertions(+), 26 deletions(-) >> > ... >> diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h >> index 29b0d354d9b7..ab045e8dfcb9 100644 >> --- a/fs/xfs/libxfs/xfs_types.h >> +++ b/fs/xfs/libxfs/xfs_types.h >> @@ -45,8 +45,16 @@ struct xfs_vc { >> xfs_failaddr_t fa; >> }; >> >> -#define XFS_CORRUPTED_RETURN(vc) ({(vc)->fa = __this_address; false;}) >> -#define XFS_VERIFIED_RETURN(vc) ({(vc)->fa = NULL; true;}) >> +/* >> + * Return the address of a label. Use barrier() so that the optimizer >> + * won't reorder code to refactor the error jumpouts into a single >> + * return, which throws off the reported address. >> + */ >> +#define __this_address ({ __label__ __here; __here: barrier(); &&__here; }) >> + > > FYI, minor whitespace damage on the line above. > >> +#define XFS_CORRUPTED_RETURN(vc) ({(vc)->fa = __this_address; false;}) >> +#define XFS_BADCRC_RETURN(vc) ({(vc)->fa = __this_address; false;}) >> +#define XFS_VERIFIED_RETURN(vc) ({(vc)->fa = NULL; true;}) >> > > A couple high level comments.. > > I don't particularly care that much whether we bury function returns in > the macro or open-code it, but the macro naming suggests the former > (based on precedent of other such macros in XFS) while we implement the > latter. If there's objection to a return within a macro, perhaps a > reasonable compromise between this and the common pattern of having to > return on a separate line is to tweak the macros to never clobber an > existing error and update the verifiers to check/return failure state at > opportune points. For example: > > ... > if (!uuid_equal(&agfl->agfl_uuid, &mp->m_sb.sb_meta_uuid)) > XFS_VC_CORRUPT(vc); > if (be32_to_cpu(agfl->agfl_magicnum) != XFS_AGFL_MAGIC) > XFS_VC_CORRUPT(vc); > if (bp->b_pag && be32_to_cpu(agfl->agfl_seqno) != bp->b_pag->pag_agno) > XFS_VC_CORRUPT(vc); > ... > > return vc->fa ? false : true; > > Of course, that assumes it's safe to keep checking the structure(s) as > such in the event of corruption, which perhaps is not ideal. Anyways, we > could also just return on a separate line or rename the macros. Just > thinking out loud a bit. > > I'm also a little curious why we have the need for the success macro at > all. I've only made a cursory pass at this point, but is there a > particular need to set anything in the xfs_vc at the point of a > successful return as opposed to just leaving the structure in the > initialized state? yeah, it's probably not necessary; it seemed consistent tho. There is some risk to this whole framework that failing to initialize a vc would cause problems that might be difficult to debug, and always marking success upon success seemed "safe." But you're right, not not necessary if it's properly initialized to success at the top of the call chain. -Eric
On Mon, Dec 10, 2018 at 10:00:08AM -0600, Eric Sandeen wrote: > On 12/7/18 7:37 AM, Brian Foster wrote: > > On Wed, Dec 05, 2018 at 03:09:48PM -0600, Eric Sandeen wrote: > >> Modify CRC checking functions to set __this_address into the > >> verifier context failaddr vc->fa using new macro XFS_BADCRC_RETURN, > >> and pass that to failure handlers as well. > >> > >> Signed-off-by: Eric Sandeen <sandeen@redhat.com> > >> --- > >> fs/xfs/libxfs/xfs_alloc.c | 4 ++-- > >> fs/xfs/libxfs/xfs_alloc_btree.c | 2 +- > >> fs/xfs/libxfs/xfs_attr_leaf.c | 2 +- > >> fs/xfs/libxfs/xfs_bmap_btree.c | 2 +- > >> fs/xfs/libxfs/xfs_cksum.h | 5 ++++- > >> fs/xfs/libxfs/xfs_da_btree.c | 3 +-- > >> fs/xfs/libxfs/xfs_dir2_block.c | 2 +- > >> fs/xfs/libxfs/xfs_dir2_data.c | 2 +- > >> fs/xfs/libxfs/xfs_dir2_leaf.c | 2 +- > >> fs/xfs/libxfs/xfs_dir2_node.c | 2 +- > >> fs/xfs/libxfs/xfs_ialloc.c | 2 +- > >> fs/xfs/libxfs/xfs_ialloc_btree.c | 2 +- > >> fs/xfs/libxfs/xfs_refcount_btree.c | 2 +- > >> fs/xfs/libxfs/xfs_rmap_btree.c | 2 +- > >> fs/xfs/libxfs/xfs_symlink_remote.c | 2 +- > >> fs/xfs/libxfs/xfs_types.h | 12 ++++++++++-- > >> fs/xfs/xfs_linux.h | 7 ------- > >> 17 files changed, 29 insertions(+), 26 deletions(-) > >> > > ... > >> diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h > >> index 29b0d354d9b7..ab045e8dfcb9 100644 > >> --- a/fs/xfs/libxfs/xfs_types.h > >> +++ b/fs/xfs/libxfs/xfs_types.h > >> @@ -45,8 +45,16 @@ struct xfs_vc { > >> xfs_failaddr_t fa; > >> }; > >> > >> -#define XFS_CORRUPTED_RETURN(vc) ({(vc)->fa = __this_address; false;}) > >> -#define XFS_VERIFIED_RETURN(vc) ({(vc)->fa = NULL; true;}) > >> +/* > >> + * Return the address of a label. Use barrier() so that the optimizer > >> + * won't reorder code to refactor the error jumpouts into a single > >> + * return, which throws off the reported address. > >> + */ > >> +#define __this_address ({ __label__ __here; __here: barrier(); &&__here; }) > >> + > > > > FYI, minor whitespace damage on the line above. > > > >> +#define XFS_CORRUPTED_RETURN(vc) ({(vc)->fa = __this_address; false;}) > >> +#define XFS_BADCRC_RETURN(vc) ({(vc)->fa = __this_address; false;}) > >> +#define XFS_VERIFIED_RETURN(vc) ({(vc)->fa = NULL; true;}) > >> > > > > A couple high level comments.. > > > > I don't particularly care that much whether we bury function returns in > > the macro or open-code it, but the macro naming suggests the former > > (based on precedent of other such macros in XFS) while we implement the > > latter. If there's objection to a return within a macro, perhaps a > > reasonable compromise between this and the common pattern of having to > > return on a separate line is to tweak the macros to never clobber an > > existing error and update the verifiers to check/return failure state at > > opportune points. For example: > > > > ... > > if (!uuid_equal(&agfl->agfl_uuid, &mp->m_sb.sb_meta_uuid)) > > XFS_VC_CORRUPT(vc); > > if (be32_to_cpu(agfl->agfl_magicnum) != XFS_AGFL_MAGIC) > > XFS_VC_CORRUPT(vc); > > if (bp->b_pag && be32_to_cpu(agfl->agfl_seqno) != bp->b_pag->pag_agno) > > XFS_VC_CORRUPT(vc); > > ... > > > > return vc->fa ? false : true; > > > > Of course, that assumes it's safe to keep checking the structure(s) as > > such in the event of corruption, which perhaps is not ideal. Anyways, we > > could also just return on a separate line or rename the macros. Just > > thinking out loud a bit. I would be fine with changing the name but leaving the return, e.g.: #define XFS_VC_CORRUPT(vc) ({vc->fa = __this_address; false;}) if (!uuid_equal(&agfl->agfl_uuid, &mp->m_sb.sb_meta_uuid)) return XFS_VC_CORRUPT(vc); Since I don't see much point in continuing once we've decided the metadata is garbage. Scrub used to try to continue to find all the errors in a metadata, but Dave had good reasons for shooting down that strategy (unnecessary work, slows down the checker, potential for straying into places we shouldn't). > > I'm also a little curious why we have the need for the success macro at > > all. I've only made a cursory pass at this point, but is there a > > particular need to set anything in the xfs_vc at the point of a > > successful return as opposed to just leaving the structure in the > > initialized state? > > yeah, it's probably not necessary; it seemed consistent tho. There is some > risk to this whole framework that failing to initialize a vc would cause > problems that might be difficult to debug, and always marking success > upon success seemed "safe." But you're right, not not necessary if it's > properly initialized to success at the top of the call chain. Yeah, how about a good static initializer so we don't have to open code struct xfs_vc vc = { NULL }; everywhere? --D > -Eric
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c index b9e3c69490eb..14611b12220a 100644 --- a/fs/xfs/libxfs/xfs_alloc.c +++ b/fs/xfs/libxfs/xfs_alloc.c @@ -610,7 +610,7 @@ xfs_agfl_read_verify( return; if (!xfs_buf_verify_cksum(vc, bp, XFS_AGFL_CRC_OFF)) - xfs_verifier_error(bp, -EFSBADCRC, __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); else { if (!xfs_agfl_verify(vc, bp)) xfs_verifier_error(bp, -EFSCORRUPTED, vc->fa); @@ -2637,7 +2637,7 @@ xfs_agf_read_verify( if (xfs_sb_version_hascrc(&mp->m_sb) && !xfs_buf_verify_cksum(vc, bp, XFS_AGF_CRC_OFF)) - xfs_verifier_error(bp, -EFSBADCRC, __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); else { if (XFS_TEST_ERROR(!xfs_agf_verify(vc, bp), mp, XFS_ERRTAG_ALLOC_READ_AGF)) diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c index 87d96e66d4ca..40040505794a 100644 --- a/fs/xfs/libxfs/xfs_alloc_btree.c +++ b/fs/xfs/libxfs/xfs_alloc_btree.c @@ -347,7 +347,7 @@ xfs_allocbt_read_verify( struct xfs_buf *bp) { if (!xfs_btree_sblock_verify_crc(vc, bp)) - xfs_verifier_error(bp, -EFSBADCRC, __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); else { if (!xfs_allocbt_verify(vc, bp)) xfs_verifier_error(bp, -EFSCORRUPTED, vc->fa); diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c index 724b13f61da6..a69ff26a4558 100644 --- a/fs/xfs/libxfs/xfs_attr_leaf.c +++ b/fs/xfs/libxfs/xfs_attr_leaf.c @@ -359,7 +359,7 @@ xfs_attr3_leaf_read_verify( if (xfs_sb_version_hascrc(&mp->m_sb) && !xfs_buf_verify_cksum(vc, bp, XFS_ATTR3_LEAF_CRC_OFF)) - xfs_verifier_error(bp, -EFSBADCRC, __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); else { if (!xfs_attr3_leaf_verify(vc, bp)) xfs_verifier_error(bp, -EFSCORRUPTED, vc->fa); diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c index fc4ea15ca6c1..485b207f715f 100644 --- a/fs/xfs/libxfs/xfs_bmap_btree.c +++ b/fs/xfs/libxfs/xfs_bmap_btree.c @@ -451,7 +451,7 @@ xfs_bmbt_read_verify( struct xfs_buf *bp) { if (!xfs_btree_lblock_verify_crc(vc, bp)) - xfs_verifier_error(bp, -EFSBADCRC, __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); else { if (!xfs_bmbt_verify(vc, bp)) xfs_verifier_error(bp, -EFSCORRUPTED, vc->fa); diff --git a/fs/xfs/libxfs/xfs_cksum.h b/fs/xfs/libxfs/xfs_cksum.h index 888ef9ec4c57..ea4a2474e64b 100644 --- a/fs/xfs/libxfs/xfs_cksum.h +++ b/fs/xfs/libxfs/xfs_cksum.h @@ -77,7 +77,10 @@ xfs_verify_cksum(struct xfs_vc *vc, char *buffer, size_t length, { uint32_t crc = xfs_start_cksum_safe(buffer, length, cksum_offset); - return *(__le32 *)(buffer + cksum_offset) == xfs_end_cksum(crc); + if (*(__le32 *)(buffer + cksum_offset) != xfs_end_cksum(crc)) + return XFS_BADCRC_RETURN(vc); + + return XFS_VERIFIED_RETURN(vc); } #endif /* _XFS_CKSUM_H */ diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c index 0b4f084766e3..b26ae562c8c7 100644 --- a/fs/xfs/libxfs/xfs_da_btree.c +++ b/fs/xfs/libxfs/xfs_da_btree.c @@ -205,8 +205,7 @@ xfs_da3_node_read_verify( switch (be16_to_cpu(info->magic)) { case XFS_DA3_NODE_MAGIC: if (!xfs_buf_verify_cksum(vc, bp, XFS_DA3_NODE_CRC_OFF)) { - xfs_verifier_error(bp, -EFSBADCRC, - __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); break; } /* fall through */ diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c index 534d5fcf13b3..4f0cd0dbc564 100644 --- a/fs/xfs/libxfs/xfs_dir2_block.c +++ b/fs/xfs/libxfs/xfs_dir2_block.c @@ -79,7 +79,7 @@ xfs_dir3_block_read_verify( if (xfs_sb_version_hascrc(&mp->m_sb) && !xfs_buf_verify_cksum(vc, bp, XFS_DIR3_DATA_CRC_OFF)) - xfs_verifier_error(bp, -EFSBADCRC, __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); else { if (!xfs_dir3_block_verify(vc, bp)) xfs_verifier_error(bp, -EFSCORRUPTED, vc->fa); diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c index a215c7adb480..ad78bfd5eea6 100644 --- a/fs/xfs/libxfs/xfs_dir2_data.c +++ b/fs/xfs/libxfs/xfs_dir2_data.c @@ -305,7 +305,7 @@ xfs_dir3_data_read_verify( if (xfs_sb_version_hascrc(&mp->m_sb) && !xfs_buf_verify_cksum(vc, bp, XFS_DIR3_DATA_CRC_OFF)) - xfs_verifier_error(bp, -EFSBADCRC, __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); else { if (!xfs_dir3_data_verify(vc, bp)) xfs_verifier_error(bp, -EFSCORRUPTED, vc->fa); diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c index e757e1f950e4..ad1af1eeda53 100644 --- a/fs/xfs/libxfs/xfs_dir2_leaf.c +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c @@ -185,7 +185,7 @@ __read_verify( if (xfs_sb_version_hascrc(&mp->m_sb) && !xfs_buf_verify_cksum(vc, bp, XFS_DIR3_LEAF_CRC_OFF)) - xfs_verifier_error(bp, -EFSBADCRC, __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); else { if (!xfs_dir3_leaf_verify(vc, bp, magic)) xfs_verifier_error(bp, -EFSCORRUPTED, vc->fa); diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c index 8105544c44fb..784534734485 100644 --- a/fs/xfs/libxfs/xfs_dir2_node.c +++ b/fs/xfs/libxfs/xfs_dir2_node.c @@ -118,7 +118,7 @@ xfs_dir3_free_read_verify( if (xfs_sb_version_hascrc(&mp->m_sb) && !xfs_buf_verify_cksum(vc, bp, XFS_DIR3_FREE_CRC_OFF)) - xfs_verifier_error(bp, -EFSBADCRC, __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); else { if (!xfs_dir3_free_verify(vc, bp)) xfs_verifier_error(bp, -EFSCORRUPTED, vc->fa); diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c index 32fb58c929f0..71745ec0d92f 100644 --- a/fs/xfs/libxfs/xfs_ialloc.c +++ b/fs/xfs/libxfs/xfs_ialloc.c @@ -2557,7 +2557,7 @@ xfs_agi_read_verify( if (xfs_sb_version_hascrc(&mp->m_sb) && !xfs_buf_verify_cksum(vc, bp, XFS_AGI_CRC_OFF)) - xfs_verifier_error(bp, -EFSBADCRC, __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); else { if (XFS_TEST_ERROR(!xfs_agi_verify(vc, bp), mp, XFS_ERRTAG_IALLOC_READ_AGI)) diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c index 30a1fcf24767..a0bb695944e9 100644 --- a/fs/xfs/libxfs/xfs_ialloc_btree.c +++ b/fs/xfs/libxfs/xfs_ialloc_btree.c @@ -300,7 +300,7 @@ xfs_inobt_read_verify( struct xfs_buf *bp) { if (!xfs_btree_sblock_verify_crc(vc, bp)) - xfs_verifier_error(bp, -EFSBADCRC, __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); else { if (!xfs_inobt_verify(vc, bp)) xfs_verifier_error(bp, -EFSCORRUPTED, vc->fa); diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c index 4b4c80fd3d6c..748074c5ebeb 100644 --- a/fs/xfs/libxfs/xfs_refcount_btree.c +++ b/fs/xfs/libxfs/xfs_refcount_btree.c @@ -235,7 +235,7 @@ xfs_refcountbt_read_verify( struct xfs_buf *bp) { if (!xfs_btree_sblock_verify_crc(vc, bp)) - xfs_verifier_error(bp, -EFSBADCRC, __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); else { if (!xfs_refcountbt_verify(vc, bp)) xfs_verifier_error(bp, -EFSCORRUPTED, vc->fa); diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c index ed0022fb03f6..6d2eba7b44bc 100644 --- a/fs/xfs/libxfs/xfs_rmap_btree.c +++ b/fs/xfs/libxfs/xfs_rmap_btree.c @@ -334,7 +334,7 @@ xfs_rmapbt_read_verify( struct xfs_buf *bp) { if (!xfs_btree_sblock_verify_crc(vc, bp)) - xfs_verifier_error(bp, -EFSBADCRC, __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); else { if (!xfs_rmapbt_verify(vc, bp)) xfs_verifier_error(bp, -EFSCORRUPTED, vc->fa); diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c index 401398d0235a..d6516068bbe7 100644 --- a/fs/xfs/libxfs/xfs_symlink_remote.c +++ b/fs/xfs/libxfs/xfs_symlink_remote.c @@ -125,7 +125,7 @@ xfs_symlink_read_verify( return; if (!xfs_buf_verify_cksum(vc, bp, XFS_SYMLINK_CRC_OFF)) - xfs_verifier_error(bp, -EFSBADCRC, __this_address); + xfs_verifier_error(bp, -EFSBADCRC, vc->fa); else { if (!xfs_symlink_verify(vc, bp)) xfs_verifier_error(bp, -EFSCORRUPTED, vc->fa); diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h index 29b0d354d9b7..ab045e8dfcb9 100644 --- a/fs/xfs/libxfs/xfs_types.h +++ b/fs/xfs/libxfs/xfs_types.h @@ -45,8 +45,16 @@ struct xfs_vc { xfs_failaddr_t fa; }; -#define XFS_CORRUPTED_RETURN(vc) ({(vc)->fa = __this_address; false;}) -#define XFS_VERIFIED_RETURN(vc) ({(vc)->fa = NULL; true;}) +/* + * Return the address of a label. Use barrier() so that the optimizer + * won't reorder code to refactor the error jumpouts into a single + * return, which throws off the reported address. + */ +#define __this_address ({ __label__ __here; __here: barrier(); &&__here; }) + +#define XFS_CORRUPTED_RETURN(vc) ({(vc)->fa = __this_address; false;}) +#define XFS_BADCRC_RETURN(vc) ({(vc)->fa = __this_address; false;}) +#define XFS_VERIFIED_RETURN(vc) ({(vc)->fa = NULL; true;}) /* * Null values for the types. diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h index edbd5a210df2..4141e70bb3fa 100644 --- a/fs/xfs/xfs_linux.h +++ b/fs/xfs/xfs_linux.h @@ -131,13 +131,6 @@ typedef __u32 xfs_nlink_t; #define SYNCHRONIZE() barrier() #define __return_address __builtin_return_address(0) -/* - * Return the address of a label. Use barrier() so that the optimizer - * won't reorder code to refactor the error jumpouts into a single - * return, which throws off the reported address. - */ -#define __this_address ({ __label__ __here; __here: barrier(); &&__here; }) - #define XFS_PROJID_DEFAULT 0 #define howmany(x, y) (((x)+((y)-1))/(y))
Modify CRC checking functions to set __this_address into the verifier context failaddr vc->fa using new macro XFS_BADCRC_RETURN, and pass that to failure handlers as well. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- fs/xfs/libxfs/xfs_alloc.c | 4 ++-- fs/xfs/libxfs/xfs_alloc_btree.c | 2 +- fs/xfs/libxfs/xfs_attr_leaf.c | 2 +- fs/xfs/libxfs/xfs_bmap_btree.c | 2 +- fs/xfs/libxfs/xfs_cksum.h | 5 ++++- fs/xfs/libxfs/xfs_da_btree.c | 3 +-- fs/xfs/libxfs/xfs_dir2_block.c | 2 +- fs/xfs/libxfs/xfs_dir2_data.c | 2 +- fs/xfs/libxfs/xfs_dir2_leaf.c | 2 +- fs/xfs/libxfs/xfs_dir2_node.c | 2 +- fs/xfs/libxfs/xfs_ialloc.c | 2 +- fs/xfs/libxfs/xfs_ialloc_btree.c | 2 +- fs/xfs/libxfs/xfs_refcount_btree.c | 2 +- fs/xfs/libxfs/xfs_rmap_btree.c | 2 +- fs/xfs/libxfs/xfs_symlink_remote.c | 2 +- fs/xfs/libxfs/xfs_types.h | 12 ++++++++++-- fs/xfs/xfs_linux.h | 7 ------- 17 files changed, 29 insertions(+), 26 deletions(-)