From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.toke.dk (mail.toke.dk [52.28.52.200]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id 7F3C03BA8E for ; Mon, 2 Jul 2018 15:38:12 -0400 (EDT) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=toke.dk; s=20161023; t=1530560291; bh=lBghliB87c4IjwYVBWQbg68hpiI38oXdE446cvyPHVo=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=apIjqVitlMdhhkQh846FmneNIK6s56UU9CgmVqJ+41/pOtod3ENMjJQdAGL+xQG93 MEYAu579XjWjjlgYESj9wkcn77tjDRmYftMyOvbrbeu4YCgFslOqqudxAmPVCJ4x0D bjEnHn1wvXjHInEC7dkzlerX3ZxMW0W1JWmVVoSCVxGqxRN95HM+e4NhLZelAXKFwG eyV/nAtqkZqtDevnlvCkk0vK0hOglb1PQjT1o8NsPPL1zsk8BBMkavarDiwZI6n424 TXy/21/SaS3CJ1/7NUcJXBue4+WxnlVz1zLoaQl7PCDdycwUw/e+z5jp7QtHNdAdMz nlaVEFaM31S0A== To: Dave Taht Cc: Kevin Darbyshire-Bryant , Cake List In-Reply-To: References: <1A8BA286-6B31-4581-86C9-6855AC28C245@heistp.net> <673EAD3F-AB09-4B90-88BB-5DCE0BD65534@heistp.net> <6FE8D434-01BE-41A1-BD6B-EFFD67AC8784@heistp.net> <94C9790F-E9BC-4D59-9845-17C305E4B910@darbyshire-bryant.me.uk> <17AF79A0-0213-44E3-95B9-62795A644A47@heistp.net> <87lgatj13k.fsf@toke.dk> <87fu11ipir.fsf@toke.dk> <9F0353E5-D191-492A-9413-AA6BD03DD4C6@darbyshire-bryant.me.uk> <877emdigqe.fsf@toke.dk> Date: Mon, 02 Jul 2018 21:38:21 +0200 X-Clacks-Overhead: GNU Terry Pratchett Message-ID: <87tvphh1hu.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Cake] Cake on openwrt - falling behind X-BeenThere: cake@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Cake - FQ_codel the next generation List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Jul 2018 19:38:12 -0000 Dave Taht writes: > On Mon, Jul 2, 2018 at 12:23 PM Toke H=C3=B8iland-J=C3=B8rgensen wrote: >> >> Kevin Darbyshire-Bryant writes: >> >> >> On 2 Jul 2018, at 19:39, Dave Taht 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