On Sun, Aug 28, 2016 at 09:44:41PM +0200, Stefan Bruens wrote:
On Freitag, 19. August 2016 15:54:51 CEST you wrote:
On Sun, Aug 14, 2016 at 05:11:04AM +0200, Stefan Brüns wrote:
The following command triggers a segfault in search_dir: ./sandbox/u-boot -c 'host bind 0 ./sandbox/test/fs/3GB.ext4.img ;
ext4write host 0 0 /./foo 0x10'The following command triggers a segfault in check_filename: ./sandbox/u-boot -c 'host bind 0 ./sandbox/test/fs/3GB.ext4.img ;
ext4write host 0 0 /. 0x10'"." is the first entry in the directory, thus previous_dir is NULL. The whole previous_dir block in search_dir seems to be a bad copy from check_filename(...). As the changed data is not written to disk, the statement is mostly harmless, save the possible NULL-ptr reference.
Typically a file is unlinked by extending the direntlen of the previous entry. If the entry is the first entry in the directory block, it is invalidated by setting inode=0.
The inode==0 case is hard to trigger without crafted filesystems. It only hits if the first entry in a directory block is deleted and later a lookup for the entry (by name) is done.
Signed-off-by: Stefan Brüns stefan.bruens@rwth-aachen.de
fs/ext4/ext4_common.c | 57 ++++++++++++++++++--------------------------------- fs/ext4/ext4_write.c | 2 +- include/ext4fs.h | 2 +- 3 files changed, 22 insertions(+), 39 deletions(-)
Can you please add the test case to the existing scripts? Thanks!
Adding this to the current test script is somewhat problematic. The test runs all tests for fat and ext4, so each testcase should be file system agnostic. Unfortunately fat and ext4 (at least as implemented in U-Boot) have different semantics, as ext4 in U-Boot requires all path to absolute paths, whereas fat seems to require something else (relative path? absolute path, but without leading '/'?).
Calling 'fatwrite host 0 0 /. 0x10' happily creates a directory! called '/.', 'fatwrite host 0 0 /./foo 0x10' creates a file and copletely messes up the filesystem (according to fsck.vfat and mounting the fs in linux).
Any advise?
Can we fix this up in the argument parsing? This sounds like it's showing some bugs in the fatwrite parsing code itself.