[U-Boot] [PATCH] ext4: fix possible crash on directory traversal, ignore deleted entries

[Stefan Bruens]

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 at 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?

Kind regards,

Stefan

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019
work: +49 2405 49936-424