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 _______________________________________________ arm mailing list arm@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/arm
