[Coverity CID :205651]Uninitialized variables in /drivers/dma/dma_stm32.c

[aasthagr]

Static code scan issues seen in File: /drivers/dma/dma_stm32.c
Category: Uninitialized variables
Function: dma_stm32_configure
Component: Drivers
CID: 205651
Please fix or provide comments to square it off in coverity in the link: https://scan9.coverity.com/reports.htm#v32951/p12996

[aescolar]

749d2d2

[cybertale]

@aescolar Is this issue points to DMA_InitStruct in dma_stm32_configure()? I see this structure do need to be initialized before used. Sorry but I don't see problem description in your comment.

[aescolar]

@cybertale (I just helped triage it, by trying to pinpoint the origin and author)
This is the whole report from Coverity:

231static int dma_stm32_configure(struct device *dev, u32_t id,
232                               struct dma_config *config)
233{
234        struct dma_stm32_data *data = dev->driver_data;
235        struct dma_stm32_stream *stream = &data->streams[id];
236        const struct dma_stm32_config *dev_config =
237                                        dev->config->config_info;
238        DMA_TypeDef *dma = (DMA_TypeDef *)dev_config->base;
   	1. var_decl: Declaring variable DMA_InitStruct without initializer.
239        LL_DMA_InitTypeDef DMA_InitStruct;
240        u32_t msize;
241        int ret;
242
   	2. Condition id >= data->max_streams, taking false branch.
243        if (id >= data->max_streams) {
244                return -EINVAL;
245        }
246
   	3. Condition stream->busy, taking false branch.
247        if (stream->busy) {
248                return -EBUSY;
249        }
250
251        stm32_dma_disable_stream(dma, id);
252        dma_stm32_clear_stream_irq(dev, id);
253
   	4. Condition config->head_block->block_size > 65535, taking false branch.
254        if (config->head_block->block_size > DMA_STM32_MAX_DATA_ITEMS) {
255                LOG_ERR("Data size too big: %d\n",
256                       config->head_block->block_size);
257                return -EINVAL;
258        }
259
   	5. Condition stream->direction == MEMORY_TO_MEMORY, taking true branch.
   	6. Condition !dev_config->support_m2m, taking false branch.
260        if ((stream->direction == MEMORY_TO_MEMORY) &&
261                (!dev_config->support_m2m)) {
262                LOG_ERR("Memcopy not supported for device %s",
263                        dev->config->name);
264                return -ENOTSUP;
265        }
266
   	7. Condition config->source_data_size != 4U, taking true branch.
   	8. Condition config->source_data_size != 2U, taking true branch.
   	9. Condition config->source_data_size != 1U, taking false branch.
267        if (config->source_data_size != 4U &&
268            config->source_data_size != 2U &&
269            config->source_data_size != 1U) {
270                LOG_ERR("Source unit size error, %d",
271                        config->source_data_size);
272                return -EINVAL;
273        }
274
   	10. Condition config->dest_data_size != 4U, taking true branch.
   	11. Condition config->dest_data_size != 2U, taking true branch.
   	12. Condition config->dest_data_size != 1U, taking false branch.
275        if (config->dest_data_size != 4U &&
276            config->dest_data_size != 2U &&
277            config->dest_data_size != 1U) {
278                LOG_ERR("Dest unit size error, %d",
279                        config->dest_data_size);
280                return -EINVAL;
281        }
282
283        /*
284         * STM32's circular mode will auto reset both source address
285         * counter and destination address counter.
286         */
   	13. Condition config->head_block->source_reload_en != config->head_block->dest_reload_en, taking false branch.
287        if (config->head_block->source_reload_en !=
288                config->head_block->dest_reload_en) {
289                LOG_ERR("source_reload_en and dest_reload_en must "
290                        "be the same.");
291                return -EINVAL;
292        }
293
294        stream->busy            = true;
295        stream->dma_callback    = config->dma_callback;
296        stream->direction       = config->channel_direction;
297        stream->callback_arg    = config->callback_arg;
298        stream->src_size        = config->source_data_size;
299        stream->dst_size        = config->dest_data_size;
300
   	14. Condition stream->direction == MEMORY_TO_PERIPHERAL, taking true branch.
301        if (stream->direction == MEMORY_TO_PERIPHERAL) {
302                DMA_InitStruct.MemoryOrM2MDstAddress =
303                                        config->head_block->source_address;
304                DMA_InitStruct.PeriphOrM2MSrcAddress =
305                                        config->head_block->dest_address;
   	15. Falling through to end of if statement.
306        } else {
307                DMA_InitStruct.PeriphOrM2MSrcAddress =
308                                        config->head_block->source_address;
309                DMA_InitStruct.MemoryOrM2MDstAddress =
310                                        config->head_block->dest_address;
311        }
312
313        u16_t memory_addr_adj, periph_addr_adj;
314
315        ret = dma_stm32_get_priority(config->channel_priority,
316                                     &DMA_InitStruct.Priority);
   	16. Condition ret < 0, taking false branch.
317        if (ret < 0) {
318                return ret;
319        }
320
321        ret = dma_stm32_get_direction(config->channel_direction,
322                                      &DMA_InitStruct.Direction);
   	17. Condition ret < 0, taking false branch.
323        if (ret < 0) {
324                return ret;
325        }
326
   	18. Switch case value MEMORY_TO_PERIPHERAL.
327        switch (config->channel_direction) {
328        case MEMORY_TO_MEMORY:
329        case PERIPHERAL_TO_MEMORY:
330                memory_addr_adj = config->head_block->dest_addr_adj;
331                periph_addr_adj = config->head_block->source_addr_adj;
332                break;
333        case MEMORY_TO_PERIPHERAL:
334                memory_addr_adj = config->head_block->source_addr_adj;
335                periph_addr_adj = config->head_block->dest_addr_adj;
   	19. Breaking from switch.
336                break;
337        /* Direction has been asserted in dma_stm32_get_direction. */
338        }
339
340        ret = dma_stm32_get_memory_increment(memory_addr_adj,
341                                        &DMA_InitStruct.MemoryOrM2MDstIncMode);
   	20. Condition ret < 0, taking false branch.
342        if (ret < 0) {
343                return ret;
344        }
345        ret = dma_stm32_get_periph_increment(periph_addr_adj,
346                                        &DMA_InitStruct.PeriphOrM2MSrcIncMode);
   	21. Condition ret < 0, taking false branch.
347        if (ret < 0) {
348                return ret;
349        }
350
   	22. Condition config->head_block->source_reload_en, taking true branch.
351        if (config->head_block->source_reload_en) {
352                DMA_InitStruct.Mode = LL_DMA_MODE_CIRCULAR;
   	23. Falling through to end of if statement.
353        } else {
354                DMA_InitStruct.Mode = LL_DMA_MODE_NORMAL;
355        }
356
   	24. Condition stream->direction == MEMORY_TO_PERIPHERAL, taking true branch.
   	25. Condition stream->direction == MEMORY_TO_PERIPHERAL, taking true branch.
357        stream->source_periph = stream->direction == MEMORY_TO_PERIPHERAL;
358        ret = dma_stm32_width_config(config, stream->source_periph, dma,
359                                     &DMA_InitStruct, id);
   	26. Condition ret < 0, taking false branch.
360        if (ret < 0) {
361                return ret;
362        }
363        msize = DMA_InitStruct.MemoryOrM2MDstDataSize;
364
365#if defined(CONFIG_DMA_STM32_V1)
366        DMA_InitStruct.MemBurst = stm32_dma_get_mburst(config,
367                                                       stream->source_periph);
368        DMA_InitStruct.PeriphBurst = stm32_dma_get_pburst(config,
369                                                        stream->source_periph);
370
   	27. Condition config->channel_direction != MEMORY_TO_MEMORY, taking true branch.
371        if (config->channel_direction != MEMORY_TO_MEMORY) {
   	28. Condition config->dma_slot >= 8, taking false branch.
372                if (config->dma_slot >= 8) {
373                        LOG_ERR("dma slot error.");
374                        return -EINVAL;
375                }
   	29. Falling through to end of if statement.
376        } else {
377                if (config->dma_slot >= 8) {
378                        LOG_ERR("dma slot is too big, using 0 as default.");
379                        config->dma_slot = 0;
380                }
381        }
382        DMA_InitStruct.Channel = table_ll_channel[config->dma_slot];
383
384        stm32_dma_get_fifo_threshold(config->head_block->fifo_mode_control);
385
   	
CID 205651 (#1 of 1): Uninitialized scalar variable (UNINIT)
30. uninit_use_in_call: Using uninitialized value DMA_InitStruct.FIFOThreshold when calling stm32_dma_check_fifo_mburst. [show details]
386        if (stm32_dma_check_fifo_mburst(&DMA_InitStruct)) {
387                DMA_InitStruct.FIFOMode = LL_DMA_FIFOMODE_ENABLE;
388        } else {
389                DMA_InitStruct.FIFOMode = LL_DMA_FIFOMODE_DISABLE;
390        }
391#endif
392        if (stream->source_periph) {
393                DMA_InitStruct.NbData = config->head_block->block_size /
394                                        config->source_data_size;
395        } else {
396                DMA_InitStruct.NbData = config->head_block->block_size /
397                                        config->dest_data_size;
398        }
399
400        LL_DMA_Init(dma, table_ll_stream[id], &DMA_InitStruct);
401
402        LL_DMA_EnableIT_TC(dma, table_ll_stream[id]);
403
404#if defined(CONFIG_DMA_STM32_V1)
405        if (DMA_InitStruct.FIFOMode == LL_DMA_FIFOMODE_ENABLE) {
406                LL_DMA_EnableIT_FE(dma, table_ll_stream[id]);
407        } else {
408                LL_DMA_DisableIT_FE(dma, table_ll_stream[id]);
409        }
410#endif
411        return ret;
412}

[dleach02]

The specific problem is that the function stm32_dma_check_fifo_mburst() is referencing the FIFOThreshold field of DMA_InitStruct which has never been initialized. I don't know the hardware but I would figure out what FIFOThreshold should be initialized to before calling the check_fifo_mburst() function.

@cybertale

[galak]

The specific problem is that the function stm32_dma_check_fifo_mburst() is referencing the FIFOThreshold field of DMA_InitStruct which has never been initialized. I don't know the hardware but I would figure out what FIFOThreshold should be initialized to before calling the check_fifo_mburst() function.

@erwango any thoughts/comments?

[erwango]

Shouldn't we do something like :

DMA_InitStruct.FIFOThreshold = stm32_dma_get_fifo_threshold(config->head_block->fifo_mode_control);

Seems function stm32_dma_get_fifo_threshold is meant to provide threshold value, but return value is not used.

Btw, some clean up might be needed as stm32_dma_enable_fifo is not used ..

@cybertale , can you check these points ?

[cybertale]

Shouldn't we do something like :

DMA_InitStruct.FIFOThreshold = stm32_dma_get_fifo_threshold(config->head_block->fifo_mode_control);

Seems function stm32_dma_get_fifo_threshold is meant to provide threshold value, but return value is not used.

Btw, some clean up might be needed as stm32_dma_enable_fifo is not used ..

@cybertale , can you check these points ?

Yes, this is a potential bug I missed, I'll start fixing this now.