bt_le_scan_stop() before finding device results in Data Access Violation

[lanvis]

Describe the bug
Stopping scan via bt_le_scan_stop() called from a timer callback expiration without finding the target device results in Data Access Violation.

Violation occurs here:
Bluetooth initialized
Scanning successfully started
:bleSM: running
...BLE_GAP_EVT_TIMEOUT = 0x0019
Scan stopped
***** MPU FAULT *****
Data Access Violation
MMFAR Address: 0x0
***** Hardware exception *****
Current thread ID = 0x20000728
Faulting instruction address = 0x1b49c
Fatal fault in thread 0x20000728! Aborting.

This is HERE:
0001b498 <sys_dlist_remove>:
node->prev->next = node->next;
1b498: e9d0 2300 ldrd r2, r3, [r0]
1b49c: 601a str r2, [r3, #0]
node->next->prev = node->prev;
1b49e: 6802 ldr r2, [r0, #0]
1b4a0: 6053 str r3, [r2, #4]
node->next = NULL;
1b4a2: 2300 movs r3, #0
node->prev = NULL;
1b4a4: e9c0 3300 strd r3, r3, [r0]
}
1b4a8: 4770 bx lr

BugsXfer.zip

To Reproduce

1. Build Zephyr as a BLE central.
2. Set primary service scan with scan params.
3. Start the scan. Start a timer that times out and stops the current scan.

.type = (BT_HCI_LE_SCAN_PASSIVE),
.filter_dup = 0,
.interval = SCAN_INTERVAL=25,
.window = SCAN_WINDOW=25
};

*Expected behavior
Scanning should stop without signalling an exception.

Impact
Not stopping scanning for now ....

Screenshots or console output
Attached is .lst file and .map file.

Environment (please complete the following information):
OS: Ubuntu 16.4
Toolchain: Zephyr SDK
Version: OS 1.13.99

Additional context
Add any other context about the problem here.

[ioannisg]

Looks like a null-pointer exception - MMFAR Address is 0x0.

[jhedberg]

Which board is this on? Could you show us your application code? (or is this some existing sample in the tree?)

[lanvis]

Board = nrf52840_pca10056 (sorry about that .. should have been in there).
Code is custom but just looks for a device with specific advertising data.
If found and stop scan, no problem. If not found and stop scan, crash.

[jhedberg]

If found and stop scan, no problem. If not found and stop scan, crash.

@lanvis that sounds very strange. Number of advertising reports (even if zero) should not influence the scan state in any way (e.g. the scan result callback). Are you able to reproduce this with some app from the tree (if you can't show us your code)? Perhaps with tests/bluetooth/shell?

[lanvis]

Calling bt_le_scan_stop() from a timer timeout callback. Is that illegal?

[jhedberg]

Calling bt_le_scan_stop() from a timer timeout callback. Is that illegal?

@lanvis yes, most Bluetooth APIs are not safe to be called from interrupt context (which k_timer callbacks are). We should probably document this better. I'd suggest using k_delayed_work instead, if that suits your needs, or then use k_work to defer execution from your timer callback (OTOH that's essentially what k_delayed_work does).

[lanvis]

Bingo ... used k_work to schedule the stop_scan and all is good again. Thanks.
Question: Are Indicate and Notify callbacks running at interrupt context? We can close this one I think.

[jhedberg]

Question: Are Indicate and Notify callbacks running at interrupt context?

@lanvis no, they're in thread context. The Bluetooth RX thread to be precise (there's a Kconfig option if you need to change its stack size).

We can close this one I think.

Will do, thanks.

[ioannisg]

You could close this yourself, @lanvis :)