[U-Boot] [PATCH] ext4: fix possible crash on directory traversal, ignore deleted entries
Tom Rini
trini at konsulko.com
Fri Sep 2 16:53:08 CEST 2016
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 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?
Can we fix this up in the argument parsing? This sounds like it's
showing some bugs in the fatwrite parsing code itself.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160902/a460e4ad/attachment.sig>
More information about the U-Boot
mailing list