linux-ext4 - buggy readdir with inline dirs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130322182608.GT10038@lenny.home.zabbo.net>
Date:	Fri, 22 Mar 2013 11:26:08 -0700
From:	Zach Brown <zab@...hat.com>
To:	linux-ext4@...r.kernel.org
Cc:	Tao Ma <boyu.mt@...bao.com>, Eric Sandeen <sandeen@...hat.com>
Subject: buggy readdir with inline dirs

I don't remember quite how, but I found myself flipping through the
inline dir code that's in mainline now.  It looked pretty fishy so Eric
and I played around with it.  It's very buggy in its current form.

ext4_read_inline_dir() doesn't seem to undertand the filldir arguments.
It suggests that offset 0 is the next offset after both the "." and ".."
entries.  It needs to have specific offsets for "." and ".." and return them
accordingly.  It looks like fixing this will trickle down into the
revalidation loop.

It doesn't understand that it's possible to only return a single "."
entry in getdents and have a subsequent call have f_pos pointing at the
fake ".." entry.  With the current code if your getdents buffer only has
room for "." it just spins returning that entry leaving f_pos at 0.

Those are all relatively simple bugs that just need to be fixed.

But the big bug is that it changes the d_off values for entries as it
converts from byte offsets in the inline dir xattr to hashed offsets in
indexed dir leaves.  A concurrent readdir could be unlucky enough to get
a bunch of duplicate entries as it reads past the nice low inline byte
offsets into the huge hashed offsets.

I'm not sure how to easily fix that.  It feels like it'd want to
maintain the dir entries in the xattr blob with the offsets that they'll
have once converted to full dir blocks.  So instead of being a magical
readdir path maybe it wants to be in the path of looking up dir blocks
so existing unindexed and indexed code would operate on the data in the
xattr blob as though it were a block?

Dunno, just wanted to share what we found.  Are these all known problems
in prototype code that isn't intended to be used?

- z
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html