linux-kernel - Re: [PATCH 1/7] staging: r8188eu: use ieee80211 define for version check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20220327181940.6czchqrjhwpoauvi@viti.kaiser.cx>
Date:   Sun, 27 Mar 2022 20:19:40 +0200
From:   Martin Kaiser <martin@...ser.cx>
To:     David Laight <David.Laight@...LAB.COM>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Larry Finger <Larry.Finger@...inger.net>,
        Phillip Potter <phil@...lpotter.co.uk>,
        Michael Straube <straube.linux@...il.com>,
        "linux-staging@...ts.linux.dev" <linux-staging@...ts.linux.dev>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/7] staging: r8188eu: use ieee80211 define for version
 check

Thus wrote David Laight (David.Laight@...LAB.COM):

> From: Martin Kaiser
> > Sent: 23 March 2022 07:49

> > Use the IEEE80211_FCTL_VERS define to check the version number
> > of a received frame.

> > Signed-off-by: Martin Kaiser <martin@...ser.cx>
> > ---
> >  drivers/staging/r8188eu/core/rtw_recv.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)

> > diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
> > index 8800ea4825ff..524a00345501 100644
> > --- a/drivers/staging/r8188eu/core/rtw_recv.c
> > +++ b/drivers/staging/r8188eu/core/rtw_recv.c
> > @@ -1063,7 +1063,6 @@ static int validate_recv_frame(struct adapter *adapter, struct recv_frame *precv
> >  	struct rx_pkt_attrib *pattrib = &precv_frame->attrib;
> >  	u8 *ptr = precv_frame->rx_data;
> >  	__le16 fc = *(__le16 *)ptr;

> Those two lines are somewhat horrid.
> Casts of pointers to integer types have a nasty habit of being bugs.

The fc is the Frame Control field, which is at the start of an incoming
80211 frame. The existing helper functions that parse the Frame Control
want an fc parameter like this.

I looked at the drawing in

https://einstein.informatik.uni-oldenburg.de/rechnernetze/frame.htm

for the structure that the r8188eu driver is trying to parse (the text
is in German, sorry).

> In any case 'ptr' should probably be 'frame_data'.

I'm trying to remove ptr complety and use existing helper functions for
all components.

> If the first two bytes are some kind of 16 bit id, then what follows?
> Should this be a 'struct' that defines the frame data layout??

I define an fc variable in functions that use only components of Frame
Control. If we need other fields, I use a struct ieee80211_hdr.

I've just sent a v2 of this series where I replaced fc with a struct
ieee80211_hdr in the validate_recv_frame function.

Best regards,
Martin