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@xxxxxxxxxxxxxxxxxxxxx>
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
>
