| Message ID | 55DB26FC.1060707@broadcom.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kalle Valo |
| Headers | show |
On 24 August 2015 at 16:15, Arend van Spriel <arend@broadcom.com> wrote: > On 08/19/2015 11:56 PM, Rafa? Mi?ecki wrote: >> >> On 16 August 2015 at 08:55, Arend van Spriel <arend@broadcom.com> wrote: >>> >>> From: Hante Meuleman <meuleman@broadcom.com> >>> >>> Next generation devices will have firmware which will have more >>> than 256 flowrings. This patch increases the maximum number of >>> supported flowrings to 512. >>> >>> Reviewed-by: Arend Van Spriel <arend@broadcom.com> >>> Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com> >>> Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com> >>> Signed-off-by: Hante Meuleman <meuleman@broadcom.com> >>> Signed-off-by: Arend van Spriel <arend@broadcom.com> >> >> >> This is a patch that for my device triggers: >> Unable to handle kernel NULL pointer dereference at virtual address >> 00000014 >> >> It seems to happen even when using only 1 AP interface. > > > Hi Rafa?, > > What happens is that your platform has no memory to allocate a flowring for > tx and there is a bug in handling the returned error. Can you try it with > following applied. You will still have memory issue so for OpenWrt you may > need to limit to 256 anyway. Hante found another issue with hash_id in > struct brcmf_flowring so we are not entirely sure which issue caused the > null pointer deref so your feedback is appreciated. > > Regards, > Arend > --- > drivers/net/wireless/brcm80211/brcmfmac/flowring.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/brcm80211/brcmfmac/flowring.c > b/drivers/net/wi > index 5b7a8ee..9fb50b3 100644 > --- a/drivers/net/wireless/brcm80211/brcmfmac/flowring.c > +++ b/drivers/net/wireless/brcm80211/brcmfmac/flowring.c > @@ -151,11 +151,11 @@ u32 brcmf_flowring_create(struct brcmf_flowring *flow, > u8 > break; > } > if (i == flow->nrofrings) > - return -ENOMEM; > + return BRCMF_FLOWRING_INVALID_ID; > > ring = kzalloc(sizeof(*ring), GFP_ATOMIC); > if (!ring) > - return -ENOMEM; > + return BRCMF_FLOWRING_INVALID_ID; > > memcpy(hash[hash_idx].mac, mac, ETH_ALEN); > hash[hash_idx].fifo = fifo; Unfortunately this patch didn't help. I tied it on top of V3 of your last series plus "brcmfmac: Increase nr of supported flowrings.". I tried it 3 times, once I even got early NULL pointer dereference, when starting AP interface (before any STA connecting to it).
On 08/24/2015 10:02 PM, Rafa? Mi?ecki wrote: > On 24 August 2015 at 16:15, Arend van Spriel <arend@broadcom.com> wrote: >> On 08/19/2015 11:56 PM, Rafa? Mi?ecki wrote: >>> >>> On 16 August 2015 at 08:55, Arend van Spriel <arend@broadcom.com> wrote: >>>> >>>> From: Hante Meuleman <meuleman@broadcom.com> >>>> >>>> Next generation devices will have firmware which will have more >>>> than 256 flowrings. This patch increases the maximum number of >>>> supported flowrings to 512. >>>> >>>> Reviewed-by: Arend Van Spriel <arend@broadcom.com> >>>> Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com> >>>> Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com> >>>> Signed-off-by: Hante Meuleman <meuleman@broadcom.com> >>>> Signed-off-by: Arend van Spriel <arend@broadcom.com> >>> >>> >>> This is a patch that for my device triggers: >>> Unable to handle kernel NULL pointer dereference at virtual address >>> 00000014 >>> >>> It seems to happen even when using only 1 AP interface. >> >> >> Hi Rafa?, >> >> What happens is that your platform has no memory to allocate a flowring for >> tx and there is a bug in handling the returned error. Can you try it with >> following applied. You will still have memory issue so for OpenWrt you may >> need to limit to 256 anyway. Hante found another issue with hash_id in >> struct brcmf_flowring so we are not entirely sure which issue caused the >> null pointer deref so your feedback is appreciated. >> >> Regards, >> Arend >> --- >> drivers/net/wireless/brcm80211/brcmfmac/flowring.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/flowring.c >> b/drivers/net/wi >> index 5b7a8ee..9fb50b3 100644 >> --- a/drivers/net/wireless/brcm80211/brcmfmac/flowring.c >> +++ b/drivers/net/wireless/brcm80211/brcmfmac/flowring.c >> @@ -151,11 +151,11 @@ u32 brcmf_flowring_create(struct brcmf_flowring *flow, >> u8 >> break; >> } >> if (i == flow->nrofrings) >> - return -ENOMEM; >> + return BRCMF_FLOWRING_INVALID_ID; >> >> ring = kzalloc(sizeof(*ring), GFP_ATOMIC); >> if (!ring) >> - return -ENOMEM; >> + return BRCMF_FLOWRING_INVALID_ID; >> >> memcpy(hash[hash_idx].mac, mac, ETH_ALEN); >> hash[hash_idx].fifo = fifo; > > Unfortunately this patch didn't help. I tied it on top of V3 of your > last series plus "brcmfmac: Increase nr of supported flowrings.". First about the warnings in core.c:1144. I believe they are fixed by one of the patches submitted last week [1]. Now about the problem upon start_xmit. The theory was that you had allocation failure leading to the null pointer deref. If you don't see any allocation failure on your setup that theory seems invalid. So there is another root cause that we need to find. > I tried it 3 times, once I even got early NULL pointer dereference, > when starting AP interface (before any STA connecting to it). This seems like a different issue. Thanks for reporting. We will look into this. Regards, Arend [1] https://patchwork.kernel.org/patch/7079891/ -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 29 August 2015 at 10:13, Arend van Spriel <arend@broadcom.com> wrote: > First about the warnings in core.c:1144. I believe they are fixed by one of > the patches submitted last week [1]. This is correct :) > Now about the problem upon start_xmit. The theory was that you had > allocation failure leading to the null pointer deref. If you don't see any > allocation failure on your setup that theory seems invalid. So there is > another root cause that we need to find. Let me know if you have any patches (fixing or debugging) you want me to test. I'll be happy to help.
On 08/29/2015 11:38 AM, Rafa? Mi?ecki wrote: > On 29 August 2015 at 10:13, Arend van Spriel <arend@broadcom.com> wrote: >> First about the warnings in core.c:1144. I believe they are fixed by one of >> the patches submitted last week [1]. > > This is correct :) > > >> Now about the problem upon start_xmit. The theory was that you had >> allocation failure leading to the null pointer deref. If you don't see any >> allocation failure on your setup that theory seems invalid. So there is >> another root cause that we need to find. > > Let me know if you have any patches (fixing or debugging) you want me > to test. I'll be happy to help. So can you indicate whether you have seen allocation failures or not. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/flowring.c b/drivers/net/wi index 5b7a8ee..9fb50b3 100644 --- a/drivers/net/wireless/brcm80211/brcmfmac/flowring.c +++ b/drivers/net/wireless/brcm80211/brcmfmac/flowring.c @@ -151,11 +151,11 @@ u32 brcmf_flowring_create(struct brcmf_flowring *flow, u8 break; } if (i == flow->nrofrings) - return -ENOMEM; + return BRCMF_FLOWRING_INVALID_ID; ring = kzalloc(sizeof(*ring), GFP_ATOMIC); if (!ring) - return -ENOMEM; + return BRCMF_FLOWRING_INVALID_ID; memcpy(hash[hash_idx].mac, mac, ETH_ALEN);