| Message ID | 1419346112-173303-1-git-send-email-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Kalle Valo |
| Headers | show |
On 12/23/14 15:48, Mika Westerberg wrote: > The driver looks for pdata->oob_irq_supported to find out if wowl can be > supported. However, not all platforms populate pdata in which case we crash > the kernel because of NULL pointer dereference. Thanks, Mika However, this was already reported by Dan Carpenter and I submitted a patch for that a couple of days ago: "[PATCH 02/10] brcmfmac: Fix possible dereference of NULL pointer." [1]. Regards, Arend [1] http://mid.gmane.org/1419162233-19492-3-git-send-email-arend@broadcom.com > Fixes: 330b4e4be937 ("brcmfmac: Add wowl support for SDIO devices.") > Reported-by: Christophe Prigent<christophe.prigent@intel.com> > Signed-off-by: Mika Westerberg<mika.westerberg@linux.intel.com> > --- > drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c > index 3c06e9365949..9880dae2a569 100644 > --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c > +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c > @@ -1070,7 +1070,7 @@ static int brcmf_ops_sdio_probe(struct sdio_func *func, > */ > if ((sdio_get_host_pm_caps(sdiodev->func[1])& MMC_PM_KEEP_POWER)&& > ((sdio_get_host_pm_caps(sdiodev->func[1])& MMC_PM_WAKE_SDIO_IRQ) || > - (sdiodev->pdata->oob_irq_supported))) > + (sdiodev->pdata&& sdiodev->pdata->oob_irq_supported))) > bus_if->wowl_supported = true; > #endif > > @@ -1167,7 +1167,7 @@ static int brcmf_ops_sdio_resume(struct device *dev) > struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio; > > brcmf_dbg(SDIO, "Enter\n"); > - if (sdiodev->pdata->oob_irq_supported) > + if (sdiodev->pdata&& sdiodev->pdata->oob_irq_supported) > disable_irq_wake(sdiodev->pdata->oob_irq_nr); > brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS); > atomic_set(&sdiodev->suspend, false); -- 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 Tue, Dec 23, 2014 at 04:37:21PM +0100, Arend van Spriel wrote: > On 12/23/14 15:48, Mika Westerberg wrote: > >The driver looks for pdata->oob_irq_supported to find out if wowl can be > >supported. However, not all platforms populate pdata in which case we crash > >the kernel because of NULL pointer dereference. > > Thanks, Mika > > However, this was already reported by Dan Carpenter and I submitted a patch > for that a couple of days ago: "[PATCH 02/10] brcmfmac: Fix possible > dereference of NULL pointer." [1]. > > Regards, > Arend > > [1] > http://mid.gmane.org/1419162233-19492-3-git-send-email-arend@broadcom.com Oh, good. I didn't notice that one. Thanks for fixing it :) BTW, that patch seems to miss brcmf_ops_sdio_resume(), perhaps it is fixed in another patch? -- 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 12/23/14 16:47, Mika Westerberg wrote: > On Tue, Dec 23, 2014 at 04:37:21PM +0100, Arend van Spriel wrote: >> On 12/23/14 15:48, Mika Westerberg wrote: >>> The driver looks for pdata->oob_irq_supported to find out if wowl can be >>> supported. However, not all platforms populate pdata in which case we crash >>> the kernel because of NULL pointer dereference. >> >> Thanks, Mika >> >> However, this was already reported by Dan Carpenter and I submitted a patch >> for that a couple of days ago: "[PATCH 02/10] brcmfmac: Fix possible >> dereference of NULL pointer." [1]. >> >> Regards, >> Arend >> >> [1] >> http://mid.gmane.org/1419162233-19492-3-git-send-email-arend@broadcom.com > > Oh, good. I didn't notice that one. > > Thanks for fixing it :) > > BTW, that patch seems to miss brcmf_ops_sdio_resume(), perhaps it is > fixed in another patch? Aargh, a nice "BTW" this is as there is no other patch for the resume code. Maybe I should ask Kalle to apply your patch instead of ours. If it did not already got applied. 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
Arend van Spriel <arend@broadcom.com> writes: > On 12/23/14 16:47, Mika Westerberg wrote: >> On Tue, Dec 23, 2014 at 04:37:21PM +0100, Arend van Spriel wrote: >>> On 12/23/14 15:48, Mika Westerberg wrote: >>>> The driver looks for pdata->oob_irq_supported to find out if wowl can be >>>> supported. However, not all platforms populate pdata in which case we crash >>>> the kernel because of NULL pointer dereference. >>> >>> Thanks, Mika >>> >>> However, this was already reported by Dan Carpenter and I submitted a patch >>> for that a couple of days ago: "[PATCH 02/10] brcmfmac: Fix possible >>> dereference of NULL pointer." [1]. >>> >>> Regards, >>> Arend >>> >>> [1] >>> http://mid.gmane.org/1419162233-19492-3-git-send-email-arend@broadcom.com >> >> Oh, good. I didn't notice that one. >> >> Thanks for fixing it :) >> >> BTW, that patch seems to miss brcmf_ops_sdio_resume(), perhaps it is >> fixed in another patch? > > Aargh, a nice "BTW" this is as there is no other patch for the resume > code. Maybe I should ask Kalle to apply your patch instead of ours. If > it did not already got applied. I have been waiting for net-next to open so I haven't applied any -next patches yet. So if you want I can take Mika's patch as well. Just send a reply to your patch I should drop. And should this patch go to 3.19?
On 12/23/14 18:00, Kalle Valo wrote: > Arend van Spriel<arend@broadcom.com> writes: > >> On 12/23/14 16:47, Mika Westerberg wrote: >>> On Tue, Dec 23, 2014 at 04:37:21PM +0100, Arend van Spriel wrote: >>>> On 12/23/14 15:48, Mika Westerberg wrote: >>>>> The driver looks for pdata->oob_irq_supported to find out if wowl can be >>>>> supported. However, not all platforms populate pdata in which case we crash >>>>> the kernel because of NULL pointer dereference. >>>> >>>> Thanks, Mika >>>> >>>> However, this was already reported by Dan Carpenter and I submitted a patch >>>> for that a couple of days ago: "[PATCH 02/10] brcmfmac: Fix possible >>>> dereference of NULL pointer." [1]. >>>> >>>> Regards, >>>> Arend >>>> >>>> [1] >>>> http://mid.gmane.org/1419162233-19492-3-git-send-email-arend@broadcom.com >>> >>> Oh, good. I didn't notice that one. >>> >>> Thanks for fixing it :) >>> >>> BTW, that patch seems to miss brcmf_ops_sdio_resume(), perhaps it is >>> fixed in another patch? >> >> Aargh, a nice "BTW" this is as there is no other patch for the resume >> code. Maybe I should ask Kalle to apply your patch instead of ours. If >> it did not already got applied. > > I have been waiting for net-next to open so I haven't applied any -next > patches yet. So if you want I can take Mika's patch as well. Just send a > reply to your patch I should drop. > > And should this patch go to 3.19? Yeah, the issue was introduced in 3.19 so that makes sense. 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
Mika Westerberg <mika.westerberg@linux.intel.com> writes: > The driver looks for pdata->oob_irq_supported to find out if wowl can be > supported. However, not all platforms populate pdata in which case we crash > the kernel because of NULL pointer dereference. > > Fixes: 330b4e4be937 ("brcmfmac: Add wowl support for SDIO devices.") > Reported-by: Christophe Prigent <christophe.prigent@intel.com> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> Thanks, applied to wireless-drivers.git.
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c index 3c06e9365949..9880dae2a569 100644 --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c @@ -1070,7 +1070,7 @@ static int brcmf_ops_sdio_probe(struct sdio_func *func, */ if ((sdio_get_host_pm_caps(sdiodev->func[1]) & MMC_PM_KEEP_POWER) && ((sdio_get_host_pm_caps(sdiodev->func[1]) & MMC_PM_WAKE_SDIO_IRQ) || - (sdiodev->pdata->oob_irq_supported))) + (sdiodev->pdata && sdiodev->pdata->oob_irq_supported))) bus_if->wowl_supported = true; #endif @@ -1167,7 +1167,7 @@ static int brcmf_ops_sdio_resume(struct device *dev) struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio; brcmf_dbg(SDIO, "Enter\n"); - if (sdiodev->pdata->oob_irq_supported) + if (sdiodev->pdata && sdiodev->pdata->oob_irq_supported) disable_irq_wake(sdiodev->pdata->oob_irq_nr); brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS); atomic_set(&sdiodev->suspend, false);
The driver looks for pdata->oob_irq_supported to find out if wowl can be supported. However, not all platforms populate pdata in which case we crash the kernel because of NULL pointer dereference. Fixes: 330b4e4be937 ("brcmfmac: Add wowl support for SDIO devices.") Reported-by: Christophe Prigent <christophe.prigent@intel.com> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)