Bug 989284

Summary: KIWI images broken due to e2fsprogs update for U-Boot based boot, uses "64bit" feature
Product: [openSUSE] openSUSE Tumbleweed Reporter: Stefan Brüns <stefan.bruens>
Component: OtherAssignee: Jan Kara <jack>
Status: RESOLVED FIXED QA Contact: E-mail List <qa-bugs>
Severity: Normal    
Priority: P3 - Medium CC: afaerber, agraf, dmueller, matwey.kornilov, mbrugger, ms
Version: Current   
Target Milestone: ---   
Hardware: Other   
OS: Other   
Whiteboard:
Found By: --- Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---

Description Stefan Brüns 2016-07-17 16:54:12 UTC
e2fsprogs-1.43 adds the "64bit" feature for ext4 filesystems.

KIWI uses the default features from /etc/mke2fs.conf

U-Boot does not support ext4 with enabled 64bit feature. Thus this affects any system using U-Boot which stores kernel and initrd on the root filesystem. (e.g. rpi3 uses a seperate ext3 /boot partition, Pine64 uses the ext4 / root filesystem)


The issue can be worked around by e.g.:
"tune2fs -O ^64bit /dev/sdc2; e2fsck -y /dev/sdc2; resize2fs -s /dev/sdc2"

assuming sdc2 is the root filesystem partition on the SD card.

Affected image: e.g. http://download.opensuse.org/repositories/devel:/ARM:/Factory:/Contrib:/Pine64:/Downstream/images/
Comment 1 Stefan Brüns 2016-07-18 15:12:56 UTC
The fixup command is slightly wrong, it should be:
tune2fs -O ^64bit /dev/sdc2; e2fsck -f -y /dev/sdc2; resize2fs -s /dev/sdc2

As a temporary workaround, it should be possible to use <type ... bootpartition="true" ...> in the KIWI description.

This affects any board which uses bootloader="efi" instead of bootloader="uboot", the latter implicitly sets bootpartion, see https://github.com/openSUSE/kiwi/blob/master/modules/KIWIBoot.pm#L2001
Comment 2 Jan Kara 2016-07-19 11:20:44 UTC
OK, I could modify mke2fs.conf to not enable 64-bit feature for filesystems smaller than 4 TB but wouldn't it be more appropriate to disable 64-bit feature when creating the filesystem in KIWI using '-O ^64-bit' option? Or teach U-Boot how to deal with filesystems with 64-bit feature enabled?

The 64-bit feature got into defaults because its overhead is minimal and it allows filesystem to be grown over 16 TB. It is hard to guess whether the fs will be grown like that or not in advance and if users find the 16 TB limitation when they want to grow e.g. 4 TB filesystem to 20 TB, the time to fsck whole 4 TB is pretty big.
Comment 3 Jan Kara 2016-07-19 11:21:05 UTC
Forgot to set NEEDINFO...
Comment 4 Dirk Mueller 2016-07-19 12:35:32 UTC
LC_ALL=C tune2fs -O ^64bit /tmp/loop 
tune2fs 1.42.13 (17-May-2015)
Clearing filesystem feature '64bit' not supported.
Comment 5 Dirk Mueller 2016-07-19 12:42:52 UTC
ah, nevermind, outdated version of the tools installed.
Comment 6 Stefan Brüns 2016-07-25 23:24:40 UTC
There is one thing I am wondering about -

kernel and initrd are loaded by grub2 in this case (u-boot just loads grub2-efi). Is the problem caused by grub2 delegating the load to u-boot, or is the grub2 ext module missing support for the 64bit feature?
Comment 7 Jan Kara 2016-07-26 09:01:54 UTC
Current version of grub2 definitely supports 64-bit feature. So I guess it has to be the first... But I don't understand the details how grub2 and u-boot interact...

Andreas, Alex is it feasible to add 64-bit feature support to u-boot? For a bootloader support of 64-bit feature is usually pretty trivial - it only changes XATTR block number in an inode from 32 to 48 bits (but bootloader usually doesn't care about this since it ignores xattrs anyway) and it changes block numbers of inode and block bitmaps in the group descriptor from 32 to 48 bits (but again bootloader generally doesn't even parse group descriptors).
Comment 8 Andreas Färber 2016-07-26 10:54:40 UTC
Alex is the one who implemented UEFI and can hopefully explain the exact interaction. I would've expected GRUB to use its own filesystem and partition modules but U-Boot's block drivers - maybe I'm wrong.

If you want to implement this ext4 feature for U-Boot (GPLv2+) that would certainly be useful either way, some boards can't use GRUB yet.

http://git.denx.de/?p=u-boot.git;a=tree;f=fs/ext4;hb=HEAD
Comment 9 Alexander Graf 2016-07-26 11:10:32 UTC
Grub2 uses its own ext2 fs module, but U-Boot needs to load the device tree from the /boot partition, which then gets handled by U-Boot's ext driver.

So yes, the real solution would be to implement 64bit support in U-Boot.
Comment 10 Stefan Brüns 2016-08-01 16:58:02 UTC
Has anyone started working on the EXT4 U-Boot support? Otherwise I would give it a try ...
Comment 11 Jan Kara 2016-08-03 11:57:58 UTC
Not really. I had a look into U-Boot and found and that at least the upstream version does no checks of feature bitmaps. Do we carry some patches that change this?

Other than that feel free to go ahead and improve U-Boot.
Comment 12 Andreas Färber 2016-08-03 12:11:09 UTC
(In reply to Jan Kara from comment #11)
> I had a look into U-Boot and found [...] that at least the
> upstream version does no checks of feature bitmaps. Do we carry some patches
> that change this?

No, you can see our patch queue here:
https://github.com/openSUSE/u-boot/commits/tumbleweed
https://build.opensuse.org/package/show/Base:System/u-boot
Comment 13 Jan Kara 2016-08-04 08:22:00 UTC
OK. So u-boot fails not because it would check feature bits in superblock and find a feature that it does not understand but because it tries to use the values in group descriptors and fails.

Let me make one thing clear: U-boot support of ext2 / ext4 is *dangerous*. Any ext? driver must check feature bits in the superblock and make sure it understands *each* set bit in the feature_incompat set. If it intends to write to the filesystem, it must also make sure it understand *each* set bit in the feature_ro_compat set. U-boot seems to completely ignore feature bits and thus can read garbage or corrupt filesystem because of filesystem features it does not understand. The same holds for journal feature bits.

And I understand this means that u-boot may refuse to touch filesystems which it previously was able to access and it is a tedious task to propely implement all ext? features u-boot may need to support to access the filesystem but returning garbage or corrupting filesystem is worse.
Comment 14 Stefan Brüns 2016-08-04 17:19:10 UTC
(In reply to Jan Kara from comment #13)
> OK. So u-boot fails not because it would check feature bits in superblock
> and find a feature that it does not understand but because it tries to use
> the values in group descriptors and fails.
> 
> Let me make one thing clear: U-boot support of ext2 / ext4 is *dangerous*.
> Any ext? driver must check feature bits in the superblock and make sure it
> understands *each* set bit in the feature_incompat set. If it intends to
> write to the filesystem, it must also make sure it understand *each* set bit
> in the feature_ro_compat set. U-boot seems to completely ignore feature bits
> and thus can read garbage or corrupt filesystem because of filesystem
> features it does not understand. The same holds for journal feature bits.
> 
> And I understand this means that u-boot may refuse to touch filesystems
> which it previously was able to access and it is a tedious task to propely
> implement all ext? features u-boot may need to support to access the
> filesystem but returning garbage or corrupting filesystem is worse.

That matches what I have seen so far. E.g. the write code seems to completely lack any Endian awareness.

I think making U-Boot aware of the FEATURE_INCOMPAT flags and implement these to be working at least read-only for the default set in mke2fs.conf is not to hard. Most features seem to be already supported (e.g INCOMPAT_FILETYPE, INCOMPAT_EXTENTS), although this is undocumented. Stuff like INLINE_DATA should be trivial.

INCOMPAT_64BIT actually is trivial for the read-only, checksum-ignoring case. If the size of the group-descriptor size is set correctly, U-Boot is able to read the root fs from our images.
Comment 15 Matwey Kornilov 2016-08-05 18:54:48 UTC
Why do we need 64bit for ARM SD Images? Does anyone really has such a large SD card?
Comment 16 Alexander Graf 2016-08-06 09:18:58 UTC
(In reply to Matwey Kornilov from comment #15)
> Why do we need 64bit for ARM SD Images? Does anyone really has such a large
> SD card?

Not all targets have to be an SD card. The RPi for example just grew support for USB boot, in which case you could put the image on storage that could definitely exceed the 32bit boundaries.
Comment 17 Matwey Kornilov 2016-08-08 12:00:32 UTC
Why don't move to btrfs then?
Comment 18 Alexander Graf 2016-08-08 12:07:45 UTC
(In reply to Matwey Kornilov from comment #17)
> Why don't move to btrfs then?

Can U-Boot read the device tree from btrfs for systems where U-Boot doesn't integrate a working one?
Comment 19 Stefan Brüns 2016-08-08 12:09:48 UTC
(In reply to Alexander Graf from comment #18)
> (In reply to Matwey Kornilov from comment #17)
> > Why don't move to btrfs then?
> 
> Can U-Boot read the device tree from btrfs for systems where U-Boot doesn't
> integrate a working one?

Unlikely:

ls ~/Sources/u-boot/fs/
cbfs  cramfs  ext4  fat  fs.c  jffs2  Kconfig  Makefile  reiserfs  sandbox  ubifs  yaffs2  zfs
Comment 20 Andreas Färber 2016-08-08 13:17:27 UTC
Hm, I recall seeing btrfs support on the Turris Omnia (https://en.opensuse.org/HCL:Turris_Omnia), but I don't find "btrload" in my mainline development tree. Anyway, it seemed incompatible with the generic load command and thus distro framework, since it wanted the volume name (e.g., @) as additional trailing argument.
Comment 21 Stefan Brüns 2016-08-10 15:40:10 UTC
Just a small warning, next U-Boot will refuse to mount FSs with FEATURE_64BIT:
http://git.denx.de/?p=u-boot.git;a=commit;h=6f94ab6656ceffb3f2a972c8de4c554502b6f2b7

Also, writes on ext3/4 have been confirmed to be broken on big endian:
http://lists.denx.de/pipermail/u-boot/2016-August/263029.html

Fortunately, big endian will never see the INCOMPAT_RECOVER flag and screw up your FS in an attempt to fix it. But stay away from ext4write ...
Comment 22 Matwey Kornilov 2016-09-07 16:56:45 UTC
(In reply to Alexander Graf from comment #16)
> (In reply to Matwey Kornilov from comment #15)
> > Why do we need 64bit for ARM SD Images? Does anyone really has such a large
> > SD card?
> 
> Not all targets have to be an SD card. The RPi for example just grew support
> for USB boot, in which case you could put the image on storage that could
> definitely exceed the 32bit boundaries.

But, we will not be able to boot from it anyway, right?

So, I suppose e2fsprogs must be patched not to use 64bit by default until u-boot grew 64bit support.
Comment 23 Stefan Brüns 2016-09-09 15:57:02 UTC
(In reply to Matwey Kornilov from comment #22)
> (In reply to Alexander Graf from comment #16)
> > (In reply to Matwey Kornilov from comment #15)
> > > Why do we need 64bit for ARM SD Images? Does anyone really has such a large
> > > SD card?
> > 
> > Not all targets have to be an SD card. The RPi for example just grew support
> > for USB boot, in which case you could put the image on storage that could
> > definitely exceed the 32bit boundaries.
> 
> But, we will not be able to boot from it anyway, right?

On the RPi3, this should be possible. On the RPi1/2, you still need the primary bootloader on the SD card, but after that everything can be on the harddisk.

And anyway, there is more stuff than RPis ...
 
> So, I suppose e2fsprogs must be patched not to use 64bit by default until
> u-boot grew 64bit support.

Experimental patch is available.

Another important aspect of the 64bit feature, it allows to add checksums to *all* ext4 metadata.
Comment 24 Dirk Mueller 2016-09-27 16:47:38 UTC
so I added a workaround to the ARM images disabling the 64bit feature during image build so that we're no longer affected by this problem until it is properly solved.
Comment 25 Stefan Brüns 2017-01-15 20:05:09 UTC
The fix has been included in u-boot 2016.11-rc1:

http://git.denx.de/?p=u-boot.git;a=commit;h=b4976b49a01ac68f7dbb33657a44dcffe584fa4a

which has arrived in Base:System meanwhile:
https://build.opensuse.org/package/show/Base:System/u-boot?rev=228
Comment 26 Swamp Workflow Management 2018-02-01 10:30:36 UTC
This is an autogenerated message for OBS integration:
This bug (989284) was mentioned in
https://build.opensuse.org/request/show/571642 15.0:Ports / JeOS
Comment 27 Swamp Workflow Management 2018-04-18 14:40:31 UTC
This is an autogenerated message for OBS integration:
This bug (989284) was mentioned in
https://build.opensuse.org/request/show/597801 Factory:ARM:Live / JeOS
Comment 28 Swamp Workflow Management 2019-04-02 16:00:41 UTC
This is an autogenerated message for OBS integration:
This bug (989284) was mentioned in
https://build.opensuse.org/request/show/690698 15.1:ARM:Images / JeOS
Comment 29 OBSbugzilla Bot 2021-04-27 07:40:40 UTC
This is an autogenerated message for OBS integration:
This bug (989284) was mentioned in
https://build.opensuse.org/request/show/888681 Factory:ARM / JeOS
Comment 30 OBSbugzilla Bot 2021-10-12 16:42:27 UTC
This is an autogenerated message for OBS integration:
This bug (989284) was mentioned in
https://build.opensuse.org/request/show/924915 Factory:ARM / JeOS
Comment 31 OBSbugzilla Bot 2021-10-12 18:42:08 UTC
This is an autogenerated message for OBS integration:
This bug (989284) was mentioned in
https://build.opensuse.org/request/show/924923 Factory:ARM / JeOS