Fwd: drivers/net/wireless/libertas/rx.c: use-after-free

[Dan Williams]

On Tue, 2007-07-03 at 15:56 -0400, Marcelo Tosatti wrote:
> Hi Holger,
> 
> That line sneaked somehow with the patch - it was part of the original
> driver.

Committed, thanks.

Dan

> process_rxed_802_11_packet is never called at the moment, but we might
> want to support monitor mode properly (there is a firmware comming out
> with it soon), and setting ETH_P_80211_RAW seems required for that.
> 
> So for now I would do this instead
> 
> diff --git a/drivers/net/wireless/libertas/rx.c b/drivers/net/wireless/libertas/rx.c
> index a84e100..1ec54e1 100644
> --- a/drivers/net/wireless/libertas/rx.c
> +++ b/drivers/net/wireless/libertas/rx.c
> @@ -432,13 +432,13 @@ static int process_rxed_802_11_packet(wl
>  	lbs_deb_rx("rx data: size of actual packet %d\n", skb->len);
>  	priv->stats.rx_bytes += skb->len;
>  	priv->stats.rx_packets++;
> +	skb->protocol = __constant_htons(0x0019);	/* ETH_P_80211_RAW */
>  
>  	libertas_upload_rx_packet(priv, skb);
>  
>  	ret = 0;
>  
>  done:
> -	skb->protocol = __constant_htons(0x0019);	/* ETH_P_80211_RAW */
>  	lbs_deb_leave_args(LBS_DEB_RX, "ret %d", ret);
>  	return ret;
>  }
> 
> 
> 
> On Sat, Jun 30, 2007 at 09:14:53PM +0200, Holger Schurig wrote:
> > Hi Marcello,
> > 
> > see the attached message from Adrian bunk.
> > 
> > I've done a
> > 
> >    git-show 9012b28a407511fb355f6d2176a12d4653489672
> > 
> > and indeed I'm seeing the line
> > 
> >    skb->protocol = __constant_htons(0x0019);       /* ETH_P_80211_RAW */
> > 
> > You did the commit into the libertas-git tree for me.
> > 
> > However, I don't have the slightest clue how this line
> > made it into this patch. I suggested this patch in on
> > 23rd February with the subject "patch to taylor debug
> > output", but the link to the git tree where the patch
> > was in is gone.
> > 
> > However, the above git-show did show a date of May, 25th.
> > How comes?  My patch was from February?!?!
> > 
> > And I'm quite sure that I didn't do anything in my
> > patch with skb->protocol. I hardly know anything about
> > skb's, just learnt recently for my if_cs.c file, so I
> > cannot imagine that I ever set any protocol field while
> > not knowing what's going on there. Did this line slip
> > in from your own code?
> > 
> > In rx.c before commit 9012b28a407511fb355f6d2176a12d4653489672,
> > there is no mentioning of skb->protocol at all. Do you
> > think the right fix is to simply remove this line?
> 
> Content-Description: Adrian Bunk <bunk at stusta.de>: drivers/net/wireless/libertas/rx.c: use-after-free
> > From: Adrian Bunk <bunk at stusta.de>
> > Date: Fri, 29 Jun 2007 21:51:16 +0200
> > To: linville at tuxdriver.com,
> >  Holger Schurig <hs4233 at mail.mn-solutions.de>
> > Cc: linux-wireless at vger.kernel.org,
> >  linux-kernel at vger.kernel.org
> > Subject: drivers/net/wireless/libertas/rx.c: use-after-free
> > X-Length: 2861
> > 
> > The Coverity checker spotted the following use-after-free of "skb" in 
> > drivers/net/wireless/libertas/rx.c introduced by
> > commit 9012b28a407511fb355f6d2176a12d4653489672 (WTF did this commit
> > with the title "libertas: make debug configurable" add the 
> > "skb->protocol = __constant_htons(0x0019);" line?):
> > 
> > <--  snip  -->
> > 
> > ...
> > static int process_rxed_802_11_packet(wlan_private * priv, struct sk_buff *skb)
> > {
> > ...
> >         libertas_upload_rx_packet(priv, skb);
> > 
> >         ret = 0;
> > 
> > done:
> >         skb->protocol = __constant_htons(0x0019);       /* ETH_P_80211_RAW */
> > ...
> > 
> > <--  snip  -->
> > 
> > 
> > cu
> > Adrian
> > 
> > -- 
> > 
> >        "Is there not promise of rain?" Ling Tan asked suddenly out
> >         of the darkness. There had been need of rain for many days.
> >        "Only a promise," Lao Er said.
> >                                        Pearl S. Buck - Dragon Seed
> > 
> > 
> > 
> 
> 
> _______________________________________________
> libertas-dev mailing list
> libertas-dev at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/libertas-dev