U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Brüns, Stefan" <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: Fri, 9 Sep 2016 16:47:09 +0000	[thread overview]
Message-ID: <1773564.W4YydK1XsB@sbruens-linux> (raw)
In-Reply-To: <20160902145308.GE4990@bill-the-cat>

On Freitag, 2. September 2016 10:53:08 CEST you wrote:
> > 
> > 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.

The fatwrite code is hardly doing any parsing at all. It does not strip any 
"/" or "\" characters, does not interpret these as dir delimiters, and just 
pushes whatever it gets into the directory.

For the lookup, it uses a function which is quite similar to the fatload/fatls 
function, but still different. It only traverses the root directory.

The whole fatwrite seems to be a 50% almost verbatim copy of the read 
implementation and shares hardly any code. The problem is the "almost" copy, 
most functions have minor differences.

I think lots of code could be removed from fatwrite if the read implementation 
where better structured, but e.g. the main entry point is a huge function 
which, depending on some flags either prints the directory listing while 
walking/traversing the tree, returns the file size, loads a specified file 
into a buffer, or errors out in case some path element was not reachable.

So, currently there are two options for the bad fatwrite behaviour:
a) add even more duplicate code to fatwrite
b) restructure fatread to be reusable

Opinions, please!

Kind regards,

Stefan

      reply	other threads:[~2016-09-09 16:47 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
2016-09-02 14:53     ` Tom Rini
2016-09-09 16:47       ` Brüns, Stefan [this message]

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=1773564.W4YydK1XsB@sbruens-linux \
    --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