|
|
|
Re: e2fsprogs alignment | |
| [Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] | |
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=680090I 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
