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
prev parent 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