Patrick Williams | d8c66bc | 2016-06-20 12:57:21 -0500 | [diff] [blame] | 1 | From: "Darrick J. Wong" <darrick.wong@oracle.com> |
| 2 | To: tytso@mit.edu, darrick.wong@oracle.com |
| 3 | Cc: linux-ext4@vger.kernel.org, Darren Hart <dvhart@linux.intel.com>, |
| 4 | Richard Purdie <richard.purdie@linuxfoundation.org> |
| 5 | Date: Sat, 13 Feb 2016 14:38:24 -0800 |
| 6 | Message-ID: <20160213223824.25381.8002.stgit@birch.djwong.org> |
| 7 | In-Reply-To: <20160213223725.25381.20929.stgit@birch.djwong.org> |
| 8 | References: <20160213223725.25381.20929.stgit@birch.djwong.org> |
| 9 | User-Agent: StGit/0.17.1-dirty |
| 10 | MIME-Version: 1.0 |
| 11 | Content-Type: text/plain; charset="utf-8" |
| 12 | X-Source-IP: aserv0022.oracle.com [141.146.126.234] |
| 13 | X-Evolution-Source: 1358860361.4566.33@ted |
| 14 | Content-Transfer-Encoding: 8bit |
| 15 | |
| 16 | Richard Purdie reports that libext2fs doesn't sort attribute keys in |
| 17 | the xattr block correctly, causing the kernel to return -ENODATA when |
| 18 | querying attributes that should be there. Therefore, sort attributes |
| 19 | so that whatever ends up in the xattr block is sorted according to |
| 20 | what the kernel expects. |
| 21 | |
| 22 | Cc: Darren Hart <dvhart@linux.intel.com> |
| 23 | Reported-by: Richard Purdie <richard.purdie@linuxfoundation.org> |
| 24 | Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> |
| 25 | --- |
| 26 | lib/ext2fs/ext_attr.c | 24 +++++++++++- |
| 27 | tests/d_xattr_sorting/expect | 29 ++++++++++++++ |
| 28 | tests/d_xattr_sorting/name | 1 |
| 29 | tests/d_xattr_sorting/script | 86 ++++++++++++++++++++++++++++++++++++++++++ |
| 30 | 4 files changed, 139 insertions(+), 1 deletion(-) |
| 31 | create mode 100644 tests/d_xattr_sorting/expect |
| 32 | create mode 100644 tests/d_xattr_sorting/name |
| 33 | create mode 100644 tests/d_xattr_sorting/script |
| 34 | |
| 35 | Upstream-Status: Submitted |
| 36 | |
| 37 | |
| 38 | diff --git a/lib/ext2fs/ext_attr.c b/lib/ext2fs/ext_attr.c |
| 39 | index 0a4f8c0..b121837 100644 |
| 40 | --- a/lib/ext2fs/ext_attr.c |
| 41 | +++ b/lib/ext2fs/ext_attr.c |
| 42 | @@ -254,10 +254,15 @@ static struct ea_name_index ea_names[] = { |
| 43 | {0, NULL}, |
| 44 | }; |
| 45 | |
| 46 | +static int find_ea_index(char *fullname, char **name, int *index); |
| 47 | + |
| 48 | /* Push empty attributes to the end and inlinedata to the front. */ |
| 49 | static int attr_compare(const void *a, const void *b) |
| 50 | { |
| 51 | const struct ext2_xattr *xa = a, *xb = b; |
| 52 | + char *xa_suffix, *xb_suffix; |
| 53 | + int xa_idx, xb_idx; |
| 54 | + int cmp; |
| 55 | |
| 56 | if (xa->name == NULL) |
| 57 | return +1; |
| 58 | @@ -267,7 +272,24 @@ static int attr_compare(const void *a, const void *b) |
| 59 | return -1; |
| 60 | else if (!strcmp(xb->name, "system.data")) |
| 61 | return +1; |
| 62 | - return 0; |
| 63 | + |
| 64 | + /* |
| 65 | + * Duplicate the kernel's sorting algorithm because xattr blocks |
| 66 | + * require sorted keys. |
| 67 | + */ |
| 68 | + xa_suffix = xa->name; |
| 69 | + xb_suffix = xb->name; |
| 70 | + xa_idx = xb_idx = 0; |
| 71 | + find_ea_index(xa->name, &xa_suffix, &xa_idx); |
| 72 | + find_ea_index(xb->name, &xb_suffix, &xb_idx); |
| 73 | + cmp = xa_idx - xb_idx; |
| 74 | + if (cmp) |
| 75 | + return cmp; |
| 76 | + cmp = strlen(xa_suffix) - strlen(xb_suffix); |
| 77 | + if (cmp) |
| 78 | + return cmp; |
| 79 | + cmp = strcmp(xa_suffix, xb_suffix); |
| 80 | + return cmp; |
| 81 | } |
| 82 | |
| 83 | static const char *find_ea_prefix(int index) |
| 84 | diff --git a/tests/d_xattr_sorting/expect b/tests/d_xattr_sorting/expect |
| 85 | new file mode 100644 |
| 86 | index 0000000..17da663 |
| 87 | --- /dev/null |
| 88 | +++ b/tests/d_xattr_sorting/expect |
| 89 | @@ -0,0 +1,29 @@ |
| 90 | +debugfs sort extended attributes |
| 91 | +mke2fs -Fq -b 1024 test.img 512 |
| 92 | +Exit status is 0 |
| 93 | +ea_set / security.SMEG64 -f /tmp/b |
| 94 | +Exit status is 0 |
| 95 | +ea_set / security.imb -f /tmp/b |
| 96 | +Exit status is 0 |
| 97 | +ea_set / user.moo cow |
| 98 | +Exit status is 0 |
| 99 | +ea_list / |
| 100 | +Extended attributes: |
| 101 | + user.moo = "cow" (3) |
| 102 | + security.imb = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" (256) |
| 103 | + security.SMEG64 = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" (256) |
| 104 | +Exit status is 0 |
| 105 | +ea_get / security.imb |
| 106 | +xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx |
| 107 | +Exit status is 0 |
| 108 | +ea_get / nosuchea |
| 109 | +ea_get: Extended attribute key not found while getting extended attribute |
| 110 | +Exit status is 0 |
| 111 | +e2fsck -yf -N test_filesys |
| 112 | +Pass 1: Checking inodes, blocks, and sizes |
| 113 | +Pass 2: Checking directory structure |
| 114 | +Pass 3: Checking directory connectivity |
| 115 | +Pass 4: Checking reference counts |
| 116 | +Pass 5: Checking group summary information |
| 117 | +test_filesys: 11/64 files (0.0% non-contiguous), 29/512 blocks |
| 118 | +Exit status is 0 |
| 119 | diff --git a/tests/d_xattr_sorting/name b/tests/d_xattr_sorting/name |
| 120 | new file mode 100644 |
| 121 | index 0000000..dde8926 |
| 122 | --- /dev/null |
| 123 | +++ b/tests/d_xattr_sorting/name |
| 124 | @@ -0,0 +1 @@ |
| 125 | +sort extended attributes in debugfs |
| 126 | diff --git a/tests/d_xattr_sorting/script b/tests/d_xattr_sorting/script |
| 127 | new file mode 100644 |
| 128 | index 0000000..30c189a |
| 129 | --- /dev/null |
| 130 | +++ b/tests/d_xattr_sorting/script |
| 131 | @@ -0,0 +1,86 @@ |
| 132 | +if test -x $DEBUGFS_EXE; then |
| 133 | + |
| 134 | +OUT=$test_name.log |
| 135 | +EXP=$test_dir/expect |
| 136 | +VERIFY_FSCK_OPT=-yf |
| 137 | + |
| 138 | +TEST_DATA=$test_name.tmp |
| 139 | +VERIFY_DATA=$test_name.ver.tmp |
| 140 | + |
| 141 | +echo "debugfs sort extended attributes" > $OUT |
| 142 | + |
| 143 | +dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1 |
| 144 | + |
| 145 | +echo "mke2fs -Fq -b 1024 test.img 512" >> $OUT |
| 146 | + |
| 147 | +$MKE2FS -Fq $TMPFILE 512 > /dev/null 2>&1 |
| 148 | +status=$? |
| 149 | +echo Exit status is $status >> $OUT |
| 150 | + |
| 151 | +perl -e 'print "x" x 256;' > /tmp/b |
| 152 | + |
| 153 | +echo "ea_set / security.SMEG64 -f /tmp/b" > $OUT.new |
| 154 | +$DEBUGFS -w -R "ea_set / security.SMEG64 -f /tmp/b" $TMPFILE >> $OUT.new 2>&1 |
| 155 | +status=$? |
| 156 | +echo Exit status is $status >> $OUT.new |
| 157 | +sed -f $cmd_dir/filter.sed $OUT.new >> $OUT |
| 158 | + |
| 159 | +echo "ea_set / security.imb -f /tmp/b" > $OUT.new |
| 160 | +$DEBUGFS -w -R "ea_set / security.imb -f /tmp/b" $TMPFILE >> $OUT.new 2>&1 |
| 161 | +status=$? |
| 162 | +echo Exit status is $status >> $OUT.new |
| 163 | +sed -f $cmd_dir/filter.sed $OUT.new >> $OUT |
| 164 | + |
| 165 | +echo "ea_set / user.moo cow" > $OUT.new |
| 166 | +$DEBUGFS -w -R "ea_set / user.moo cow" $TMPFILE >> $OUT.new 2>&1 |
| 167 | +status=$? |
| 168 | +echo Exit status is $status >> $OUT.new |
| 169 | +sed -f $cmd_dir/filter.sed $OUT.new >> $OUT |
| 170 | + |
| 171 | +rm -rf /tmp/b |
| 172 | + |
| 173 | +echo "ea_list /" > $OUT.new |
| 174 | +$DEBUGFS -w -R "ea_list /" $TMPFILE >> $OUT.new 2>&1 |
| 175 | +status=$? |
| 176 | +echo Exit status is $status >> $OUT.new |
| 177 | +sed -f $cmd_dir/filter.sed $OUT.new >> $OUT |
| 178 | + |
| 179 | +echo "ea_get / security.imb" > $OUT.new |
| 180 | +$DEBUGFS -w -R "ea_get / security.imb" $TMPFILE >> $OUT.new 2>&1 |
| 181 | +status=$? |
| 182 | +echo Exit status is $status >> $OUT.new |
| 183 | +sed -f $cmd_dir/filter.sed $OUT.new >> $OUT |
| 184 | + |
| 185 | +echo "ea_get / nosuchea" > $OUT.new |
| 186 | +$DEBUGFS -w -R "ea_get / nosuchea" $TMPFILE >> $OUT.new 2>&1 |
| 187 | +status=$? |
| 188 | +echo Exit status is $status >> $OUT.new |
| 189 | +sed -f $cmd_dir/filter.sed $OUT.new >> $OUT |
| 190 | + |
| 191 | +echo e2fsck $VERIFY_FSCK_OPT -N test_filesys > $OUT.new |
| 192 | +$FSCK $VERIFY_FSCK_OPT -N test_filesys $TMPFILE >> $OUT.new 2>&1 |
| 193 | +status=$? |
| 194 | +echo Exit status is $status >> $OUT.new |
| 195 | +sed -f $cmd_dir/filter.sed $OUT.new >> $OUT |
| 196 | + |
| 197 | +# |
| 198 | +# Do the verification |
| 199 | +# |
| 200 | + |
| 201 | +rm -f $TMPFILE $OUT.new |
| 202 | +cmp -s $OUT $EXP |
| 203 | +status=$? |
| 204 | + |
| 205 | +if [ "$status" = 0 ] ; then |
| 206 | + echo "$test_name: $test_description: ok" |
| 207 | + touch $test_name.ok |
| 208 | +else |
| 209 | + echo "$test_name: $test_description: failed" |
| 210 | + diff $DIFF_OPTS $EXP $OUT > $test_name.failed |
| 211 | +fi |
| 212 | + |
| 213 | +unset VERIFY_FSCK_OPT NATIVE_FSCK_OPT OUT EXP TEST_DATA VERIFY_DATA |
| 214 | + |
| 215 | +else #if test -x $DEBUGFS_EXE; then |
| 216 | + echo "$test_name: $test_description: skipped" |
| 217 | +fi |
| 218 | |
| 219 | |