On Fri, 2012-08-24 at 11:58 -0500, Larry Finger wrote: > On 08/24/2012 11:23 AM, Eric Dumazet wrote: > > On Fri, 2012-08-24 at 18:18 +0200, Eric Dumazet wrote: > >> On Fri, 2012-08-24 at 10:58 -0500, Larry Finger wrote: > >>> On 08/24/2012 10:19 AM, David Miller wrote: > >>>> > >>>> This looks like full-on data corruption to me. > >>> > >>> I agree. The question is why does it happen with r8712u, and only after the > >>> commit in the subject. Drivers for other devices that I have are OK. Thus far, I > >>> have tested b43, rtl8187, ath9k_htc, and rtl8192cu. To my knowledge, there are > >>> no reports posted for this bug with any other device. > >> > >> bugs can sit unnoticed, and one change somewhere can uncover them. > >> > >> Really this driver must have a bug, if not half a dozen of bugs. > >> > >> For example this sequence of code is a clear bug : > >> > >> sub_skb = dev_alloc_skb(nSubframe_Length + 12); > >> skb_reserve(sub_skb, 12); > >> > >> > >> Also the free_recv_skb_queue looks really suspect to me > >> > >> What the hell is doing recv_tasklet() I really wonder. > >> > >> This code, combined with the skb_clone() in recvbuf2recvframe() > >> can clearly reuse an skb passed to upper stacks. > >> > >> > >> queueing one skb in free_recv_skb_queue should be done > >> only if no clone of this skb exist somewhere. > >> > >> Please someone fix this buggy driver. > >> > > > > Try the following patch for a start > > > > diff --git a/drivers/staging/rtl8712/rtl8712_recv.c b/drivers/staging/rtl8712/rtl8712_recv.c > > index 8e82ce2..88e3ca6 100644 > > --- a/drivers/staging/rtl8712/rtl8712_recv.c > > +++ b/drivers/staging/rtl8712/rtl8712_recv.c > > @@ -1127,6 +1127,9 @@ static void recv_tasklet(void *priv) > > recvbuf2recvframe(padapter, pskb); > > skb_reset_tail_pointer(pskb); > > pskb->len = 0; > > - skb_queue_tail(&precvpriv->free_recv_skb_queue, pskb); > > + if (!skb_cloned(pskb)) > > + skb_queue_tail(&precvpriv->free_recv_skb_queue, pskb); > > + else > > + consume_skb(pskb); > > } > > } > > This one did not help. There is no doubt it is needed for the case where memory > is tight, an allocation fails, and the driver clones the skb. In the present > case, debug statements have shown that the skb_clone() call was not made. > > In the long term, this driver will be replaced with one that uses mac80211, but > in the short term, I am trying to fix it. > > As I said earlier, my skb skills are minimal. Could you explain what is wrong > with the following sequence? > > sub_skb = dev_alloc_skb(nSubframe_Length + 12); > skb_reserve(sub_skb, 12); dev_alloc_skb() can return NULL -> crash skb_clone() can also return NULL -> crash -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
