Possible race condition in fstorage causes NMI-Interrupt

[Adrian Eggenberger]

Hi all

I was investigating a NMI_interrupt today and found out that it was caused by a race condition in the fstorage module.

The problem may occur after a flash operation has been started and a new operation is just initiated by calling fs_store or fs_erase.

The call of fs_store or fs_erase uses queue_get_next_free to get a free operation entry from the queue. After queue_get_next_free completed a pointer to an free empty entry in the queue is returned. The problem is, that the entry already is an active part of the queue because count has been increase already in the queue_get_next_free function. Just after queue_get_next_free has returned the interrupt for the finished flash operation kicks in and fs_sys_event_handler is called in interrupt context.

In the fs_sys_event_handler the finished operation is handled and afterwards queue_process is called. There the check for (m_queue.count > 0) is positiv as the count variable was already increased for the new operation. The following switch for the opcode will result in a return value "FS_ERR_INTERNAL" because content of the new queue entry has not been written before the interrupt.

The later call to send_event causes afterwards a NMI_Interrupt because it tries to access the p_config of the operation, which is NULL in this case.

Can anyone from Nordic have a look at this issue?

Regards Adrian

[bjorn-spockeli]

Hi Adrian, do you have a code snippet that reproduces this issue?

[Adrian Eggenberger]

Hi Bjorn I've a full size project with forced FDS write access that may reproduce the behavior in a few seconds. As it contains a lot of product logic I can't share it here.

[emdi]

Hello Adrian, I think I can see what's happening.

I believe that protecting parts of the code in fstorage with critical sections may help you solve the problem, but I think a better solution would be to avoid running the fs_sys_event_handler function in the interrupt context. Rather, you could schedule its execution and run it in a context with the same interrupt level as the one where you call fs_store/fs_erase, to prevent these calls to be interrupted in the first place.

Does that make sense for you?

[Adrian Eggenberger]

Hi emdi

I totally agree with you. Actually protecting the critical area in fs_store/fs_erase by disabling interrupts solves the problem. I already verified that this works as expected. I guess it should be possible to write the code thread save without blocking interrupts, but I didn't had the time to have a look at it.

Also your second proposal will be a possible was to solve the issue, but with some performance decrease in flash access. In my case this is not an option as I need the the fs_store to complete without additional cpu time in the normal context.

[bjorn-spockeli]

@adrian: FYI: @emdi is one of the SW developers that wrote fstorage/FDS. So I guess that the only viable solution is to wrap the fs_store/fs_erase calls in critical sections since you can't schedule them to run in mains context?