xattr hash error in 4.13-rc with overlayfs over ext4

xattr hash error in 4.13-rc with overlayfs over ext4

[Miklos Szeredi]

I get the following error on fsck:

Pass 1: Checking inodes, blocks, and sizes
Extended attribute in inode 3093 has a hash (2257320705) which is invalid
Clear<y>? yes
Inode 3093, i_blocks is 16, should be 8.  Fix<y>? yes

To reproduce:

[mount ext4 image]
mkdir xattrhashtest
cd xattrhashtest/
mkdir upper lower work mnt
cd lower/
echo foo > bar
ln bar baz
mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work mnt
cd mnt/
echo xxx >> bar
[umount and fsck]

Reverting commit daf8328172df ("ext4: eliminate xattr entry e_hash
recalculation for removes") fixes it.

Might be an ext4 or overlayfs bug.  Let me know what you think.

Thanks,
Miklos

Re: xattr hash error in 4.13-rc with overlayfs over ext4

[Tahsin Erdogan]

Hi Miklos,
I made a first attempt to reproduce the failure but did not get lucky.

> Inode 3093, i_blocks is 16, should be 8.  Fix<y>? yes
Does this inode correspond to foo, bar or a preexisting file?

Do you mind sharing the output of the following command?
debugfs -R "stat <3093>" /dev/${ext4_dev}

thanks

Re: xattr hash error in 4.13-rc with overlayfs over ext4

[Miklos Szeredi]

On Thu, Jul 27, 2017 at 8:20 PM, Tahsin Erdogan <tahsin@google.com> wrote:
> Hi Miklos,
> I made a first attempt to reproduce the failure but did not get lucky.
>
>> Inode 3093, i_blocks is 16, should be 8.  Fix<y>? yes
> Does this inode correspond to foo, bar or a preexisting file?
>
> Do you mind sharing the output of the following command?
> debugfs -R "stat <3093>" /dev/${ext4_dev}

Inode: 17093   Type: regular    Mode:  0644   Flags: 0x80000
Generation: 2184376062    Version: 0x00000000:00000001
User:     0   Group:     0   Project:     0   Size: 8
File ACL: 65561    Directory ACL: 0
Links: 2   Blockcount: 16
Fragment:  Address: 0    Number: 0    Size: 0
 ctime: 0x597a3e23:7d3fc138 -- Thu Jul 27 19:25:23 2017
 atime: 0x597a3dfc:a80bc138 -- Thu Jul 27 19:24:44 2017
 mtime: 0x597a3e23:7d3fc138 -- Thu Jul 27 19:25:23 2017
crtime: 0x597a3e23:787b0938 -- Thu Jul 27 19:25:23 2017
Size of extra inode fields: 32
EXTENTS:
(0):142340

Resulting fsck output:

Pass 1: Checking inodes, blocks, and sizes
Extended attribute in inode 17093 has a hash (2257320705) which is invalid
Clear<y>? yes
Inode 17093, i_blocks is 16, should be 8.  Fix<y>? yes
Pass 2: Checking directory structure
Pass 3: Checking directory connectivity
Pass 4: Checking reference counts
Pass 5: Checking group summary information
Block bitmap differences:  -65561
Fix<y>? yes
Free blocks count wrong for group #2 (13797, counted=13798).
Fix<y>? yes
Free blocks count wrong (86361, counted=86362).
Fix<y>? yes

Re: xattr hash error in 4.13-rc with overlayfs over ext4

[Miklos Szeredi]

On Thu, Jul 27, 2017 at 9:28 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Thu, Jul 27, 2017 at 8:20 PM, Tahsin Erdogan <tahsin@google.com> wrote:
>> Hi Miklos,
>> I made a first attempt to reproduce the failure but did not get lucky.
>>
>>> Inode 3093, i_blocks is 16, should be 8.  Fix<y>? yes
>> Does this inode correspond to foo, bar or a preexisting file?

The inode in question corresponds to the copy-up of bar into upper.

There's quite a lot overlayfs does in this apparently simple
operation, and that's probably why I've not been able to reproduce
without overlayfs (although admittedly I didn't try very hard).

It's basically the following:

 - create tempfile
 - copy data from lower/bar to work/index/XXX  using reflink
 - copy metadata
 - set trusted.overlay.origin xattr
 - set trusted.overlay.nlink xattr
 - hardlink index file to upper/bar
 - overwrite trusted.overlay.nlink with different value

And ther's also some attr and xattr setting going on for the upper directory.

Thanks,
Miklos

Re: xattr hash error in 4.13-rc with overlayfs over ext4

[Tahsin Erdogan]

Still no luck reproducing the error. I am using the test script below
at v4.13-rc2. Do you mind checking my script to see what I am missing?

#!/bin/bash
set -ex

while grep /dev/sdb /proc/mounts;do umount /dev/sdb; done
mke2fs -F -t ext4 /dev/sdb
mount /dev/sdb /mnt/sdb
cd /mnt/sdb

mkdir xattrhashtest
cd xattrhashtest/
mkdir upper lower work mnt
cd lower/
echo foo > bar
ln bar baz
cd ..
mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work mnt
cd mnt/
echo xxx >> bar

cd /root
umount overlay
umount /dev/sdb

e2fsck -f -n /dev/sdb
echo done

Re: xattr hash error in 4.13-rc with overlayfs over ext4

[Miklos Szeredi]

On Thu, Jul 27, 2017 at 10:54 PM, Tahsin Erdogan <tahsin@google.com> wrote:
> Still no luck reproducing the error. I am using the test script below
> at v4.13-rc2. Do you mind checking my script to see what I am missing?

Don't know.   I can reliably reproduce it under UML, but not with a
VM.  What the hell is going on?

Thanks,
Miklos

Re: xattr hash error in 4.13-rc with overlayfs over ext4

[Amir Goldstein]

On Mon, Jul 31, 2017 at 7:08 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Thu, Jul 27, 2017 at 10:54 PM, Tahsin Erdogan <tahsin@google.com> wrote:
>> Still no luck reproducing the error. I am using the test script below
>> at v4.13-rc2. Do you mind checking my script to see what I am missing?
>
> Don't know.   I can reliably reproduce it under UML, but not with a
> VM.  What the hell is going on?
>

Is it the same ext4 image for UML and VM? same image size? same mkfs version?

What inode size do you have in UML setup? e.g.:

dumpe2fs -h /dev/sda3|grep Inode.size
dumpe2fs 1.42.13 (17-May-2015)
Inode size:          256

have extra_isize feature? (not sure if that matters)

dumpe2fs -h /dev/sda3|grep isize
dumpe2fs 1.42.13 (17-May-2015)
Filesystem features:      has_journal ext_attr resize_inode dir_index
filetype needs_recovery extent flex_bg sparse_super large_file
huge_file uninit_bg dir_nlink extra_isize
Required extra isize:     28
Desired extra isize:      28

Re: xattr hash error in 4.13-rc with overlayfs over ext4

[Miklos Szeredi]

On Tue, Aug 1, 2017 at 12:34 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Mon, Jul 31, 2017 at 7:08 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>> On Thu, Jul 27, 2017 at 10:54 PM, Tahsin Erdogan <tahsin@google.com> wrote:
>>> Still no luck reproducing the error. I am using the test script below
>>> at v4.13-rc2. Do you mind checking my script to see what I am missing?
>>
>> Don't know.   I can reliably reproduce it under UML, but not with a
>> VM.  What the hell is going on?
>>
>
> Is it the same ext4 image for UML and VM? same image size? same mkfs version?

Exact same starting image.  Same userspace.  Same EXT4 kernel config options.

Thanks,
Miklos

Re: xattr hash error in 4.13-rc with overlayfs over ext4

[zhangyi (F)]

On 2017/8/3 19:10, Miklos Szeredi Wrote:
> On Tue, Aug 1, 2017 at 12:34 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>> On Mon, Jul 31, 2017 at 7:08 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>> On Thu, Jul 27, 2017 at 10:54 PM, Tahsin Erdogan <tahsin@google.com> wrote:
>>>> Still no luck reproducing the error. I am using the test script below
>>>> at v4.13-rc2. Do you mind checking my script to see what I am missing?
>>>
>>> Don't know.   I can reliably reproduce it under UML, but not with a
>>> VM.  What the hell is going on?
>>>
>>
>> Is it the same ext4 image for UML and VM? same image size? same mkfs version?
> 
> Exact same starting image.  Same userspace.  Same EXT4 kernel config options.
> 
> Thanks,
> Miklos
> 

Hi:
I think the root cause of this problem is the hash value not updated
when we overwrite the same xattr, please see ext4_xattr_set_entry():

static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
				struct ext4_xattr_search *s,
				handle_t *handle, struct inode *inode,
				bool is_block)
{
...
	if (new_size && new_size == old_size) {
		size_t offs = le16_to_cpu(here->e_value_offs);
		void *val = s->base + offs;

		here->e_value_size = cpu_to_le32(i->value_len);
		if (i->value == EXT4_ZERO_XATTR_VALUE) {
			memset(val, 0, new_size);
		} else {
			memcpy(val, i->value, i->value_len);
			/* Clear padding bytes. */
			memset(val + i->value_len, 0, new_size - i->value_len);
		}

		/* Forget updating *here->e_hash* and *s->base->h_hash* here. */

		return 0;
	}
...
	/* Call ext4_xattr_hash_entry && ext4_xattr_rehash first set */
...
}

Thanks,
ZhangYi

Re: xattr hash error in 4.13-rc with overlayfs over ext4

[Tahsin Erdogan]

On Sun, Aug 6, 2017 at 5:47 AM, zhangyi (F) <yizhang089@gmail.com> wrote:
> I think the root cause of this problem is the hash value not updated
> when we overwrite the same xattr, please see ext4_xattr_set_entry():

Yes, you are right. Thanks a lot for finding the root cause.

I just sent out a fix: ("[PATCH] ext4: add missing xattr hash update")

http://patchwork.ozlabs.org/patch/798445/