[PATCH] ASoC: SOF: Intel: hda-stream: Move three variable assignments behind condition checks in hda_dsp_iccmax_stream_hw_params()
Date: Wed, 19 Apr 2023 20:42:19 +0200
The address of a data structure member was determined before a corresponding null pointer check in the implementation of the function “hda_dsp_iccmax_stream_hw_params”.
Thus avoid the risk for undefined behaviour by moving the assignment for three local variables behind some condition checks.
This issue was detected by using the Coccinelle software.
Fixes: 7d88b9608142f95ccdd3dfb190da4a5faddb1cc7 ("ASoC: SOF: Intel: hdac_ext_stream: consistent prefixes for variables/members") Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/soc/sof/intel/hda-stream.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 79d818e6a0fa..9c44127014fc 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -407,10 +407,10 @@ int hda_dsp_iccmax_stream_hw_params(struct snd_sof_dev *sdev, struct hdac_ext_st struct snd_dma_buffer *dmab, struct snd_pcm_hw_params *params) { - struct hdac_stream *hstream = &hext_stream->hstream; - int sd_offset = SOF_STREAM_SD_OFFSET(hstream); + struct hdac_stream *hstream; + int sd_offset; int ret; - u32 mask = 0x1 << hstream->index; + u32 mask;
if (!hext_stream) { dev_err(sdev->dev, "error: no stream available\n"); @@ -422,9 +422,12 @@ int hda_dsp_iccmax_stream_hw_params(struct snd_sof_dev *sdev, struct hdac_ext_st return -ENODEV; }
+ hstream = &hext_stream->hstream; if (hstream->posbuf) *hstream->posbuf = 0;
+ sd_offset = SOF_STREAM_SD_OFFSET(hstream); + /* reset BDL address */ snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, sd_offset + SOF_HDA_ADSP_REG_SD_BDLPL, @@ -459,6 +462,8 @@ int hda_dsp_iccmax_stream_hw_params(struct snd_sof_dev *sdev, struct hdac_ext_st sd_offset + SOF_HDA_ADSP_REG_SD_LVI, 0xffff, (hstream->frags - 1));
+ mask = 0x1 << hstream->index; + /* decouple host and link DMA, enable DSP features */ snd_sof_dsp_update_bits(sdev, HDA_DSP_PP_BAR, SOF_HDA_REG_PP_PPCTL, mask, mask); -- 2.40.0
On 4/19/23 13:54, Markus Elfring wrote:
Date: Wed, 19 Apr 2023 20:42:19 +0200
The address of a data structure member was determined before a corresponding null pointer check in the implementation of the function “hda_dsp_iccmax_stream_hw_params”.
Thus avoid the risk for undefined behaviour by moving the assignment for three local variables behind some condition checks.
This issue was detected by using the Coccinelle software.
Fixes: 7d88b9608142f95ccdd3dfb190da4a5faddb1cc7 ("ASoC: SOF: Intel: hdac_ext_stream: consistent prefixes for variables/members") Signed-off-by: Markus Elfring elfring@users.sourceforge.net
Yes indeed, for some reason this was fixed in hda_dsp_stream_hw_params() but not in the hda_dsp_iccmax_stream_hw_params() variant.
Could we however use the same code as in hda_dsp_stream_hw_params() for consistency?
if (!hext_stream) { dev_err(sdev->dev, "error: no stream available\n"); return -ENODEV; }
if (!dmab) { dev_err(sdev->dev, "error: no dma buffer allocated!\n"); return -ENODEV; }
hstream = &hext_stream->hstream; sd_offset = SOF_STREAM_SD_OFFSET(hstream); mask = BIT(hstream->index);
Thanks!
sound/soc/sof/intel/hda-stream.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 79d818e6a0fa..9c44127014fc 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -407,10 +407,10 @@ int hda_dsp_iccmax_stream_hw_params(struct snd_sof_dev *sdev, struct hdac_ext_st struct snd_dma_buffer *dmab, struct snd_pcm_hw_params *params) {
- struct hdac_stream *hstream = &hext_stream->hstream;
- int sd_offset = SOF_STREAM_SD_OFFSET(hstream);
- struct hdac_stream *hstream;
- int sd_offset; int ret;
- u32 mask = 0x1 << hstream->index;
u32 mask;
if (!hext_stream) { dev_err(sdev->dev, "error: no stream available\n");
@@ -422,9 +422,12 @@ int hda_dsp_iccmax_stream_hw_params(struct snd_sof_dev *sdev, struct hdac_ext_st return -ENODEV; }
hstream = &hext_stream->hstream; if (hstream->posbuf) *hstream->posbuf = 0;
sd_offset = SOF_STREAM_SD_OFFSET(hstream);
/* reset BDL address */ snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, sd_offset + SOF_HDA_ADSP_REG_SD_BDLPL,
@@ -459,6 +462,8 @@ int hda_dsp_iccmax_stream_hw_params(struct snd_sof_dev *sdev, struct hdac_ext_st sd_offset + SOF_HDA_ADSP_REG_SD_LVI, 0xffff, (hstream->frags - 1));
- mask = 0x1 << hstream->index;
- /* decouple host and link DMA, enable DSP features */ snd_sof_dsp_update_bits(sdev, HDA_DSP_PP_BAR, SOF_HDA_REG_PP_PPCTL, mask, mask);
-- 2.40.0
The address of a data structure member was determined before a corresponding null pointer check in the implementation of the function “hda_dsp_iccmax_stream_hw_params”.
Thus avoid the risk for undefined behaviour by moving the assignment for three local variables behind some condition checks.
This issue was detected by using the Coccinelle software.
Fixes: 7d88b9608142f95ccdd3dfb190da4a5faddb1cc7 ("ASoC: SOF: Intel: hdac_ext_stream: consistent prefixes for variables/members") Signed-off-by: Markus Elfring elfring@users.sourceforge.net
Yes indeed, for some reason this was fixed in hda_dsp_stream_hw_params() but not in the hda_dsp_iccmax_stream_hw_params() variant.
Would Peter Ujfalusi like to support similar source code adjustments also according to his commit 09255c7ed8ca1f1ed99357b845d2f63fe2ef3e1e ("ASoC: SOF: Intel: hda-stream: Do not dereference hstream until it is safe") from 2023-04-04?
Could we however use the same code as in hda_dsp_stream_hw_params() for consistency?
…
hstream = &hext_stream->hstream; sd_offset = SOF_STREAM_SD_OFFSET(hstream); mask = BIT(hstream->index);
Can it matter to move such assignment statements a bit closer to subsequent statements?
Regards, Markus
participants (2)
-
Markus Elfring
-
Pierre-Louis Bossart