U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Bruens <stefan.bruens@rwth-aachen.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] ext4: fix possible crash on directory traversal, ignore deleted entries
Date: Sun, 28 Aug 2016 21:44:41 +0200	[thread overview]
Message-ID: <5096748.Z86yB2JU2R@pebbles.site> (raw)
In-Reply-To: <20160819195451.GI5342@bill-the-cat>

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?

Kind regards,

Stefan

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

  reply	other threads:[~2016-08-28 19:44 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-14  3:11 [U-Boot] [PATCH] ext4: fix possible crash on directory traversal, ignore deleted entries Stefan Brüns
2016-08-19 19:54 ` Tom Rini
2016-08-28 19:44   ` Stefan Bruens [this message]
2016-09-02 14:53     ` Tom Rini
2016-09-09 16:47       ` Brüns, Stefan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5096748.Z86yB2JU2R@pebbles.site \
    --to=stefan.bruens@rwth-aachen.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox