On Wed, Dec 05, 2018 at 03:02:15PM -0600, Eric Sandeen wrote:
> xfs_attr3_rmt_hdr_ok is only checked for true/false and the caller
> does nothing with the returned failaddr, so make it a bool.
> (its usage and naming lend itself to a bool too, i.e. test (!ok))
>
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_attr_remote.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
> index d89363c6b523..f86680e08613 100644
> --- a/fs/xfs/libxfs/xfs_attr_remote.c
> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
> @@ -53,7 +53,7 @@ xfs_attr3_rmt_blocks(
> * does CRC, location and bounds checking, the unpacking function checks the
> * attribute parameters and owner.
> */
> -static xfs_failaddr_t
> +static bool
> xfs_attr3_rmt_hdr_ok(
> void *ptr,
> xfs_ino_t ino,
> @@ -64,16 +64,16 @@ xfs_attr3_rmt_hdr_ok(
> struct xfs_attr3_rmt_hdr *rmt = ptr;
>
> if (bno != be64_to_cpu(rmt->rm_blkno))
> - return __this_address;
> + return false;
> if (offset != be32_to_cpu(rmt->rm_offset))
> - return __this_address;
> + return false;
> if (size != be32_to_cpu(rmt->rm_bytes))
> - return __this_address;
> + return false;
> if (ino != be64_to_cpu(rmt->rm_owner))
> - return __this_address;
> + return false;
>
> /* ok */
> - return NULL;
> + return true;
> }
>
> static xfs_failaddr_t
> @@ -287,7 +287,7 @@ xfs_attr_rmtval_copyout(
> byte_cnt = min(*valuelen, byte_cnt);
>
> if (xfs_sb_version_hascrc(&mp->m_sb)) {
> - if (xfs_attr3_rmt_hdr_ok(src, ino, *offset,
> + if (!xfs_attr3_rmt_hdr_ok(src, ino, *offset,
Why not pass a verifier context to attr3_rmt_hdr_ok and enable
rmtval_copyout to report the approximate instruction address of the
failed check?
(Or maybe I missed you doing this later...?)
--D
> byte_cnt, bno)) {
> xfs_alert(mp,
> "remote attribute header mismatch bno/off/len/owner (0x%llx/0x%x/Ox%x/0x%llx)",
> --
> 2.17.0
>
>
