[PATCH for stable/back-port] staging: vt6656: fix headers - key.c only

Greg Kroah-Hartman gregkh at linuxfoundation.org
Thu Jan 17 21:07:21 UTC 2013


On Thu, Jan 17, 2013 at 01:06:52PM -0800, Greg Kroah-Hartman wrote:
> On Sun, Jan 13, 2013 at 08:50:43PM +0000, Malcolm Priestley wrote:
> > On Sun, 2013-01-13 at 15:26 +0000, Ben Hutchings wrote:
> > > On Sun, 2012-12-30 at 15:01 +0000, Malcolm Priestley wrote:
> > > > On Sun, 2012-12-30 at 00:50 +0100, Ben Hutchings wrote:
> > > > > On Sat, 2012-12-29 at 13:10 +0000, Malcolm Priestley wrote:
> > > > > > shorted back-ported version of upstream commit
> > > > > > 11d404cb56ecd53bb23499897fbe7be1a9ac4827
> > > > > > staging: vt6656: fix headers and add cfg80211.
> > > > > > key.c only
> > > > > > 
> > > > > > This patch fixes the deadlock of 64 bit systems
> > > > > > on successful association.
> > > > > > 
> > > > > > In key.h void pointer pvKeyTable in SKeyItem is out of alignment
> > > > > > on 64 bit kernel.
> > > > > > 
> > > > > > The upstream arrangement of headers fixes this.
> > > > > [...]
> > > > > 
> > > > > Please explain how.  I don't see anything weird about key.h and mac.h
> > > > > that would cause structure definitions to be interpreted differently
> > > > > depending on inclusion order.
> > > > 
> > > > 
> > > > The difference is that key.h is no longer defined in key.c.
> > > >
> > > > It is declared in path;
> > > > mac.h
> > > > ----->device.h
> > > > -------------->key.h
> > > > 
> > > > The structure is now as seen by rxtx.c and dpc.c.
> > > > 
> > > > Before the patch, key.c has it's own localised version
> > > > declared in key.h and for some reason(?) is not packed the same.
> > > [...]
> > > 
> > > Here are the key structure definitions and the layout they should have
> > > depending on word size and native/packed alignment:
> > > 
> > >                                         32n 32p 64n 64p
> > > typedef struct tagSKeyItem
> > > {
> > >     BOOL        bKeyValid;		  0   0   0   0
> > > 	u32 uKeyLength;			  4   4   4   4
> > >     BYTE        abyKey[MAX_KEY_LEN];	  8   8   8   8
> > >     QWORD       KeyRSC;			 40  40  40  40
> > >     DWORD       dwTSC47_16;		 48  48  48  48
> > >     WORD        wTSC15_0;		 52  52  52  52
> > >     BYTE        byCipherSuite;		 54  54  54  54
> > >     BYTE        byReserved0;		 55  55  55  55
> > >     DWORD       dwKeyIndex;		 56  56  56  56
> > >     void *pvKeyTable;			 60  60  64  60
> > > } SKeyItem, *PSKeyItem; //64		 64  64  72  68
> > > 
> > > typedef struct tagSKeyTable
> > > {
> > >     BYTE        abyBSSID[ETH_ALEN];	  0   0   0   0
> > >     BYTE        byReserved0[2];           6   6   6   6
> > >     SKeyItem    PairwiseKey;		  8   8   8   8
> > >     SKeyItem    GroupKey[MAX_GROUP_KEY]; 72  72  80  76
> > >     DWORD       dwGTKeyIndex;           328 328 368 348
> > >     BOOL        bInUse;			332 332 372 352
> > >     WORD        wKeyCtl;		336 336 376 356
> > >     BOOL        bSoftWEP;		340 338 380 358
> > >     BYTE        byReserved1[6];		344 342 384 362
> > > } SKeyTable, *PSKeyTable; //352		352 348 392 368
> > > 
> > > This agrees with your observed offsets of bSoftWEP, if SKeyItem and
> > > SKeyTable are defined packed or not depending on the order of inclusion.
> > > 
> > > The only thing that should cause alignment to change in this way is the
> > > abominable '#pragma pack', and sure enough several of the headers have
> > > '#pragma pack(1)' but no '#pragma pack()' to reset this.
> > > 
> > > Does the following work for you?  (It's based on 3.7; the context is
> > > slightly different for earlier versions.)
> > > 
> > > ---
> > > Subject: vt6656: Fix inconsistent structure packing
> > > 
> > > vt6656 has several headers that use the #pragma pack(1) directive to
> > > enable structure packing, but never disable it.  The layout of
> > > structures defined in other headers can then depend on which order the
> > > various headers are included in, breaking the One Definition Rule.
> > > 
> > > This removes those directives and adds __packed to structure
> > > definitions where packing appears to have been intended.
> > > 
> > > Reported-by: Malcolm Priestley <tvboxspy at gmail.com>
> > > Signed-off-by: Ben Hutchings <ben at decadent.org.uk>
> > 
> > Thanks Ben
> > 
> > I have tested the patch on 3.7 and 3.8-rc2 and it has resolved the
> > issue.
> 
> Great, can someone send me the patch so that I can apply it upstream?

Oh nevermind, you did on the 14th, I have it in my inbox...

greg k-h




More information about the kernel-team mailing list