On 07/09/2012 03:57 PM, Eric Sandeen wrote:
On 7/9/12 9:45 AM, Gordan Bobic wrote:
On 07/09/2012 03:24 PM, Eric Sandeen wrote:
On 7/8/12 1:07 AM, Jon Masters wrote:
On 07/07/2012 08:42 PM, Gordan Bobic wrote:
Can anyone confirm whether this has been fixed since the bug
was filed? Or whether there is still a risk of trashing the
file system on ARMv5/ARMv6?
https://bugzilla.redhat.com/show_bug.cgi?id=680090
I believe Eric said this was now likely fixed upstream. Have you
asked him to confirm? Let me copy him, and I'm sure he'll say
it's all fine :)
nope, sorry - well, not exactly fine. ;)
We fixed on-disk structure alignment errors for XFS - this is a
different problem, with in-memory alignment...
These memory access problem on arm may well still exist.
It's "only" a warning about alignment though, right - I don't think
this results in any corruption etc, does it (anyway, filefrag is
read-only).
What concerns me is that the buffers are allocated as char[4096] all
over the place in e2fsprogs code files. That is inherently unsafe on
everything except x86 and ARMv7+. I wouldn't want to assume that
filefrag is the only affected program from the e2fsprogs suite,
Running xfstests& e2fsprogs "make check" should expose any others.
Maybe. But if make check missed filefrag issues, so how far can it be
trusted?
especially since the scope for trashing the file system is pretty
high.
Maybe I should know, but what happens on< ARMv7+ ? Memory corruption?
Essentially, yes.
You have an unaligned array being cast into a struct. You then
dereference an element in that struct. Because the array wasn't aligned,
the struct isn't aligned. Thus, the elements in the struct aren't
aligned. So when you dereference a pointer into the struct, it'll get
truncated to the word boundary, then dereferenced. So your data will be
the prefixed with unexpected garbage, and you won't get the tail of the
data.
This issue also affects SPARC and Itanium, IIRC, likely a lot of others,
too. It's only x86 and ARMv7+ that have transparent automatic alignment
fixup in hardware. Note that even on those there may be a performance
penalty, and there is always a performance penalty for straddling cache
lines (which is much more likely when you're not paying attention to
alignment - one of the reasons why ICC has an option to align all data
to a 16-byte boundary, just to make sure).
If so I'm surprised that extN doesn't fall down all over the place today.
Me too. :)
But it is possible that fsck doesn't get run on older ARMs often.
And as the comments in this bug indicate, I'm really not quite sure
what the accepted method of fixing is.
The options I can see are:
1) Detect sizeof(int) and declare the arrays as a suitable multiple
of that. int _should_ be word sized on most things AFAIK.
now I'll sound really dense, but isn't 4096 a multiple of sizeof(int)
on arm?
Sure, it is, but arrays of type foo are aligned to a size increment of
foo. So the beginning of an array of char will be byte aligned since
char is 1 byte.
I guess the weird char buf[4096] construct was to avoid malloc/free or something.
I wouldn't know, I didn't write it. :)
2) Check the required alignment for the arch at build time, and apply
it with "__attribute__ aligned" in every relevant instance. There's a
LOT of those, but that's what you get for using non-portable
techniques. :(
Yeah, that's nasty. On xfs we did some #ifdef macros to add __attributes__
to a few on-disk structures for arm old ABI, but that was fairly well-
contained.
3) Add a new flag to gcc to align all arrays to a particular
boundary. Unlikely to happen. FWIW, ICC has such an option, IIRC as
it helps with performance, too, if things aren't straddling cache
lines.
I imagine there are several applications that exhibit similar
behavior?
Not many, if you are referring to other packages. I filed bugs for
most of them, but I filed against the available ARM releases (since
rawhide wasn't up to running on ARM), so they all got closed without
resolution pretty quickly. I gave up on bothering to file bugs after
that.
Sorry about that. Obviously it's a higher priority now.
Yeah, it's understandable, but it was highly demotivating. Eventually I
ended up causing confusion by filing against rawhide even though it
wasn't quite applicable to ARM (well, "rawhide" for ARM was a few
releases behind, at least :) just to avoid the bugzapper. There didn't
seem to be a less wrong solution at the time.
Gordan
_______________________________________________
arm mailing list
arm@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/arm
