[Ocfs2-devel] [PATCH V2] ocfs2: xattr: fix inlined xattr reflink
Andrew Morton
akpm at linux-foundation.org
Thu Jun 27 16:08:28 PDT 2013
On Fri, 21 Jun 2013 09:27:24 +0800 Junxiao Bi <junxiao.bi at oracle.com> wrote:
> Inlined xattr shared free space of inode block with inlined data
> or data extent record, so the size of the later two should be
> adjusted when inlined xattr is enabled. See ocfs2_xattr_ibody_init().
> But this isn't done well when reflink. For inode with inlined data,
> its max inlined data size is adjusted in ocfs2_duplicate_inline_data(),
> no problem. But for inode with data extent record, its record count isn't
> adjusted. Fix it, or data extent record and inlined xattr may overwrite
> each other, then cause data corruption or xattr failure.
>
> One panic caused by this bug in our test environment is the following:
>
> kernel BUG at fs/ocfs2/xattr.c:1435!
>
> ...
>
> @@ -6499,6 +6499,16 @@ static int ocfs2_reflink_xattr_inline(struct ocfs2_xattr_reflink *args)
> }
>
> new_oi = OCFS2_I(args->new_inode);
> + /*
> + * Adjust extent record count to reserve space for extended attribute.
> + * Inline data count had been adjusted in ocfs2_duplicate_inline_data().
> + */
> + if (!(new_oi->ip_dyn_features & OCFS2_INLINE_DATA_FL) &&
> + !(ocfs2_inode_is_fast_symlink(new_inode))) {
> + struct ocfs2_extent_list *el = &new_di->id2.i_list;
> + le16_add_cpu(&el->l_count, -(inline_size /
> + sizeof(struct ocfs2_extent_rec)));
> + }
This is badly screwed up. There is no local variable `new_inode' -
new_inode() is a global function!
I assume you meant args->new_inode, but this patch clearly wasn't
tested (or wasn't what you _did_ test).
I'll drop it. Please fix, retest, resend.
More information about the Ocfs2-devel
mailing list