Re: [U-Boot] ext4 delete file fails when ext4 extents enabled in filesystem
On Donnerstag, 1. September 2016 19:25:30 CEST you wrote:
On Donnerstag, 1. September 2016 16:08:51 CEST you wrote:
Hi Stefan,
applying patch [U-Boot,v4,06/13]ext4 and Michael Walles patch [U-Boot,v4,3/4]ext4, I'm now able to write into directories on ext4 fs from u-boot. However, when deleting a given file (i.e. when writing to an existing filename), u-boot crashes when ext4 extents are enabled.
Some debugging showd that blknr from 'read_allocated_block' function returns negative value. I can only guess, maybe its due to 64 bit values calculated from ee_start_hi and ee_start_lo entries in the ext4_extent structure.
When disabling extents in the ext4 fs, deleting a given file is working.
Hi Thomas,
U-boots ext4 implementation currently does not support 64bit or even 48bit block numbers, so this may be the cause.
Can you provide some information about your test setup?
You can use the debugsfs ext tool to gather some information about the problematic file. Just access the filesystem with:
/sbin/debugfs /dev/sda1 ; (or whatever your partion name is)
or
/sbin/debugfs /path/to/imagefile
debugfs supports commands like cd, stat, ls. stat gives you the block number list.
Kind regards,
Stefan
Hi Thomas,
short followup:
read_allocated_blocks returns either 0 or -1 in case of an error. Unfortunately, the return value is only checked for 0 equality in most/all? cases, and seemingly my patch series introduced some more occasions.
Now, what *should* read_allocated_blocks return in case of an error? Either:
- 0: a file block can never be allocated as block 0, as that is always in use by the superblock and/or the bootsector block.
- <0: Extents allow 48 bit block numbers. "Limiting" the return value to the positive half of int64_t for valid block numbers and and reserving negative values for error codes is fine.
I would go for negative error codes, as these are more expressive. Comments/ opinions welcome!
I will update the patch series for correct checking of read_allocated_blocks return values and fix all the other block number checks.
Anyway, it would be good to know why *exactly* read_allocated_blocks returns an error code in your case. Do you remember the exact negative value returned (there are -EINVAL and -ENOMEN, and many several unspecific uses of 0 and -1).
Can you provide a disk image of the failing file system?
Kind regards,
Stefan
-----Ursprüngliche Nachricht----- Von: Brüns, Stefan [mailto:Stefan.Bruens@rwth-aachen.de] Gesendet: Freitag, 2. September 2016 13:43 An: Thomas Schaefer Cc: u-boot@lists.denx.de; Michael Walle Betreff: Re: ext4 delete file fails when ext4 extents enabled in filesystem
On Donnerstag, 1. September 2016 19:25:30 CEST you wrote:
On Donnerstag, 1. September 2016 16:08:51 CEST you wrote:
Hi Stefan,
applying patch [U-Boot,v4,06/13]ext4 and Michael Walles patch [U-Boot,v4,3/4]ext4, I'm now able to write into directories on ext4 fs from u-boot. However, when deleting a given file (i.e. when writing to an existing filename), u-boot crashes when ext4 extents are enabled.
Some debugging showd that blknr from 'read_allocated_block' function returns negative value. I can only guess, maybe its due to 64 bit values calculated from ee_start_hi and ee_start_lo entries in the ext4_extent structure.
When disabling extents in the ext4 fs, deleting a given file is working.
Hi Thomas,
U-boots ext4 implementation currently does not support 64bit or even 48bit block numbers, so this may be the cause.
Can you provide some information about your test setup?
You can use the debugsfs ext tool to gather some information about the problematic file. Just access the filesystem with:
/sbin/debugfs /dev/sda1 ; (or whatever your partion name is)
or
/sbin/debugfs /path/to/imagefile
debugfs supports commands like cd, stat, ls. stat gives you the block number list.
Kind regards,
Stefan
Hi Thomas,
short followup:
read_allocated_blocks returns either 0 or -1 in case of an error. Unfortunately, the return value is only checked for 0 equality in most/all? cases, and seemingly my patch series introduced some more occasions.
Now, what *should* read_allocated_blocks return in case of an error? Either:
0: a file block can never be allocated as block 0, as that is always in use by the superblock and/or the bootsector block.
<0: Extents allow 48 bit block numbers. "Limiting" the return value to the positive half of int64_t for valid block numbers and and reserving negative values for error codes is fine.
I would go for negative error codes, as these are more expressive. Comments/ opinions welcome!
I will update the patch series for correct checking of read_allocated_blocks return values and fix all the other block number checks.
Anyway, it would be good to know why *exactly* read_allocated_blocks returns an error code in your case. Do you remember the exact negative value returned (there are -EINVAL and -> > > > > ENOMEN, and many several unspecific uses of 0 and -1).
Can you provide a disk image of the failing file system?
Kind regards,
Stefan
Hi Stefan,
the attachment contains an image file that causes u-boot to crash when trying to overwrite existing files in ext4 fs.
Calling debugfs show the following output
root@s1909:~# /sbin/debugfs /dev/mmcblk0p1 debugfs 1.43-WIP (18-May-2015) debugfs: ls 2 (12) . 2 (12) .. 11 (20) lost+found 12 (12) foo 7729 (4040) images debugfs: ls foo 12 (12) . 2 (12) .. 13 (16) test.bin 14 (4056) test2.bin debugfs: stat foo/test.bin Inode: 13 Type: regular Mode: 0644 Flags: 0x80000 Generation: 2312001584 Version: 0x00000000:00000001 User: 0 Group: 0 Size: 559364 File ACL: 0 Directory ACL: 0 Links: 1 Blockcount: 1096 Fragment: Address: 0 Number: 0 Size: 0 ctime: 0x57c93d53:3a2e2ac0 -- Fri Sep 2 08:50:27 2016 atime: 0x57c98a2b:00001708 -- Fri Sep 2 14:18:19 2016 mtime: 0x57c93d53:3a2e2ac0 -- Fri Sep 2 08:50:27 2016 crtime: 0x57c93d53:243eee64 -- Fri Sep 2 08:50:27 2016 Size of extra inode fields: 32 EXTENTS: (0-136):32852-32988 debugfs: stat foo/test2.bin Inode: 14 Type: regular Mode: 0644 Flags: 0x80000 Generation: 2312001585 Version: 0x00000000:00000001 User: 0 Group: 0 Size: 559364 File ACL: 0 Directory ACL: 0 Links: 1 Blockcount: 1096 Fragment: Address: 0 Number: 0 Size: 0 ctime: 0x57c93d5d:e206ed50 -- Fri Sep 2 08:50:37 2016 atime: 0x57c93d5d:cc17b0f4 -- Fri Sep 2 08:50:37 2016 mtime: 0x57c93d5d:e206ed50 -- Fri Sep 2 08:50:37 2016 crtime: 0x57c93d5d:cc17b0f4 -- Fri Sep 2 08:50:37 2016 Size of extra inode fields: 32 EXTENTS: (0-136):32989-33125 debugfs: q
Thank you and best regards, Thomas
On Freitag, 2. September 2016 14:51:59 CEST Thomas Schaefer wrote:
-----Ursprüngliche Nachricht----- Von: Brüns, Stefan [mailto:Stefan.Bruens@rwth-aachen.de] Gesendet: Freitag, 2. September 2016 13:43 An: Thomas Schaefer Cc: u-boot@lists.denx.de; Michael Walle Betreff: Re: ext4 delete file fails when ext4 extents enabled in filesystem
On Donnerstag, 1. September 2016 19:25:30 CEST you wrote:
On Donnerstag, 1. September 2016 16:08:51 CEST you wrote:
Hi Stefan,
applying patch [U-Boot,v4,06/13]ext4 and Michael Walles patch [U-Boot,v4,3/4]ext4, I'm now able to write into directories on ext4 fs from u-boot. However, when deleting a given file (i.e. when writing to an existing filename), u-boot crashes when ext4 extents are enabled.
Some debugging showd that blknr from 'read_allocated_block' function returns negative value. I can only guess, maybe its due to 64 bit values calculated from ee_start_hi and ee_start_lo entries in the ext4_extent structure.
When disabling extents in the ext4 fs, deleting a given file is working.
[...]
Hi Thomas,
short followup:
read_allocated_blocks returns either 0 or -1 in case of an error. Unfortunately, the return value is only checked for 0 equality in most/all? cases, and seemingly my patch series introduced some more occasions.
Now, what *should* read_allocated_blocks return in case of an error? Either:
- 0: a file block can never be allocated as block 0, as that is always in
use by the superblock and/or the bootsector block.
- <0: Extents allow 48 bit block numbers. "Limiting" the return value to
the positive half of int64_t for valid block numbers and and reserving negative values for error codes is fine.
I would go for negative error codes, as these are more expressive. Comments/ opinions welcome!
Following up on this, the correct behaviour is <0 on "real" errors, like - ENOMEM, and 0 on blocks not backed on device (e.g. sparse files).
Followup patch in progress.
[...]
Hi Stefan,
the attachment contains an image file that causes u-boot to crash when trying to overwrite existing files in ext4 fs.
Could reproduce this. The problem seems to be an out-of-bound access of an in- memory struct ext2_inode, and mixing up its size with fs->inodesz. I have found at least one place, will investigate further.
Kind regards,
Stefan
participants (2)
-
Brüns, Stefan -
Thomas Schaefer