From: "Darrick J. Wong" To: tytso@mit.edu, darrick.wong@oracle.com Cc: linux-ext4@vger.kernel.org, Darren Hart , Richard Purdie Date: Sat, 13 Feb 2016 14:38:24 -0800 Message-ID: <20160213223824.25381.8002.stgit@birch.djwong.org> In-Reply-To: <20160213223725.25381.20929.stgit@birch.djwong.org> References: <20160213223725.25381.20929.stgit@birch.djwong.org> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" X-Source-IP: aserv0022.oracle.com [141.146.126.234] X-Evolution-Source: 1358860361.4566.33@ted Content-Transfer-Encoding: 8bit Richard Purdie reports that libext2fs doesn't sort attribute keys in the xattr block correctly, causing the kernel to return -ENODATA when querying attributes that should be there. Therefore, sort attributes so that whatever ends up in the xattr block is sorted according to what the kernel expects. Cc: Darren Hart Reported-by: Richard Purdie Signed-off-by: Darrick J. Wong --- lib/ext2fs/ext_attr.c | 24 +++++++++++- tests/d_xattr_sorting/expect | 29 ++++++++++++++ tests/d_xattr_sorting/name | 1 tests/d_xattr_sorting/script | 86 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 tests/d_xattr_sorting/expect create mode 100644 tests/d_xattr_sorting/name create mode 100644 tests/d_xattr_sorting/script Upstream-Status: Submitted diff --git a/lib/ext2fs/ext_attr.c b/lib/ext2fs/ext_attr.c index 0a4f8c0..b121837 100644 --- a/lib/ext2fs/ext_attr.c +++ b/lib/ext2fs/ext_attr.c @@ -254,10 +254,15 @@ static struct ea_name_index ea_names[] = { {0, NULL}, }; +static int find_ea_index(char *fullname, char **name, int *index); + /* Push empty attributes to the end and inlinedata to the front. */ static int attr_compare(const void *a, const void *b) { const struct ext2_xattr *xa = a, *xb = b; + char *xa_suffix, *xb_suffix; + int xa_idx, xb_idx; + int cmp; if (xa->name == NULL) return +1; @@ -267,7 +272,24 @@ static int attr_compare(const void *a, const void *b) return -1; else if (!strcmp(xb->name, "system.data")) return +1; - return 0; + + /* + * Duplicate the kernel's sorting algorithm because xattr blocks + * require sorted keys. + */ + xa_suffix = xa->name; + xb_suffix = xb->name; + xa_idx = xb_idx = 0; + find_ea_index(xa->name, &xa_suffix, &xa_idx); + find_ea_index(xb->name, &xb_suffix, &xb_idx); + cmp = xa_idx - xb_idx; + if (cmp) + return cmp; + cmp = strlen(xa_suffix) - strlen(xb_suffix); + if (cmp) + return cmp; + cmp = strcmp(xa_suffix, xb_suffix); + return cmp; } static const char *find_ea_prefix(int index) diff --git a/tests/d_xattr_sorting/expect b/tests/d_xattr_sorting/expect new file mode 100644 index 0000000..17da663 --- /dev/null +++ b/tests/d_xattr_sorting/expect @@ -0,0 +1,29 @@ +debugfs sort extended attributes +mke2fs -Fq -b 1024 test.img 512 +Exit status is 0 +ea_set / security.SMEG64 -f /tmp/b +Exit status is 0 +ea_set / security.imb -f /tmp/b +Exit status is 0 +ea_set / user.moo cow +Exit status is 0 +ea_list / +Extended attributes: + user.moo = "cow" (3) + security.imb = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" (256) + security.SMEG64 = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" (256) +Exit status is 0 +ea_get / security.imb +xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +Exit status is 0 +ea_get / nosuchea +ea_get: Extended attribute key not found while getting extended attribute +Exit status is 0 +e2fsck -yf -N test_filesys +Pass 1: Checking inodes, blocks, and sizes +Pass 2: Checking directory structure +Pass 3: Checking directory connectivity +Pass 4: Checking reference counts +Pass 5: Checking group summary information +test_filesys: 11/64 files (0.0% non-contiguous), 29/512 blocks +Exit status is 0 diff --git a/tests/d_xattr_sorting/name b/tests/d_xattr_sorting/name new file mode 100644 index 0000000..dde8926 --- /dev/null +++ b/tests/d_xattr_sorting/name @@ -0,0 +1 @@ +sort extended attributes in debugfs diff --git a/tests/d_xattr_sorting/script b/tests/d_xattr_sorting/script new file mode 100644 index 0000000..30c189a --- /dev/null +++ b/tests/d_xattr_sorting/script @@ -0,0 +1,86 @@ +if test -x $DEBUGFS_EXE; then + +OUT=$test_name.log +EXP=$test_dir/expect +VERIFY_FSCK_OPT=-yf + +TEST_DATA=$test_name.tmp +VERIFY_DATA=$test_name.ver.tmp + +echo "debugfs sort extended attributes" > $OUT + +dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1 + +echo "mke2fs -Fq -b 1024 test.img 512" >> $OUT + +$MKE2FS -Fq $TMPFILE 512 > /dev/null 2>&1 +status=$? +echo Exit status is $status >> $OUT + +perl -e 'print "x" x 256;' > /tmp/b + +echo "ea_set / security.SMEG64 -f /tmp/b" > $OUT.new +$DEBUGFS -w -R "ea_set / security.SMEG64 -f /tmp/b" $TMPFILE >> $OUT.new 2>&1 +status=$? +echo Exit status is $status >> $OUT.new +sed -f $cmd_dir/filter.sed $OUT.new >> $OUT + +echo "ea_set / security.imb -f /tmp/b" > $OUT.new +$DEBUGFS -w -R "ea_set / security.imb -f /tmp/b" $TMPFILE >> $OUT.new 2>&1 +status=$? +echo Exit status is $status >> $OUT.new +sed -f $cmd_dir/filter.sed $OUT.new >> $OUT + +echo "ea_set / user.moo cow" > $OUT.new +$DEBUGFS -w -R "ea_set / user.moo cow" $TMPFILE >> $OUT.new 2>&1 +status=$? +echo Exit status is $status >> $OUT.new +sed -f $cmd_dir/filter.sed $OUT.new >> $OUT + +rm -rf /tmp/b + +echo "ea_list /" > $OUT.new +$DEBUGFS -w -R "ea_list /" $TMPFILE >> $OUT.new 2>&1 +status=$? +echo Exit status is $status >> $OUT.new +sed -f $cmd_dir/filter.sed $OUT.new >> $OUT + +echo "ea_get / security.imb" > $OUT.new +$DEBUGFS -w -R "ea_get / security.imb" $TMPFILE >> $OUT.new 2>&1 +status=$? +echo Exit status is $status >> $OUT.new +sed -f $cmd_dir/filter.sed $OUT.new >> $OUT + +echo "ea_get / nosuchea" > $OUT.new +$DEBUGFS -w -R "ea_get / nosuchea" $TMPFILE >> $OUT.new 2>&1 +status=$? +echo Exit status is $status >> $OUT.new +sed -f $cmd_dir/filter.sed $OUT.new >> $OUT + +echo e2fsck $VERIFY_FSCK_OPT -N test_filesys > $OUT.new +$FSCK $VERIFY_FSCK_OPT -N test_filesys $TMPFILE >> $OUT.new 2>&1 +status=$? +echo Exit status is $status >> $OUT.new +sed -f $cmd_dir/filter.sed $OUT.new >> $OUT + +# +# Do the verification +# + +rm -f $TMPFILE $OUT.new +cmp -s $OUT $EXP +status=$? + +if [ "$status" = 0 ] ; then + echo "$test_name: $test_description: ok" + touch $test_name.ok +else + echo "$test_name: $test_description: failed" + diff $DIFF_OPTS $EXP $OUT > $test_name.failed +fi + +unset VERIFY_FSCK_OPT NATIVE_FSCK_OPT OUT EXP TEST_DATA VERIFY_DATA + +else #if test -x $DEBUGFS_EXE; then + echo "$test_name: $test_description: skipped" +fi