From: Ben Myers <bpm@sgi.com>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 10/21] xfs: add CRC checks to remote symlinks
Date: Wed, 20 Mar 2013 17:03:34 -0500 [thread overview]
Message-ID: <20130320220334.GN22182@sgi.com> (raw)
In-Reply-To: <1363091454-8852-11-git-send-email-david@fromorbit.com>
On Tue, Mar 12, 2013 at 11:30:43PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Add a header to the remote symlink block, containing location and
> owner information, as well as CRCs and LSN fields. This requires
> verifiers to be added to the remote symlink buffers for CRC enabled
> filesystems.
>
> This also fixes a bug reading multiple block symlinks, where the second block
> overwrites the first block when copying out the link name.
More comments below.
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/Makefile | 5 +-
> fs/xfs/xfs_bmap.c | 21 +-
> fs/xfs/xfs_buf_item.h | 4 +-
> fs/xfs/xfs_log_recover.c | 9 +
> fs/xfs/xfs_symlink.c | 719 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/xfs/xfs_symlink.h | 53 ++++
> fs/xfs/xfs_vnodeops.c | 480 +------------------------------
> 7 files changed, 797 insertions(+), 494 deletions(-)
> create mode 100644 fs/xfs/xfs_symlink.c
> create mode 100644 fs/xfs/xfs_symlink.h
>
> diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
> index d02201d..062f25c 100644
> --- a/fs/xfs/Makefile
> +++ b/fs/xfs/Makefile
> @@ -45,11 +45,11 @@ xfs-y += xfs_aops.o \
> xfs_itable.o \
> xfs_message.o \
> xfs_mru_cache.o \
> - xfs_super.o \
> - xfs_xattr.o \
> xfs_rename.o \
> + xfs_super.o \
> xfs_utils.o \
> xfs_vnodeops.o \
> + xfs_xattr.o \
> kmem.o \
> uuid.o
>
> @@ -73,6 +73,7 @@ xfs-y += xfs_alloc.o \
> xfs_inode.o \
> xfs_log_recover.o \
> xfs_mount.o \
> + xfs_symlink.o \
> xfs_trans.o
>
> # low-level transaction/log code
> diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
> index 0531cd3..0fcb7f4 100644
> --- a/fs/xfs/xfs_bmap.c
> +++ b/fs/xfs/xfs_bmap.c
> @@ -47,6 +47,7 @@
> #include "xfs_filestream.h"
> #include "xfs_vnodeops.h"
> #include "xfs_trace.h"
> +#include "xfs_symlink.h"
>
>
> kmem_zone_t *xfs_bmap_free_item_zone;
> @@ -1321,9 +1322,10 @@ xfs_bmap_add_attrfork_extents(
> }
>
> /*
> - * Block initialisation functions for local to extent format conversion.
> - * As these get more complex, they will be moved to the relevant files,
> - * but for now they are too simple to worry about.
> + * Block initialisation function for local to extent format conversion.
> + *
> + * This shouldn't actually be called by anyone, so make sure debug kernels cause
> + * a noticable failure.
That's greater than 80 columns.
> */
> STATIC void
> xfs_bmap_local_to_extents_init_fn(
> @@ -1332,23 +1334,12 @@ xfs_bmap_local_to_extents_init_fn(
> struct xfs_inode *ip,
> struct xfs_ifork *ifp)
> {
> + ASSERT(0);
That has nothing to do with crcs.
> bp->b_ops = &xfs_bmbt_buf_ops;
> memcpy(bp->b_addr, ifp->if_u1.if_data, ifp->if_bytes);
> xfs_trans_buf_set_type(tp, bp, XFS_BLF_BTREE_BUF);
> }
>
> -STATIC void
> -xfs_symlink_local_to_remote(
> - struct xfs_trans *tp,
> - struct xfs_buf *bp,
> - struct xfs_inode *ip,
> - struct xfs_ifork *ifp)
> -{
> - /* remote symlink blocks are not verifiable until CRCs come along */
> - bp->b_ops = NULL;
> - memcpy(bp->b_addr, ifp->if_u1.if_data, ifp->if_bytes);
> -}
> -
> /*
> * Called from xfs_bmap_add_attrfork to handle local format files. Each
> * different data fork content type needs a different callout to do the
> diff --git a/fs/xfs/xfs_buf_item.h b/fs/xfs/xfs_buf_item.h
> index abae8c8..09cab4e 100644
> --- a/fs/xfs/xfs_buf_item.h
> +++ b/fs/xfs/xfs_buf_item.h
> @@ -49,6 +49,7 @@ extern kmem_zone_t *xfs_buf_item_zone;
> #define XFS_BLF_AGFL_BUF (1<<7)
> #define XFS_BLF_AGI_BUF (1<<8)
> #define XFS_BLF_DINO_BUF (1<<9)
> +#define XFS_BLF_SYMLINK_BUF (1<<10)
>
> #define XFS_BLF_TYPE_MASK \
> (XFS_BLF_UDQUOT_BUF | \
> @@ -58,7 +59,8 @@ extern kmem_zone_t *xfs_buf_item_zone;
> XFS_BLF_AGF_BUF | \
> XFS_BLF_AGFL_BUF | \
> XFS_BLF_AGI_BUF | \
> - XFS_BLF_DINO_BUF)
> + XFS_BLF_DINO_BUF | \
> + XFS_BLF_SYMLINK_BUF)
>
> #define XFS_BLF_CHUNK 128
> #define XFS_BLF_SHIFT 7
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 6d08eaa..352d794 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -45,6 +45,7 @@
> #include "xfs_cksum.h"
> #include "xfs_trace.h"
> #include "xfs_icache.h"
> +#include "xfs_symlink.h"
>
> STATIC int
> xlog_find_zeroed(
> @@ -2005,6 +2006,14 @@ xlog_recover_do_reg_buffer(
> }
> bp->b_ops = &xfs_inode_buf_ops;
> break;
> + case XFS_BLF_SYMLINK_BUF:
> + if (*(__be32 *)bp->b_addr != cpu_to_be32(XFS_SYMLINK_MAGIC)) {
> + xfs_warn(mp, "Bad symlink block magic!");
> + ASSERT(0);
> + break;
> + }
> + bp->b_ops = &xfs_symlink_buf_ops;
> + break;
> default:
> break;
> }
> diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
> new file mode 100644
> index 0000000..33e8f13
> --- /dev/null
> +++ b/fs/xfs/xfs_symlink.c
> @@ -0,0 +1,719 @@
> +/*
> + * Copyright (c) 2000-2006 Silicon Graphics, Inc.
> + * Copyright (c) 2012-2013 Red Hat, Inc.
> + * All rights reserved.
> + */
> +
> +#include "xfs.h"
> +#include "xfs_fs.h"
> +#include "xfs_types.h"
> +#include "xfs_bit.h"
> +#include "xfs_log.h"
> +#include "xfs_trans.h"
> +#include "xfs_sb.h"
> +#include "xfs_ag.h"
> +#include "xfs_dir2.h"
> +#include "xfs_mount.h"
> +#include "xfs_da_btree.h"
> +#include "xfs_bmap_btree.h"
> +#include "xfs_ialloc_btree.h"
> +#include "xfs_dinode.h"
> +#include "xfs_inode.h"
> +#include "xfs_inode_item.h"
> +#include "xfs_itable.h"
> +#include "xfs_ialloc.h"
> +#include "xfs_alloc.h"
> +#include "xfs_bmap.h"
> +#include "xfs_error.h"
> +#include "xfs_quota.h"
> +#include "xfs_utils.h"
> +#include "xfs_trans_space.h"
> +#include "xfs_log_priv.h"
> +#include "xfs_trace.h"
> +#include "xfs_symlink.h"
> +#include "xfs_cksum.h"
> +#include "xfs_buf_item.h"
> +
> +
> +/*
> + * Each contiguous block has a header, so it is not just a simple pathlen
> + * to FSB conversion.
> + */
> +int
> +xfs_symlink_blocks(
> + struct xfs_mount *mp,
> + int pathlen)
> +{
> + int fsblocks = 0;
> + int len = pathlen;
> +
> + do {
> + fsblocks++;
> + len -= XFS_SYMLINK_BUF_SPACE(mp, mp->m_sb.sb_blocksize);
> + } while (len > 0);
> +
> + ASSERT(fsblocks <= XFS_SYMLINK_MAPS);
> + return fsblocks;
> +}
> +
> +static int
> +xfs_symlink_hdr_set(
> + struct xfs_mount *mp,
> + xfs_ino_t ino,
> + uint32_t offset,
> + uint32_t size,
> + struct xfs_buf *bp)
> +{
> + struct xfs_dsymlink_hdr *dsl = bp->b_addr;
> +
> + if (!xfs_sb_version_hascrc(&mp->m_sb))
> + return 0;
> +
> + dsl->sl_magic = cpu_to_be32(XFS_SYMLINK_MAGIC);
> + dsl->sl_offset = cpu_to_be32(offset);
> + dsl->sl_bytes = cpu_to_be32(size);
> + uuid_copy(&dsl->sl_uuid, &mp->m_sb.sb_uuid);
> + dsl->sl_owner = cpu_to_be64(ino);
> + dsl->sl_blkno = cpu_to_be64(bp->b_bn);
> + bp->b_ops = &xfs_symlink_buf_ops;
> +
> + return sizeof(struct xfs_dsymlink_hdr);
> +}
> +
> +/*
> + * Checking of the symlink header is split into two parts. the verifier does
> + * CRC, location and bounds checking, the unpacking function checks the path
> + * parameters and owner.
> + */
> +bool
> +xfs_symlink_hdr_ok(
> + struct xfs_mount *mp,
> + xfs_ino_t ino,
> + uint32_t offset,
> + uint32_t size,
> + struct xfs_buf *bp)
> +{
> + struct xfs_dsymlink_hdr *dsl = bp->b_addr;
> +
> + if (offset != be32_to_cpu(dsl->sl_offset))
> + return false;
> + if (size != be32_to_cpu(dsl->sl_bytes))
> + return false;
> + if (ino != be64_to_cpu(dsl->sl_owner))
> + return false;
> +
> + /* ok */
> + return true;
> +
Looks like an extra line here.
-Ben
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2013-03-20 22:03 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-12 12:30 [PATCH 00/21] xfs: metadata CRCs, third version Dave Chinner
2013-03-12 12:30 ` [PATCH 01/21] xfs: ensure we capture IO errors correctly Dave Chinner
2013-03-12 12:30 ` [PATCH 02/21] xfs: increase hexdump output in xfs_corruption_error Dave Chinner
2013-03-14 21:18 ` Ben Myers
2013-03-15 1:13 ` Dave Chinner
2013-03-12 12:30 ` [PATCH 03/21] xfs: take inode version into account in XFS_LITINO Dave Chinner
2013-03-12 12:30 ` [PATCH 04/21] xfs: add support for large btree blocks Dave Chinner
2013-03-12 12:30 ` [PATCH 05/21] xfs: add CRC checks to the AGF Dave Chinner
2013-03-12 12:30 ` [PATCH 06/21] xfs: add CRC checks to the AGFL Dave Chinner
2013-03-12 12:30 ` [PATCH 07/21] xfs: add CRC checks to the AGI Dave Chinner
2013-03-12 12:30 ` [PATCH 08/21] xfs: add CRC checks for quota blocks Dave Chinner
2013-03-12 12:30 ` [PATCH 09/21] xfs: add version 3 inode format with CRCs Dave Chinner
2013-03-14 16:03 ` Ben Myers
2013-03-14 19:01 ` Ben Myers
2013-03-15 1:11 ` Dave Chinner
2013-03-26 22:56 ` Dave Chinner
2013-03-27 0:53 ` Ben Myers
2013-03-27 1:48 ` Dave Chinner
2013-04-02 22:44 ` Ben Myers
2013-04-03 4:08 ` Dave Chinner
2013-04-02 22:49 ` Ben Myers
2013-03-12 12:30 ` [PATCH 10/21] xfs: add CRC checks to remote symlinks Dave Chinner
2013-03-20 21:14 ` Ben Myers
2013-03-21 1:22 ` Dave Chinner
2013-03-21 14:59 ` Ben Myers
2013-03-20 22:03 ` Ben Myers [this message]
2013-03-21 1:32 ` Dave Chinner
2013-03-12 12:30 ` [PATCH 11/21] xfs: add CRC checks to block format directory blocks Dave Chinner
2013-03-26 18:39 ` Ben Myers
2013-03-26 21:40 ` Dave Chinner
2013-03-12 12:30 ` [PATCH 12/21] xfs: add CRC checking to dir2 free blocks Dave Chinner
2013-03-28 23:40 ` Ben Myers
2013-03-29 3:13 ` Dave Chinner
2013-03-12 12:30 ` [PATCH 13/21] xfs: add CRC checking to dir2 data blocks Dave Chinner
2013-04-03 22:13 ` Ben Myers
2013-03-12 12:30 ` [PATCH 14/21] xfs: add CRC checking to dir2 leaf blocks Dave Chinner
2013-03-12 12:30 ` [PATCH 15/21] xfs: shortform directory offsets change for dir3 format Dave Chinner
2013-03-12 12:30 ` [PATCH 16/21] xfs: add CRCs to dir2/da node blocks Dave Chinner
2013-03-12 12:30 ` [PATCH 17/21] xfs: add CRCs to attr leaf blocks Dave Chinner
2013-03-12 12:30 ` [PATCH 18/21] xfs: split remote attribute code out Dave Chinner
2013-03-12 12:30 ` [PATCH 19/21] xfs: add CRC protection to remote attributes Dave Chinner
2013-03-12 12:30 ` [PATCH 20/21] xfs: add buffer types to directory and attribute buffers Dave Chinner
2013-03-12 12:30 ` [PATCH 21/21] xfs: add CRC checks to the superblock Dave Chinner
2013-03-26 20:58 ` Chandra Seetharaman
2013-03-27 1:06 ` Dave Chinner
2013-03-27 23:07 ` Chandra Seetharaman
2013-03-28 1:36 ` Dave Chinner
2013-03-12 12:43 ` [PATCH 22/21] xfs: Fix magic number assert in xfs_dir3_leaf_log_bests Dave Chinner
2013-03-13 0:29 ` [PATCH 23/21] xfs: fix endian issues reported by sparse Dave Chinner
2013-03-13 1:34 ` [PATCH 24/21] xfs: buffer type overruns blf_flags field Dave Chinner
2013-03-14 21:41 ` [PATCH 00/21] xfs: metadata CRCs, third version Ben Myers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130320220334.GN22182@sgi.com \
--to=bpm@sgi.com \
--cc=david@fromorbit.com \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox