| Message ID | 20190523071534.254611-1-tientzu@chromium.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4b553f3ca4cbde67399aa3a756c37eb92145b8a1 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath10k: add missing error handling | expand |
On Thu, May 23, 2019 at 12:15 AM Claire Chang <tientzu@chromium.org> wrote: > --- a/drivers/net/wireless/ath/ath10k/sdio.c > +++ b/drivers/net/wireless/ath/ath10k/sdio.c > @@ -607,6 +607,10 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar, > full_len, > last_in_bundle, > last_in_bundle); > + if (ret) { IIUC, you have basically the same failure case a few lines up, where ath10k_sdio_mbox_alloc_pkt_bundle() may fail. Do the same there? This (including the error label to which it's jumping) looks fine to me though: Reviewed-by: Brian Norris <briannorris@chromium.org> > + ath10k_warn(ar, "alloc_rx_pkt error %d\n", ret); > + goto err; > + } > } > > ar_sdio->n_rx_pkts = i;
On Thu, May 23, 2019 at 9:42 AM Brian Norris <briannorris@chromium.org> wrote: > IIUC, you have basically the same failure case a few lines up, where > ath10k_sdio_mbox_alloc_pkt_bundle() may fail. Do the same there? Oh, I see Erik Stromdahl already got that one: ath10k: sdio: add missing error check https://patchwork.kernel.org/patch/10906009/
Claire Chang <tientzu@chromium.org> wrote: > In function ath10k_sdio_mbox_rx_alloc() [sdio.c], > ath10k_sdio_mbox_alloc_rx_pkt() is called without handling the error cases. > This will make the driver think the allocation for skb is successful and > try to access the skb. If we enable failslab, system will easily crash with > NULL pointer dereferencing. > > Call trace of CONFIG_FAILSLAB: > ath10k_sdio_irq_handler+0x570/0xa88 [ath10k_sdio] > process_sdio_pending_irqs+0x4c/0x174 > sdio_run_irqs+0x3c/0x64 > sdio_irq_work+0x1c/0x28 > > Fixes: d96db25d2025 ("ath10k: add initial SDIO support") > Signed-off-by: Claire Chang <tientzu@chromium.org> > Reviewed-by: Brian Norris <briannorris@chromium.org> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> Patch applied to ath-next branch of ath.git, thanks. 4b553f3ca4cb ath10k: add missing error handling
diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c index 9bbd5b54b8ca..4b7f1c6f13e2 100644 --- a/drivers/net/wireless/ath/ath10k/sdio.c +++ b/drivers/net/wireless/ath/ath10k/sdio.c @@ -607,6 +607,10 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar, full_len, last_in_bundle, last_in_bundle); + if (ret) { + ath10k_warn(ar, "alloc_rx_pkt error %d\n", ret); + goto err; + } } ar_sdio->n_rx_pkts = i;
In function ath10k_sdio_mbox_rx_alloc() [sdio.c], ath10k_sdio_mbox_alloc_rx_pkt() is called without handling the error cases. This will make the driver think the allocation for skb is successful and try to access the skb. If we enable failslab, system will easily crash with NULL pointer dereferencing. Call trace of CONFIG_FAILSLAB: ath10k_sdio_irq_handler+0x570/0xa88 [ath10k_sdio] process_sdio_pending_irqs+0x4c/0x174 sdio_run_irqs+0x3c/0x64 sdio_irq_work+0x1c/0x28 Fixes: d96db25d2025 ("ath10k: add initial SDIO support") Signed-off-by: Claire Chang <tientzu@chromium.org> --- drivers/net/wireless/ath/ath10k/sdio.c | 4 ++++ 1 file changed, 4 insertions(+)