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.
especially since the scope for trashing the file system is pretty
high.
Maybe I should know, but what happens on < ARMv7+ ? Memory corruption?
If so I'm surprised that extN doesn't fall down all over the place today.
> 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?
I guess the weird char buf[4096] construct was to avoid malloc/free or something.
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.
-Eric
Gordan