From: Greg Kroah-Hartman Date: Fri, 1 Mar 2013 23:50:47 +0000 (-0800) Subject: 3.8-stable patches X-Git-Tag: v3.8.2~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3fbfe9ff445694409e39cebb8ccf0c5c8519a710;p=thirdparty%2Fkernel%2Fstable-queue.git 3.8-stable patches added patches: xfs-xfs_bmap_add_attrfork_local-is-too-generic.patch --- diff --git a/queue-3.8/series b/queue-3.8/series index 0a5f7f0dca9..35ab49d33b9 100644 --- a/queue-3.8/series +++ b/queue-3.8/series @@ -75,3 +75,4 @@ x86-efi-allow-slash-in-file-path-of-initrd.patch acpi-overriding-acpi-tables-via-initrd-only-works-with-an-initrd-and-on-x86.patch efivarfs-validate-filenames-much-more-aggressively.patch efivarfs-guid-part-of-filenames-are-case-insensitive.patch +xfs-xfs_bmap_add_attrfork_local-is-too-generic.patch diff --git a/queue-3.8/xfs-xfs_bmap_add_attrfork_local-is-too-generic.patch b/queue-3.8/xfs-xfs_bmap_add_attrfork_local-is-too-generic.patch new file mode 100644 index 00000000000..3932efed0a2 --- /dev/null +++ b/queue-3.8/xfs-xfs_bmap_add_attrfork_local-is-too-generic.patch @@ -0,0 +1,241 @@ +From 1e82379b018ceed0f0912327c60d73107dacbcb3 Mon Sep 17 00:00:00 2001 +From: Dave Chinner +Date: Mon, 11 Feb 2013 15:58:13 +1100 +Subject: xfs: xfs_bmap_add_attrfork_local is too generic + +From: Dave Chinner + +commit 1e82379b018ceed0f0912327c60d73107dacbcb3 upstream. + +When we are converting local data to an extent format as a result of +adding an attribute, the type of data contained in the local fork +determines the behaviour that needs to occur. + +xfs_bmap_add_attrfork_local() already handles the directory data +case specially by using S_ISDIR() and calling out to +xfs_dir2_sf_to_block(), but with verifiers we now need to handle +each different type of metadata specially and different metadata +formats require different verifiers (and eventually block header +initialisation). + +There is only a single place that we add and attribute fork to +the inode, but that is in the attribute code and it knows nothing +about the specific contents of the data fork. It is only the case of +local data that is the issue here, so adding code to hadnle this +case in the attribute specific code is wrong. Hence we are really +stuck trying to detect the data fork contents in +xfs_bmap_add_attrfork_local() and performing the correct callout +there. + +Luckily the current cases can be determined by S_IS* macros, and we +can push the work off to data specific callouts, but each of those +callouts does a lot of work in common with +xfs_bmap_local_to_extents(). The only reason that this fails for +symlinks right now is is that xfs_bmap_local_to_extents() assumes +the data fork contains extent data, and so attaches a a bmap extent +data verifier to the buffer and simply copies the data fork +information straight into it. + +To fix this, allow us to pass a "formatting" callback into +xfs_bmap_local_to_extents() which is responsible for setting the +buffer type, initialising it and copying the data fork contents over +to the new buffer. This allows callers to specify how they want to +format the new buffer (which is necessary for the upcoming CRC +enabled metadata blocks) and hence make xfs_bmap_local_to_extents() +useful for any type of data fork content. + +Signed-off-by: Dave Chinner +Reviewed-by: Mark Tinguely +Signed-off-by: Ben Myers +Signed-off-by: Greg Kroah-Hartman + + +--- + fs/xfs/xfs_bmap.c | 114 ++++++++++++++++++++++++++++++++++++++++++++---------- + 1 file changed, 93 insertions(+), 21 deletions(-) + +--- a/fs/xfs/xfs_bmap.c ++++ b/fs/xfs/xfs_bmap.c +@@ -147,7 +147,10 @@ xfs_bmap_local_to_extents( + xfs_fsblock_t *firstblock, /* first block allocated in xaction */ + xfs_extlen_t total, /* total blocks needed by transaction */ + int *logflagsp, /* inode logging flags */ +- int whichfork); /* data or attr fork */ ++ int whichfork, /* data or attr fork */ ++ void (*init_fn)(struct xfs_buf *bp, ++ struct xfs_inode *ip, ++ struct xfs_ifork *ifp)); + + /* + * Search the extents list for the inode, for the extent containing bno. +@@ -357,7 +360,42 @@ xfs_bmap_add_attrfork_extents( + } + + /* +- * Called from xfs_bmap_add_attrfork to handle local format files. ++ * 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. ++ */ ++STATIC void ++xfs_bmap_local_to_extents_init_fn( ++ struct xfs_buf *bp, ++ struct xfs_inode *ip, ++ struct xfs_ifork *ifp) ++{ ++ bp->b_ops = &xfs_bmbt_buf_ops; ++ memcpy(bp->b_addr, ifp->if_u1.if_data, ifp->if_bytes); ++} ++ ++STATIC void ++xfs_symlink_local_to_remote( ++ 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 ++ * conversion. Some are basic and only require special block initialisation ++ * callouts for the data formating, others (directories) are so specialised they ++ * handle everything themselves. ++ * ++ * XXX (dgc): investigate whether directory conversion can use the generic ++ * formatting callout. It should be possible - it's just a very complex ++ * formatter. it would also require passing the transaction through to the init ++ * function. + */ + STATIC int /* error */ + xfs_bmap_add_attrfork_local( +@@ -368,25 +406,29 @@ xfs_bmap_add_attrfork_local( + int *flags) /* inode logging flags */ + { + xfs_da_args_t dargs; /* args for dir/attr code */ +- int error; /* error return value */ +- xfs_mount_t *mp; /* mount structure pointer */ + + if (ip->i_df.if_bytes <= XFS_IFORK_DSIZE(ip)) + return 0; ++ + if (S_ISDIR(ip->i_d.di_mode)) { +- mp = ip->i_mount; + memset(&dargs, 0, sizeof(dargs)); + dargs.dp = ip; + dargs.firstblock = firstblock; + dargs.flist = flist; +- dargs.total = mp->m_dirblkfsbs; ++ dargs.total = ip->i_mount->m_dirblkfsbs; + dargs.whichfork = XFS_DATA_FORK; + dargs.trans = tp; +- error = xfs_dir2_sf_to_block(&dargs); +- } else +- error = xfs_bmap_local_to_extents(tp, ip, firstblock, 1, flags, +- XFS_DATA_FORK); +- return error; ++ return xfs_dir2_sf_to_block(&dargs); ++ } ++ ++ if (S_ISLNK(ip->i_d.di_mode)) ++ return xfs_bmap_local_to_extents(tp, ip, firstblock, 1, ++ flags, XFS_DATA_FORK, ++ xfs_symlink_local_to_remote); ++ ++ return xfs_bmap_local_to_extents(tp, ip, firstblock, 1, flags, ++ XFS_DATA_FORK, ++ xfs_bmap_local_to_extents_init_fn); + } + + /* +@@ -3221,7 +3263,10 @@ xfs_bmap_local_to_extents( + xfs_fsblock_t *firstblock, /* first block allocated in xaction */ + xfs_extlen_t total, /* total blocks needed by transaction */ + int *logflagsp, /* inode logging flags */ +- int whichfork) /* data or attr fork */ ++ int whichfork, ++ void (*init_fn)(struct xfs_buf *bp, ++ struct xfs_inode *ip, ++ struct xfs_ifork *ifp)) + { + int error; /* error return value */ + int flags; /* logging flags returned */ +@@ -3241,12 +3286,12 @@ xfs_bmap_local_to_extents( + xfs_buf_t *bp; /* buffer for extent block */ + xfs_bmbt_rec_host_t *ep;/* extent record pointer */ + ++ ASSERT((ifp->if_flags & ++ (XFS_IFINLINE|XFS_IFEXTENTS|XFS_IFEXTIREC)) == XFS_IFINLINE); + memset(&args, 0, sizeof(args)); + args.tp = tp; + args.mp = ip->i_mount; + args.firstblock = *firstblock; +- ASSERT((ifp->if_flags & +- (XFS_IFINLINE|XFS_IFEXTENTS|XFS_IFEXTIREC)) == XFS_IFINLINE); + /* + * Allocate a block. We know we need only one, since the + * file currently fits in an inode. +@@ -3262,17 +3307,20 @@ xfs_bmap_local_to_extents( + args.mod = args.minleft = args.alignment = args.wasdel = + args.isfl = args.minalignslop = 0; + args.minlen = args.maxlen = args.prod = 1; +- if ((error = xfs_alloc_vextent(&args))) ++ error = xfs_alloc_vextent(&args); ++ if (error) + goto done; +- /* +- * Can't fail, the space was reserved. +- */ ++ ++ /* Can't fail, the space was reserved. */ + ASSERT(args.fsbno != NULLFSBLOCK); + ASSERT(args.len == 1); + *firstblock = args.fsbno; + bp = xfs_btree_get_bufl(args.mp, tp, args.fsbno, 0); +- bp->b_ops = &xfs_bmbt_buf_ops; +- memcpy(bp->b_addr, ifp->if_u1.if_data, ifp->if_bytes); ++ ++ /* initialise the block and copy the data */ ++ init_fn(bp, ip, ifp); ++ ++ /* account for the change in fork size and log everything */ + xfs_trans_log_buf(tp, bp, 0, ifp->if_bytes - 1); + xfs_bmap_forkoff_reset(args.mp, ip, whichfork); + xfs_idata_realloc(ip, -ifp->if_bytes, whichfork); +@@ -4919,8 +4967,32 @@ xfs_bmapi_write( + XFS_STATS_INC(xs_blk_mapw); + + if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) { ++ /* ++ * XXX (dgc): This assumes we are only called for inodes that ++ * contain content neutral data in local format. Anything that ++ * contains caller-specific data in local format that needs ++ * transformation to move to a block format needs to do the ++ * conversion to extent format itself. ++ * ++ * Directory data forks and attribute forks handle this ++ * themselves, but with the addition of metadata verifiers every ++ * data fork in local format now contains caller specific data ++ * and as such conversion through this function is likely to be ++ * broken. ++ * ++ * The only likely user of this branch is for remote symlinks, ++ * but we cannot overwrite the data fork contents of the symlink ++ * (EEXIST occurs higher up the stack) and so it will never go ++ * from local format to extent format here. Hence I don't think ++ * this branch is ever executed intentionally and we should ++ * consider removing it and asserting that xfs_bmapi_write() ++ * cannot be called directly on local format forks. i.e. callers ++ * are completely responsible for local to extent format ++ * conversion, not xfs_bmapi_write(). ++ */ + error = xfs_bmap_local_to_extents(tp, ip, firstblock, total, +- &bma.logflags, whichfork); ++ &bma.logflags, whichfork, ++ xfs_bmap_local_to_extents_init_fn); + if (error) + goto error0; + }