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 <[email protected]>
> > Signed-off-by: Ben Hutchings <[email protected]>
>
> 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?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
