On Sat, 2016-02-06 at 11:23 +0000, Richard Purdie wrote:
> I'm using the -d option of mke2fs to construct a filesystem, I'm
> seeing
> that some xattrs are being corrupted. The filesystem builds with no
> errors but when mounted by the kernel, I see errors like
> "security.ima:
> No such attribute". The strace from such a failure is:
Interesting. +Ted and +Darrick who helped us merge the -d argument
originally.
> mmap(NULL, 26258, PROT_READ, MAP_SHARED, 3, 0) = 0x7fdb36a8c000
> close(3) = 0
> getrlimit(RLIMIT_NOFILE, {rlim_cur=1024, rlim_max=64*1024}) = 0
> lstat("mnt/foobar", {st_mode=S_IFREG|0755, st_size=1, ...}) = 0
> listxattr("mnt/foobar", NULL, 0) = 30
> listxattr("mnt/foobar", "security.SMACK64\0security.ima\0", 256) = 30
> getxattr("mnt/foobar", "security.SMACK64", 0x0, 0) = 1
> getxattr("mnt/foobar", "security.SMACK64", "_", 256) = 1
> fstat(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(136, 13), ...}) = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
> 0) = 0x7fdb36a8b000
> write(1, "# file: mnt/foobar\n", 19# file: mnt/foobar) = 19
> write(1, "security.SMACK64=\"_\"\n", 21security.SMACK64="_") = 21
> getxattr("mnt/foobar", "security.ima", 0x0, 0) = -1 ENODATA (No data
> available)
> write(2, "mnt/foobar: ", 12mnt/foobar: ) = 12
> write(2, "security.ima: No such attribute\n", 32security.ima: No such
> attribute) = 32= 32
>
> so the attribute is there but the kernel gives ENODATA when trying
> to read it.
>
> http://www.nongnu.org/ext2-doc/ext2.html#CONTRIB-EXTENDED-ATTRIBUTES
> co
> ntains the small snippet that " The entry descriptors are sorted by
> attribute name, so that two extended attribute blocks can be compared
> efficiently. ". It doesn't specify what kind of sort.
>
> Looking at ext2fs, there is some sorting code through the qsort call
> using attr_compare() but it doesn't match what the kernel is doing in
> ext4_xattr_find_entry().
>
> I put together this quick patch to test my theory that this causing
> the
> problem:
>
>
> This makes my filesystems work.
>
> Is this a bug? I'm assuming ext2fs shouldn't generate filesystems the
> kernel can't read? Is the above the correct fix?
>
Reviewing the kernel ext4_attr_find_entry():
...
if (cmp <= 0 && (sorted || cmp == 0))
break;
}
*pentry = entry;
if (!cmp && ext4_xattr_check_entry(entry, size))
return -EFSCORRUPTED;
return cmp ? -ENODATA : 0;
...
It would seem that a different sorting algorithm would result in the
kernel interpreting the FS to be corrupted.
> Cheers,
>
> Richard
> ---
> To unsubscribe from this list: send the line "unsubscribe linux-ext4"
> in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> Index: git/lib/ext2fs/ext_attr.c
> ===================================================================
> --- git.orig/lib/ext2fs/ext_attr.c
> +++ git/lib/ext2fs/ext_attr.c
> @@ -258,6 +258,7 @@ static struct ea_name_index ea_names[] =
> static int attr_compare(const void *a, const void *b)
> {
> const struct ext2_xattr *xa = a, *xb = b;
> + size_t len;
>
> if (xa->name == NULL)
> return +1;
> @@ -267,7 +268,11 @@ static int attr_compare(const void *a, c
> return -1;
> else if (!strcmp(xb->name, "system.data"))
> return +1;
> - return 0;
> + len = strlen(xa->name) - strlen(xb->name);
> + if (len)
> + return len;
I *think* the index and len comparisons in the kernel are simply
optimizations to avoid the memcmp, but to properly sort them here, I
think you can drop the len block above and just return the strcmp
below.
Ted, Darrick?
> +
> + return strcmp(xa->name, xb->name);
> }
>
> static const char *find_ea_prefix(int index)
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
