Return-path: Received: from he.sipsolutions.net ([78.46.109.217]:44461 "EHLO sipsolutions.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756471Ab0I3OkP (ORCPT ); Thu, 30 Sep 2010 10:40:15 -0400 Subject: Re: [Patch 1/1] wext: fix 32/64 bit alignment issue for 'point' type From: Johannes Berg To: Gerrit Renker Cc: "John W. Linville" , linux-wireless@vger.kernel.org In-Reply-To: <1285857487.5137.2.camel@jlt3.sipsolutions.net> References: <20100930102508.GB3581@gerrit.erg.abdn.ac.uk> <1285857487.5137.2.camel@jlt3.sipsolutions.net> Content-Type: text/plain; charset="UTF-8" Date: Thu, 30 Sep 2010 16:40:48 +0200 Message-ID: <1285857648.5137.4.camel@jlt3.sipsolutions.net> Mime-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org List-ID: On Thu, 2010-09-30 at 16:38 +0200, Johannes Berg wrote: > On Thu, 2010-09-30 at 12:25 +0200, Gerrit Renker wrote: > > wireless: fix alignment problem > > > > With the current definition, IW_EV_POINT_PK_LEN is 12 on 64-bit and 8 on 32-bit systems, > > due to the way struct iw_event is packed (24 byte on 64-bit systems instead of 20 byte > > on 32-bit systems). Furthermore, the iwe_stream_add_point() in include/net/iw_handler.h > > also uses IW_EV_LCP_PK_LEN as header length. > > > > The current definition appears to be a typo (PK_LEN instead of LEN); it causes > > misalignment on 64 bit systems. > > So, correct me if I'm wrong, the only effect this change has is changing > the second memcpy() in iwe_stream_add_point() to not copy an extra 4 > bytes that will be overwritten right away by the next memcpy() > (presumably, unless the data is < 4, in which case the memcpy might > actually be out of bounds). Interestingly -- yay for wext! -- wireless-tools source code actually uses the define you propose: #define IW_EV_POINT_PK_LEN (IW_EV_LCP_PK_LEN + 4) which probably means that it was fixed there but the fix never propagated to the kernel ... johannes