Null pointer dereferenced in sdc_handler in SDK 17.0.0

[blac01]

Hi,

While upmerging our project from SDK 15.3.0 to 17.0.0 we faced an issue in the sdc_handler due to usage of a NULL pointer.

In the case SDC_EVT_INIT if the p_event->result is not SDC_SUCCESS the block_dev_sdc_uninit function is called. It in turn sets m_active_sdc_dev = NULL (line 223) if uninit is success.
Then when calling the "if (m_active_sdc_dev->p_work->ev_handler)" in the sdc_handler at line 93 a NULL pointer is used.

Do you confirm this ?

Could you provide a solution ?

Thanks in advance

[Jørgen Holmefjord]

Hi,

I see two possible workarounds.

1. Add a break; at the end of the first if statement.
   

   case SDC_EVT_INIT:
               {
                   if (p_event->result != SDC_SUCCESS)
                   {
                       nrf_block_dev_t const * block_dev = &(p_sdc_dev->block_dev);
                       ret_code_t err_code = block_dev_sdc_uninit(block_dev);
                       APP_ERROR_CHECK(err_code);
                       break;
                   }
   
                   p_work->geometry.blk_count = app_sdc_info_get()->num_blocks;
                   p_work->geometry.blk_size  = SDC_SECTOR_SIZE;
                   if (m_active_sdc_dev->p_work->ev_handler)
                   {
                       const nrf_block_dev_event_t ev = {
                               NRF_BLOCK_DEV_EVT_INIT,
                               ((p_event->result == SDC_SUCCESS) ? \
                                       NRF_BLOCK_DEV_RESULT_SUCCESS : NRF_BLOCK_DEV_RESULT_IO_ERROR),
                               NULL,
                               p_work->p_context
                       };
                       p_work->ev_handler(&p_sdc_dev->block_dev, &ev);
                   }
               }
               break;
   

2. Move the first if statement after the second if statement, to allow passing the result of the init back to the event handler before uninit.
   

   case SDC_EVT_INIT:
               {
                   p_work->geometry.blk_count = app_sdc_info_get()->num_blocks;
                   p_work->geometry.blk_size  = SDC_SECTOR_SIZE;
                   if (m_active_sdc_dev->p_work->ev_handler)
                   {
                       const nrf_block_dev_event_t ev = {
                               NRF_BLOCK_DEV_EVT_INIT,
                               ((p_event->result == SDC_SUCCESS) ? \
                                       NRF_BLOCK_DEV_RESULT_SUCCESS : NRF_BLOCK_DEV_RESULT_IO_ERROR),
                               NULL,
                               p_work->p_context
                       };
                       p_work->ev_handler(&p_sdc_dev->block_dev, &ev);
                   }
                   
                   if (p_event->result != SDC_SUCCESS)
                   {
                       nrf_block_dev_t const * block_dev = &(p_sdc_dev->block_dev);
                       ret_code_t err_code = block_dev_sdc_uninit(block_dev);
                       APP_ERROR_CHECK(err_code);
                   }
               }
               break;
   

Best regards,
Jørgen

[blac01]

Hi Jørgen,

Sorry to come back late on your answer.

We’ve selected 2. as final fix. This is cleaner to us (uses the ev_handler, avoids having an early break statement, ...).

Note that we didn’t test the 1.

Best regards

Benoît