* [PATCH] ASoC: Intel: avoid Oops if DMA setup fails @ 2019-04-26 16:47 Ross Zwisler 2019-04-26 21:03 ` Pierre-Louis Bossart [not found] ` <20190426185246.AD8E5206A3@mail.kernel.org> 0 siblings, 2 replies; 14+ messages in thread From: Ross Zwisler @ 2019-04-26 16:47 UTC (permalink / raw) To: linux-kernel Cc: Ross Zwisler, Jaroslav Kysela, Jie Yang, Liam Girdwood, Mark Brown, Pierre-Louis Bossart, Takashi Iwai, alsa-devel, stable Currently in sst_dsp_new() if we get an error return from sst_dma_new() we just print an error message and then still complete the function successfully. This means that we are trying to run without sst->dma properly set up, which will result in NULL pointer dereference when sst->dma is later used. This was happening for me in sst_dsp_dma_get_channel(): struct sst_dma *dma = dsp->dma; ... dma->ch = dma_request_channel(mask, dma_chan_filter, dsp); This resulted in: BUG: unable to handle kernel NULL pointer dereference at 0000000000000018 IP: sst_dsp_dma_get_channel+0x4f/0x125 [snd_soc_sst_firmware] Fix this by adding proper error handling for the case where we fail to set up DMA. Signed-off-by: Ross Zwisler <zwisler@google.com> Cc: stable@vger.kernel.org --- sound/soc/intel/common/sst-firmware.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c index 1e067504b6043..9be3a793a55e3 100644 --- a/sound/soc/intel/common/sst-firmware.c +++ b/sound/soc/intel/common/sst-firmware.c @@ -1251,11 +1251,15 @@ struct sst_dsp *sst_dsp_new(struct device *dev, goto irq_err; err = sst_dma_new(sst); - if (err) + if (err) { dev_warn(dev, "sst_dma_new failed %d\n", err); + goto dma_err; + } return sst; +dma_err: + free_irq(sst->irq, sst); irq_err: if (sst->ops->free) sst->ops->free(sst); -- 2.21.0.593.g511ec345e18-goog ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] ASoC: Intel: avoid Oops if DMA setup fails 2019-04-26 16:47 [PATCH] ASoC: Intel: avoid Oops if DMA setup fails Ross Zwisler @ 2019-04-26 21:03 ` Pierre-Louis Bossart 2019-04-29 17:02 ` Ross Zwisler 2019-04-29 18:25 ` [PATCH v2] " Ross Zwisler [not found] ` <20190426185246.AD8E5206A3@mail.kernel.org> 1 sibling, 2 replies; 14+ messages in thread From: Pierre-Louis Bossart @ 2019-04-26 21:03 UTC (permalink / raw) To: Ross Zwisler, linux-kernel Cc: Ross Zwisler, Jaroslav Kysela, Jie Yang, Liam Girdwood, Mark Brown, Takashi Iwai, alsa-devel, stable On 4/26/19 11:47 AM, Ross Zwisler wrote: > Currently in sst_dsp_new() if we get an error return from sst_dma_new() > we just print an error message and then still complete the function > successfully. This means that we are trying to run without sst->dma > properly set up, which will result in NULL pointer dereference when > sst->dma is later used. This was happening for me in > sst_dsp_dma_get_channel(): > > struct sst_dma *dma = dsp->dma; > ... > dma->ch = dma_request_channel(mask, dma_chan_filter, dsp); > > This resulted in: > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000018 > IP: sst_dsp_dma_get_channel+0x4f/0x125 [snd_soc_sst_firmware] > > Fix this by adding proper error handling for the case where we fail to > set up DMA. > > Signed-off-by: Ross Zwisler <zwisler@google.com> > Cc: stable@vger.kernel.org > --- > sound/soc/intel/common/sst-firmware.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c > index 1e067504b6043..9be3a793a55e3 100644 > --- a/sound/soc/intel/common/sst-firmware.c > +++ b/sound/soc/intel/common/sst-firmware.c > @@ -1251,11 +1251,15 @@ struct sst_dsp *sst_dsp_new(struct device *dev, > goto irq_err; > > err = sst_dma_new(sst); > - if (err) > + if (err) { > dev_warn(dev, "sst_dma_new failed %d\n", err); > + goto dma_err; > + } Thanks for the patch. The fix looks correct, but does it make sense to keep a dev_warn() here? Should it be changed to dev_err() instead since as you mentioned it's fatal to keep going. Also you may want to mention in the commit message that this should only impact Broadwell and maybe the legacy Baytrail driver. IIRC we don't use the DMAs in other cases. > > return sst; > > +dma_err: > + free_irq(sst->irq, sst); > irq_err: > if (sst->ops->free) > sst->ops->free(sst); > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] ASoC: Intel: avoid Oops if DMA setup fails 2019-04-26 21:03 ` Pierre-Louis Bossart @ 2019-04-29 17:02 ` Ross Zwisler 2019-04-29 18:25 ` [PATCH v2] " Ross Zwisler 1 sibling, 0 replies; 14+ messages in thread From: Ross Zwisler @ 2019-04-29 17:02 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Ross Zwisler, linux-kernel, Jaroslav Kysela, Jie Yang, Liam Girdwood, Mark Brown, Takashi Iwai, alsa-devel, stable On Fri, Apr 26, 2019 at 04:03:47PM -0500, Pierre-Louis Bossart wrote: > On 4/26/19 11:47 AM, Ross Zwisler wrote: > > Currently in sst_dsp_new() if we get an error return from sst_dma_new() > > we just print an error message and then still complete the function > > successfully. This means that we are trying to run without sst->dma > > properly set up, which will result in NULL pointer dereference when > > sst->dma is later used. This was happening for me in > > sst_dsp_dma_get_channel(): > > > > struct sst_dma *dma = dsp->dma; > > ... > > dma->ch = dma_request_channel(mask, dma_chan_filter, dsp); > > > > This resulted in: > > > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000018 > > IP: sst_dsp_dma_get_channel+0x4f/0x125 [snd_soc_sst_firmware] > > > > Fix this by adding proper error handling for the case where we fail to > > set up DMA. > > > > Signed-off-by: Ross Zwisler <zwisler@google.com> > > Cc: stable@vger.kernel.org > > --- > > sound/soc/intel/common/sst-firmware.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c > > index 1e067504b6043..9be3a793a55e3 100644 > > --- a/sound/soc/intel/common/sst-firmware.c > > +++ b/sound/soc/intel/common/sst-firmware.c > > @@ -1251,11 +1251,15 @@ struct sst_dsp *sst_dsp_new(struct device *dev, > > goto irq_err; > > err = sst_dma_new(sst); > > - if (err) > > + if (err) { > > dev_warn(dev, "sst_dma_new failed %d\n", err); > > + goto dma_err; > > + } > > Thanks for the patch. > The fix looks correct, but does it make sense to keep a dev_warn() here? > Should it be changed to dev_err() instead since as you mentioned it's fatal > to keep going. > Also you may want to mention in the commit message that this should only > impact Broadwell and maybe the legacy Baytrail driver. IIRC we don't use the > DMAs in other cases. Sure, I'll address both of these in a v2. Thank you for the quick review. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] ASoC: Intel: avoid Oops if DMA setup fails 2019-04-26 21:03 ` Pierre-Louis Bossart 2019-04-29 17:02 ` Ross Zwisler @ 2019-04-29 18:25 ` Ross Zwisler 2019-04-29 18:45 ` Pierre-Louis Bossart ` (4 more replies) 1 sibling, 5 replies; 14+ messages in thread From: Ross Zwisler @ 2019-04-29 18:25 UTC (permalink / raw) To: Pierre-Louis Bossart, linux-kernel Cc: Ross Zwisler, Jaroslav Kysela, Jie Yang, Liam Girdwood, Mark Brown, Takashi Iwai, alsa-devel, stable Currently in sst_dsp_new() if we get an error return from sst_dma_new() we just print an error message and then still complete the function successfully. This means that we are trying to run without sst->dma properly set up, which will result in NULL pointer dereference when sst->dma is later used. This was happening for me in sst_dsp_dma_get_channel(): struct sst_dma *dma = dsp->dma; ... dma->ch = dma_request_channel(mask, dma_chan_filter, dsp); This resulted in: BUG: unable to handle kernel NULL pointer dereference at 0000000000000018 IP: sst_dsp_dma_get_channel+0x4f/0x125 [snd_soc_sst_firmware] Fix this by adding proper error handling for the case where we fail to set up DMA. This change only affects Haswell and Broadwell systems. Baytrail systems explicilty opt-out of DMA via sst->pdata->resindex_dma_base being set to -1. Signed-off-by: Ross Zwisler <zwisler@google.com> Cc: stable@vger.kernel.org --- Changes in v2: - Upgraded the sst_dma_new() failure message from dev_warn() to dev_err() (Pierre-Louis). - Noted in the changelog that this change only affects Haswell and Broadwell (Pierre-Louis). --- sound/soc/intel/common/sst-firmware.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c index 1e067504b6043..f830e59f93eaa 100644 --- a/sound/soc/intel/common/sst-firmware.c +++ b/sound/soc/intel/common/sst-firmware.c @@ -1251,11 +1251,15 @@ struct sst_dsp *sst_dsp_new(struct device *dev, goto irq_err; err = sst_dma_new(sst); - if (err) - dev_warn(dev, "sst_dma_new failed %d\n", err); + if (err) { + dev_err(dev, "sst_dma_new failed %d\n", err); + goto dma_err; + } return sst; +dma_err: + free_irq(sst->irq, sst); irq_err: if (sst->ops->free) sst->ops->free(sst); -- 2.21.0.593.g511ec345e18-goog ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ASoC: Intel: avoid Oops if DMA setup fails 2019-04-29 18:25 ` [PATCH v2] " Ross Zwisler @ 2019-04-29 18:45 ` Pierre-Louis Bossart 2019-05-03 4:34 ` Mark Brown ` (3 subsequent siblings) 4 siblings, 0 replies; 14+ messages in thread From: Pierre-Louis Bossart @ 2019-04-29 18:45 UTC (permalink / raw) To: Ross Zwisler, linux-kernel Cc: Ross Zwisler, Jaroslav Kysela, Jie Yang, Liam Girdwood, Mark Brown, Takashi Iwai, alsa-devel, stable On 4/29/19 1:25 PM, Ross Zwisler wrote: > Currently in sst_dsp_new() if we get an error return from sst_dma_new() > we just print an error message and then still complete the function > successfully. This means that we are trying to run without sst->dma > properly set up, which will result in NULL pointer dereference when > sst->dma is later used. This was happening for me in > sst_dsp_dma_get_channel(): > > struct sst_dma *dma = dsp->dma; > ... > dma->ch = dma_request_channel(mask, dma_chan_filter, dsp); > > This resulted in: > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000018 > IP: sst_dsp_dma_get_channel+0x4f/0x125 [snd_soc_sst_firmware] > > Fix this by adding proper error handling for the case where we fail to > set up DMA. > > This change only affects Haswell and Broadwell systems. Baytrail > systems explicilty opt-out of DMA via sst->pdata->resindex_dma_base > being set to -1. > > Signed-off-by: Ross Zwisler <zwisler@google.com> > Cc: stable@vger.kernel.org Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Thanks Ross! FWIW we should start deprecating this driver now and transition to SOF. I'll double-check how the upcoming 1.3 release works on my Pixel 2015/Samus device later this week. > --- > > Changes in v2: > - Upgraded the sst_dma_new() failure message from dev_warn() to dev_err() > (Pierre-Louis). > - Noted in the changelog that this change only affects Haswell and > Broadwell (Pierre-Louis). > > --- > sound/soc/intel/common/sst-firmware.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c > index 1e067504b6043..f830e59f93eaa 100644 > --- a/sound/soc/intel/common/sst-firmware.c > +++ b/sound/soc/intel/common/sst-firmware.c > @@ -1251,11 +1251,15 @@ struct sst_dsp *sst_dsp_new(struct device *dev, > goto irq_err; > > err = sst_dma_new(sst); > - if (err) > - dev_warn(dev, "sst_dma_new failed %d\n", err); > + if (err) { > + dev_err(dev, "sst_dma_new failed %d\n", err); > + goto dma_err; > + } > > return sst; > > +dma_err: > + free_irq(sst->irq, sst); > irq_err: > if (sst->ops->free) > sst->ops->free(sst); > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ASoC: Intel: avoid Oops if DMA setup fails 2019-04-29 18:25 ` [PATCH v2] " Ross Zwisler 2019-04-29 18:45 ` Pierre-Louis Bossart @ 2019-05-03 4:34 ` Mark Brown 2019-05-03 6:18 ` Applied "ASoC: Intel: avoid Oops if DMA setup fails" to the asoc tree Mark Brown ` (2 subsequent siblings) 4 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2019-05-03 4:34 UTC (permalink / raw) To: Ross Zwisler Cc: Pierre-Louis Bossart, linux-kernel, Ross Zwisler, Jaroslav Kysela, Jie Yang, Liam Girdwood, Takashi Iwai, alsa-devel, stable [-- Attachment #1: Type: text/plain, Size: 373 bytes --] On Mon, Apr 29, 2019 at 12:25:17PM -0600, Ross Zwisler wrote: > Currently in sst_dsp_new() if we get an error return from sst_dma_new() > we just print an error message and then still complete the function > successfully. This means that we are trying to run without sst->dma Please don't bury patches in the replies to existing threads, it makes it harder to spot them. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Applied "ASoC: Intel: avoid Oops if DMA setup fails" to the asoc tree 2019-04-29 18:25 ` [PATCH v2] " Ross Zwisler 2019-04-29 18:45 ` Pierre-Louis Bossart 2019-05-03 4:34 ` Mark Brown @ 2019-05-03 6:18 ` Mark Brown 2019-05-03 6:21 ` Mark Brown 2019-05-03 6:23 ` Mark Brown 4 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2019-05-03 6:18 UTC (permalink / raw) To: Ross Zwisler Cc: alsa-devel, Jaroslav Kysela, Jie Yang, Liam Girdwood, linux-kernel, Mark Brown, Pierre-Louis Bossart, Ross Zwisler, stable, Takashi Iwai The patch ASoC: Intel: avoid Oops if DMA setup fails has been applied to the asoc tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.2 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark From 0efa3334d65b7f421ba12382dfa58f6ff5bf83c4 Mon Sep 17 00:00:00 2001 From: Ross Zwisler <zwisler@chromium.org> Date: Mon, 29 Apr 2019 12:25:17 -0600 Subject: [PATCH] ASoC: Intel: avoid Oops if DMA setup fails Currently in sst_dsp_new() if we get an error return from sst_dma_new() we just print an error message and then still complete the function successfully. This means that we are trying to run without sst->dma properly set up, which will result in NULL pointer dereference when sst->dma is later used. This was happening for me in sst_dsp_dma_get_channel(): struct sst_dma *dma = dsp->dma; ... dma->ch = dma_request_channel(mask, dma_chan_filter, dsp); This resulted in: BUG: unable to handle kernel NULL pointer dereference at 0000000000000018 IP: sst_dsp_dma_get_channel+0x4f/0x125 [snd_soc_sst_firmware] Fix this by adding proper error handling for the case where we fail to set up DMA. This change only affects Haswell and Broadwell systems. Baytrail systems explicilty opt-out of DMA via sst->pdata->resindex_dma_base being set to -1. Signed-off-by: Ross Zwisler <zwisler@google.com> Cc: stable@vger.kernel.org Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- sound/soc/intel/common/sst-firmware.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c index 1e067504b604..f830e59f93ea 100644 --- a/sound/soc/intel/common/sst-firmware.c +++ b/sound/soc/intel/common/sst-firmware.c @@ -1251,11 +1251,15 @@ struct sst_dsp *sst_dsp_new(struct device *dev, goto irq_err; err = sst_dma_new(sst); - if (err) - dev_warn(dev, "sst_dma_new failed %d\n", err); + if (err) { + dev_err(dev, "sst_dma_new failed %d\n", err); + goto dma_err; + } return sst; +dma_err: + free_irq(sst->irq, sst); irq_err: if (sst->ops->free) sst->ops->free(sst); -- 2.20.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Applied "ASoC: Intel: avoid Oops if DMA setup fails" to the asoc tree 2019-04-29 18:25 ` [PATCH v2] " Ross Zwisler ` (2 preceding siblings ...) 2019-05-03 6:18 ` Applied "ASoC: Intel: avoid Oops if DMA setup fails" to the asoc tree Mark Brown @ 2019-05-03 6:21 ` Mark Brown 2019-05-03 6:23 ` Mark Brown 4 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2019-05-03 6:21 UTC (permalink / raw) To: Ross Zwisler Cc: alsa-devel, Jaroslav Kysela, Jie Yang, Liam Girdwood, linux-kernel, Mark Brown, Pierre-Louis Bossart, Ross Zwisler, stable, Takashi Iwai The patch ASoC: Intel: avoid Oops if DMA setup fails has been applied to the asoc tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.2 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark From 0efa3334d65b7f421ba12382dfa58f6ff5bf83c4 Mon Sep 17 00:00:00 2001 From: Ross Zwisler <zwisler@chromium.org> Date: Mon, 29 Apr 2019 12:25:17 -0600 Subject: [PATCH] ASoC: Intel: avoid Oops if DMA setup fails Currently in sst_dsp_new() if we get an error return from sst_dma_new() we just print an error message and then still complete the function successfully. This means that we are trying to run without sst->dma properly set up, which will result in NULL pointer dereference when sst->dma is later used. This was happening for me in sst_dsp_dma_get_channel(): struct sst_dma *dma = dsp->dma; ... dma->ch = dma_request_channel(mask, dma_chan_filter, dsp); This resulted in: BUG: unable to handle kernel NULL pointer dereference at 0000000000000018 IP: sst_dsp_dma_get_channel+0x4f/0x125 [snd_soc_sst_firmware] Fix this by adding proper error handling for the case where we fail to set up DMA. This change only affects Haswell and Broadwell systems. Baytrail systems explicilty opt-out of DMA via sst->pdata->resindex_dma_base being set to -1. Signed-off-by: Ross Zwisler <zwisler@google.com> Cc: stable@vger.kernel.org Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- sound/soc/intel/common/sst-firmware.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c index 1e067504b604..f830e59f93ea 100644 --- a/sound/soc/intel/common/sst-firmware.c +++ b/sound/soc/intel/common/sst-firmware.c @@ -1251,11 +1251,15 @@ struct sst_dsp *sst_dsp_new(struct device *dev, goto irq_err; err = sst_dma_new(sst); - if (err) - dev_warn(dev, "sst_dma_new failed %d\n", err); + if (err) { + dev_err(dev, "sst_dma_new failed %d\n", err); + goto dma_err; + } return sst; +dma_err: + free_irq(sst->irq, sst); irq_err: if (sst->ops->free) sst->ops->free(sst); -- 2.20.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Applied "ASoC: Intel: avoid Oops if DMA setup fails" to the asoc tree 2019-04-29 18:25 ` [PATCH v2] " Ross Zwisler ` (3 preceding siblings ...) 2019-05-03 6:21 ` Mark Brown @ 2019-05-03 6:23 ` Mark Brown 4 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2019-05-03 6:23 UTC (permalink / raw) To: Ross Zwisler Cc: alsa-devel, Jaroslav Kysela, Jie Yang, Liam Girdwood, linux-kernel, Mark Brown, Pierre-Louis Bossart, Ross Zwisler, stable, Takashi Iwai The patch ASoC: Intel: avoid Oops if DMA setup fails has been applied to the asoc tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.2 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark From 0efa3334d65b7f421ba12382dfa58f6ff5bf83c4 Mon Sep 17 00:00:00 2001 From: Ross Zwisler <zwisler@chromium.org> Date: Mon, 29 Apr 2019 12:25:17 -0600 Subject: [PATCH] ASoC: Intel: avoid Oops if DMA setup fails Currently in sst_dsp_new() if we get an error return from sst_dma_new() we just print an error message and then still complete the function successfully. This means that we are trying to run without sst->dma properly set up, which will result in NULL pointer dereference when sst->dma is later used. This was happening for me in sst_dsp_dma_get_channel(): struct sst_dma *dma = dsp->dma; ... dma->ch = dma_request_channel(mask, dma_chan_filter, dsp); This resulted in: BUG: unable to handle kernel NULL pointer dereference at 0000000000000018 IP: sst_dsp_dma_get_channel+0x4f/0x125 [snd_soc_sst_firmware] Fix this by adding proper error handling for the case where we fail to set up DMA. This change only affects Haswell and Broadwell systems. Baytrail systems explicilty opt-out of DMA via sst->pdata->resindex_dma_base being set to -1. Signed-off-by: Ross Zwisler <zwisler@google.com> Cc: stable@vger.kernel.org Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- sound/soc/intel/common/sst-firmware.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c index 1e067504b604..f830e59f93ea 100644 --- a/sound/soc/intel/common/sst-firmware.c +++ b/sound/soc/intel/common/sst-firmware.c @@ -1251,11 +1251,15 @@ struct sst_dsp *sst_dsp_new(struct device *dev, goto irq_err; err = sst_dma_new(sst); - if (err) - dev_warn(dev, "sst_dma_new failed %d\n", err); + if (err) { + dev_err(dev, "sst_dma_new failed %d\n", err); + goto dma_err; + } return sst; +dma_err: + free_irq(sst->irq, sst); irq_err: if (sst->ops->free) sst->ops->free(sst); -- 2.20.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
[parent not found: <20190426185246.AD8E5206A3@mail.kernel.org>]
* Re: [PATCH] ASoC: Intel: avoid Oops if DMA setup fails [not found] ` <20190426185246.AD8E5206A3@mail.kernel.org> @ 2019-04-29 18:27 ` Ross Zwisler 2019-05-03 19:45 ` [linux-4.4.y PATCH] " Ross Zwisler 0 siblings, 1 reply; 14+ messages in thread From: Ross Zwisler @ 2019-04-29 18:27 UTC (permalink / raw) To: Sasha Levin; +Cc: Ross Zwisler, linux-kernel, stable On Fri, Apr 26, 2019 at 06:52:45PM +0000, Sasha Levin wrote: > Hi, > > [This is an automated email] > > This commit has been processed because it contains a -stable tag. > The stable tag indicates that it's relevant for the following trees: all > > The bot has tested the following trees: v5.0.9, v4.19.36, v4.14.113, v4.9.170, v4.4.178, v3.18.138. > > v5.0.9: Build OK! > v4.19.36: Build OK! > v4.14.113: Build OK! > v4.9.170: Build OK! > v4.4.178: Failed to apply! Possible dependencies: > 2bd5bd15a518 ("ASoC: Intel: add bytct-rt5651 machine driver") > 2dcffcee23a2 ("ASoC: Intel: Create independent acpi match module") > 595788e475d0 ("ASoC: Intel: tag byt-rt5640 machine driver as deprecated") > 95f098014815 ("ASoC: Intel: Move apci find machine routines") > a395bdd6b24b ("ASoC: intel: Fix sst-dsp dependency on dw stuff") > a92ea59b74e2 ("ASoC: Intel: sst: only select sst-firmware when DW DMAC is built-in") > cfffcc66a89a ("ASoC: Intel: Load the atom DPCM driver only") > > v3.18.138: Failed to apply! Possible dependencies: > 0d2135ecadb0 ("ASoC: Intel: Work around to fix HW D3 potential crash issue") > 13735d1cecec ("ASoC: intel - kconfig: remove SND_SOC_INTEL_SST prompt") > 161aa49ef1b9 ("ASoC: Intel: Add new dependency for Haswell machine") > 2106241a6803 ("ASoC: Intel: create common folder and move common files in") > 282a331fe25c ("ASoC: Intel: Add new dependency for Broadwell machine") > 2e4f75919e5a ("ASoC: Intel: Add PM support to HSW/BDW PCM driver") > 34084a436703 ("ASoC: intel: Remove superfluous backslash in Kconfig") > 544c55c810a5 ("ASoC: Intel: Delete an unnecessary check before the function call "sst_dma_free"") > 63ae1fe7739e ("ASoC: Intel: Add dependency on DesignWare DMA controller") > 7dd6bd8926f3 ("ASoC: intel: kconfig - Move DW_DMAC_CORE dependency to machines") > 85b88a8dd0c7 ("ASoC: Intel: Store the entry_point read from FW file") > 9449d39b990d ("ASoC: Intel: add function to load firmware image") > a395bdd6b24b ("ASoC: intel: Fix sst-dsp dependency on dw stuff") > a92ea59b74e2 ("ASoC: Intel: sst: only select sst-firmware when DW DMAC is built-in") > aed3c7b77c85 ("ASoC: Intel: Add PM support to HSW/BDW IPC driver") > d96c53a193dd ("ASoC: Intel: Add generic support for DSP wake, sleep and stall") > e9600bc166d5 ("ASoC: Intel: Make ADSP memory block allocation more generic") > > > How should we proceed with this patch? After reviews I'll send backport patches for v4.4.X and v3.18.X as necessary. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [linux-4.4.y PATCH] ASoC: Intel: avoid Oops if DMA setup fails 2019-04-29 18:27 ` [PATCH] ASoC: Intel: avoid Oops if DMA setup fails Ross Zwisler @ 2019-05-03 19:45 ` Ross Zwisler 2019-05-05 13:15 ` Greg KH 0 siblings, 1 reply; 14+ messages in thread From: Ross Zwisler @ 2019-05-03 19:45 UTC (permalink / raw) To: Sasha Levin Cc: Ross Zwisler, linux-kernel, stable, Ross Zwisler, Pierre-Louis Bossart, Mark Brown From: Ross Zwisler <zwisler@chromium.org> commit 0efa3334d65b7f421ba12382dfa58f6ff5bf83c4 upstream. Currently in sst_dsp_new() if we get an error return from sst_dma_new() we just print an error message and then still complete the function successfully. This means that we are trying to run without sst->dma properly set up, which will result in NULL pointer dereference when sst->dma is later used. This was happening for me in sst_dsp_dma_get_channel(): struct sst_dma *dma = dsp->dma; ... dma->ch = dma_request_channel(mask, dma_chan_filter, dsp); This resulted in: BUG: unable to handle kernel NULL pointer dereference at 0000000000000018 IP: sst_dsp_dma_get_channel+0x4f/0x125 [snd_soc_sst_firmware] Fix this by adding proper error handling for the case where we fail to set up DMA. This change only affects Haswell and Broadwell systems. Baytrail systems explicilty opt-out of DMA via sst->pdata->resindex_dma_base being set to -1. Signed-off-by: Ross Zwisler <zwisler@google.com> Cc: stable@vger.kernel.org Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- The upstream patch applied cleanly to all stable trees except linux-4.4.y and linux-3.18.y. This is the backport for linux-4.4.y, and the code I'm fixing was introduced in v4.0 so there is no need for a linux-3.18.y backport. The upstream patch is currently in Mark Brown's tree: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/log/?h=for-next Is that good enough, or should I resend after it's been merged in the v5.2 merge window? --- sound/soc/intel/common/sst-dsp.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sound/soc/intel/common/sst-dsp.c b/sound/soc/intel/common/sst-dsp.c index c9452e02e0dda..c0a50ecb6dbda 100644 --- a/sound/soc/intel/common/sst-dsp.c +++ b/sound/soc/intel/common/sst-dsp.c @@ -463,11 +463,15 @@ struct sst_dsp *sst_dsp_new(struct device *dev, goto irq_err; err = sst_dma_new(sst); - if (err) - dev_warn(dev, "sst_dma_new failed %d\n", err); + if (err) { + dev_err(dev, "sst_dma_new failed %d\n", err); + goto dma_err; + } return sst; +dma_err: + free_irq(sst->irq, sst); irq_err: if (sst->ops->free) sst->ops->free(sst); -- 2.21.0.1020.gf2820cf01a-goog ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [linux-4.4.y PATCH] ASoC: Intel: avoid Oops if DMA setup fails 2019-05-03 19:45 ` [linux-4.4.y PATCH] " Ross Zwisler @ 2019-05-05 13:15 ` Greg KH 2019-05-09 17:41 ` Greg KH 0 siblings, 1 reply; 14+ messages in thread From: Greg KH @ 2019-05-05 13:15 UTC (permalink / raw) To: Ross Zwisler Cc: Sasha Levin, linux-kernel, stable, Ross Zwisler, Pierre-Louis Bossart, Mark Brown On Fri, May 03, 2019 at 01:45:03PM -0600, Ross Zwisler wrote: > From: Ross Zwisler <zwisler@chromium.org> > > commit 0efa3334d65b7f421ba12382dfa58f6ff5bf83c4 upstream. This commit id is not in Linus's tree :( ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [linux-4.4.y PATCH] ASoC: Intel: avoid Oops if DMA setup fails 2019-05-05 13:15 ` Greg KH @ 2019-05-09 17:41 ` Greg KH 2019-05-09 18:11 ` Ross Zwisler 0 siblings, 1 reply; 14+ messages in thread From: Greg KH @ 2019-05-09 17:41 UTC (permalink / raw) To: Ross Zwisler Cc: Sasha Levin, linux-kernel, stable, Ross Zwisler, Pierre-Louis Bossart, Mark Brown On Sun, May 05, 2019 at 03:15:53PM +0200, Greg KH wrote: > On Fri, May 03, 2019 at 01:45:03PM -0600, Ross Zwisler wrote: > > From: Ross Zwisler <zwisler@chromium.org> > > > > commit 0efa3334d65b7f421ba12382dfa58f6ff5bf83c4 upstream. > > This commit id is not in Linus's tree :( > Sorry for the noise, I didn't read your note :( ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [linux-4.4.y PATCH] ASoC: Intel: avoid Oops if DMA setup fails 2019-05-09 17:41 ` Greg KH @ 2019-05-09 18:11 ` Ross Zwisler 0 siblings, 0 replies; 14+ messages in thread From: Ross Zwisler @ 2019-05-09 18:11 UTC (permalink / raw) To: Greg KH Cc: Ross Zwisler, Sasha Levin, linux-kernel, stable, Pierre-Louis Bossart, Mark Brown On Thu, May 09, 2019 at 07:41:47PM +0200, Greg KH wrote: > On Sun, May 05, 2019 at 03:15:53PM +0200, Greg KH wrote: > > On Fri, May 03, 2019 at 01:45:03PM -0600, Ross Zwisler wrote: > > > From: Ross Zwisler <zwisler@chromium.org> > > > > > > commit 0efa3334d65b7f421ba12382dfa58f6ff5bf83c4 upstream. > > > > This commit id is not in Linus's tree :( > > > > Sorry for the noise, I didn't read your note :( No worries, sorry for sending early. I'll wait until after it's merged next time. :) Thanks for pulling it in. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-05-09 18:11 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-26 16:47 [PATCH] ASoC: Intel: avoid Oops if DMA setup fails Ross Zwisler 2019-04-26 21:03 ` Pierre-Louis Bossart 2019-04-29 17:02 ` Ross Zwisler 2019-04-29 18:25 ` [PATCH v2] " Ross Zwisler 2019-04-29 18:45 ` Pierre-Louis Bossart 2019-05-03 4:34 ` Mark Brown 2019-05-03 6:18 ` Applied "ASoC: Intel: avoid Oops if DMA setup fails" to the asoc tree Mark Brown 2019-05-03 6:21 ` Mark Brown 2019-05-03 6:23 ` Mark Brown [not found] ` <20190426185246.AD8E5206A3@mail.kernel.org> 2019-04-29 18:27 ` [PATCH] ASoC: Intel: avoid Oops if DMA setup fails Ross Zwisler 2019-05-03 19:45 ` [linux-4.4.y PATCH] " Ross Zwisler 2019-05-05 13:15 ` Greg KH 2019-05-09 17:41 ` Greg KH 2019-05-09 18:11 ` Ross Zwisler
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).