Dave Taht <[email protected]> writes: > On Mon, Jul 2, 2018 at 12:23 PM Toke Høiland-Jørgensen <[email protected]> wrote: >> >> Kevin Darbyshire-Bryant <[email protected]> writes: >> >> >> On 2 Jul 2018, at 19:39, Dave Taht <[email protected]> wrote: >> >> >> >>> >> >> >> >> This seems like it will introduce problems with stuff that isn't or is >> >> legitimately broken in the first place, pointing to potentially random >> >> data in the wrong place. >> >> >> >> would a workaround be adding more padding to the cake stats output so >> >> it's always even? >> >> >> >> why does it work as written on arm? >> > >> > If I understand correctly: This will only be a problem on >> > architectures that require alignment of 64 bit values to 8 byte >> > boundaries which is achieved by padding the structure by a dummy (4 >> > byte) value if required. So to hit this bug we need kernel symbol >> > CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS undefined *and* we need a >> > netlink stats structure that needs a 4 byte dummy pad value to align >> > to 8 bytes. Of the architectures tested, MIPS is the only one that >> > DOES NOT set CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS and thus may be >> > exposed to the bug. >> > >> > arm sets CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS and thus no padding is >> > ever required/added, thus pointers always point to the correct data >> > location. >> >> Yup, exactly. This has always been broken on MIPS, I assume, but because >> most other qdiscs send their stats output as a serialised struct, tc >> just automatically falls back to the legacy data format, and no one has >> noticed. But because we switched to sending each stat as an individual >> netlink attribute (and thus no fallback legacy stats struct), we expose >> the bug... > > Well, if you wrap that patch in > > #ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS > bla > #endif > > I guess I'd sleep better but I do generally get nervous when > arbitrarily subtracting something > from a pointer
Ah, right; well, that was just a proof of concept to make sure it fixed the issue. I'll look harder at the code to see if I can find a better solution before submitting it upstream (at a first glance the API doesn't make it easy, but I'll have another look). -Toke _______________________________________________ Cake mailing list [email protected] https://lists.bufferbloat.net/listinfo/cake
