Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esp_ble_resolve_adv_data Trigger crash (IDFGH-11791) #12886

Closed
3 tasks done
maenkai opened this issue Dec 28, 2023 · 18 comments
Closed
3 tasks done

esp_ble_resolve_adv_data Trigger crash (IDFGH-11791) #12886

maenkai opened this issue Dec 28, 2023 · 18 comments
Assignees
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Bug bugs in IDF

Comments

@maenkai
Copy link

maenkai commented Dec 28, 2023

Answers checklist.

  • I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v4.3.1

Espressif SoC revision.

ESP32D-WROVE

Operating System used.

Linux

How did you build your project?

Command line with idf.py

If you are using Windows, please specify command line type.

None

Development Kit.

ESP32D-WROVE

Power Supply used.

USB

What is the expected behavior?

The scan adv is normal

What is the actual behavior?

Trigger crash

Steps to reproduce.

Run on standby for a period of time

Debug Logs.

16:22:56.292  Guru Meditation Error: Core  0 panic'ed (LoadStoreError). Exception was unhandled.
16:22:56.293  
16:22:56.295  Core  0 register dump:
16:22:56.302  PC      : 0x400ff194  PS      : 0x00060330  A0      : 0x800f6411  A1      : 0x3ffe22b0  
16:22:56.328  A2      : 0x3fffffcc  A3      : 0x000000ff  A4      : 0x3ffe233f  A5      : 0x3fffffe1  
16:22:56.329  A6      : 0x3ffe2380  A7      : 0x3ffe2340  A8      : 0x4000002d  A9      : 0x0000004c  
16:22:56.340  A10     : 0x000000ce  A11     : 0x0000003e  A12     : 0x00060523  A13     : 0x00060523  
16:22:56.341  A14     : 0x00000001  A15     : 0x0000001a  SAR     : 0x0000001d  EXCCAUSE: 0x00000003  
16:22:56.343  EXCVADDR: 0x4000002d  LBEG    : 0x4000c2e0  LEND    : 0x4000c2f6  LCOUNT  : 0xffffffff  
16:22:56.366  
16:22:56.367  Backtrace:0x400ff191:0x3ffe22b0 0x400f640e:0x3ffe22d0 0x400e94b6:0x3ffe2300 0x400e9dbe:0x3ffe23a0 0x4014bf2d:0x3ffe23c0 0x4013bd2b:0x3ffe23f0 0x4013d993:0x3ffe2410
16:22:56.409  
16:22:56.409  
16:22:56.409  ELF file SHA256: ff826356d3b860d2
16:22:56.410  
16:22:56.410  Rebooting...
0x400ff194: BTM_CheckAdvData at /home/mek/esp/esp-idf-v4-3-1/components/bt/host/bluedroid/stack/btm/btm_ble_gap.c:2081
0x400f640e: esp_ble_resolve_adv_data at /home/mek/esp/esp-idf-v4-3-1/components/bt/host/bluedroid/api/esp_gap_ble_api.c:420
0x400e94b6: mz_handle_scan_result at /home/mek/Music/Lipro/common/bt/mz_ble_central.c:552
0x400e9dbe: mz_central_ble_adv_callback at /home/mek/Music/Lipro/common/bt/mz_ble_central.c:1210
 (inlined by) mz_central_ble_adv_callback at /home/mek/Music/Lipro/common/bt/mz_ble_central.c:1207
0x4014bf2d: btc_ble_mesh_ble_cb_to_app at /home/mek/esp/esp-idf-v4-3-1/components/bt/esp_ble_mesh/btc/btc_ble_mesh_ble.c:179
 (inlined by) btc_ble_mesh_ble_cb_handler at /home/mek/esp/esp-idf-v4-3-1/components/bt/esp_ble_mesh/btc/btc_ble_mesh_ble.c:195
0x4013bd2b: btc_thread_handler at /home/mek/esp/esp-idf-v4-3-1/components/bt/common/btc/core/btc_task.c:184
0x4013d993: osi_thread_run at /home/mek/esp/esp-idf-v4-3-1/components/bt/common/osi/thread.c:68

More Information.

No response

@maenkai maenkai added the Type: Bug bugs in IDF label Dec 28, 2023
@espressif-bot espressif-bot added the Status: Opened Issue is new label Dec 28, 2023
@github-actions github-actions bot changed the title esp_ble_resolve_adv_data Trigger crash esp_ble_resolve_adv_data Trigger crash (IDFGH-11791) Dec 28, 2023
@maenkai
Copy link
Author

maenkai commented Dec 28, 2023

I tried to fix it this way,I wonder if this problem can be solved

UINT8 *BTM_CheckAdvData( UINT8 *p_adv, UINT8 type, UINT8 *p_length)
{
    UINT8 *p = p_adv;
    UINT8 length;
    UINT8 adv_type;
    BTM_TRACE_API("BTM_CheckAdvData type=0x%02X", type);

    STREAM_TO_UINT8(length, p);

    while ( length && (p - p_adv <= BTM_BLE_CACHE_ADV_DATA_MAX)) {
        STREAM_TO_UINT8(adv_type, p);

        if ( adv_type == type ) {
            /* length doesn't include itself */
            *p_length = length - 1; /* minus the length of type */
            return p;
        }
        p += length - 1; /* skip the length of data */

        //mz_fix_start 
        //广播里的length字段出现错误,可能是对端发了一个错误的广播或者其它原因导致数据被污染,
        //这里判断一下防止内存越界
        if(p >= p_adv + BTM_BLE_CACHE_ADV_DATA_MAX){
          break;
        }
        //mz_fix_end

        STREAM_TO_UINT8(length, p);
    }

    *p_length = 0;
    return NULL;
}

@espressif-bot espressif-bot assigned forx157 and esp-zhp and unassigned forx157 Dec 28, 2023
@maenkai
Copy link
Author

maenkai commented Dec 29, 2023

When BTM_BLE_CACHE_ADV_DATA_MAX is greater than p_length, two wrong types and lengths will be accessed illegally,
e.g:“ b6 fb ”

09:42:33.475  E (13208) BT_BTM: 182 adv invalid len.
09:42:33.480  I (00:00:12.463) BT_BTM: addr = 5c:cd:7c:57:c0:61
09:42:33.491  I (00:00:12.463) BT_BTM: 02 01 06 03 03 27 18 15 16 27 18 03 ab 5c cd 7c 
09:42:33.497  I (00:00:12.470) BT_BTM: 57 c0 61 09 88 18 00 05 05 12 40 00 00 b6 fb 

@esp-zhp
Copy link
Collaborator

esp-zhp commented Jan 3, 2024

Could you please try version 4.4 or later to see if there are any issues?

@maenkai
Copy link
Author

maenkai commented Jan 3, 2024

Could you please try version 4.4 or later to see if there are any issues?

Can you provide relevant patches for repair? Because my product has been released based on version 4.3.1, I cannot change the version

@esp-zhp
Copy link
Collaborator

esp-zhp commented Jan 3, 2024

@maenkai
ok,
how to reproduce your issue?

@maenkai
Copy link
Author

maenkai commented Jan 3, 2024

It's not duplicated in the actual test, it's code

//handle
static void handle_scan_result(uint8_t *ble_data, uint16_t length, uint8_t addr[6], uint8_t addr_type, int8_t rssi, bool from_mesh) {
  uint16_t company_id = 0;
  uint32_t device_type = 0xffffffff;
  uint8_t *manuf_data = NULL;
  uint8_t manuf_data_len = 0;

  manuf_data = esp_ble_resolve_adv_data(ble_data,
                                        ESP_BLE_AD_MANUFACTURER_SPECIFIC_TYPE,
                                        &manuf_data_len);
  if (manuf_data == NULL) {
    return;
  }
  //doing..............
}

//from gap evetn
    case ESP_GAP_BLE_SCAN_RESULT_EVT: {
      esp_ble_gap_cb_param_t *scan_result = (esp_ble_gap_cb_param_t *)param;
      switch (scan_result->scan_rst.search_evt) {
        case ESP_GAP_SEARCH_INQ_RES_EVT:
          mz_handle_scan_result(scan_result->scan_rst.ble_adv,
                                scan_result->scan_rst.adv_data_len,
                                scan_result->scan_rst.bda,
                                scan_result->scan_rst.ble_addr_type,
                                scan_result->scan_rst.rssi,
                                false);
          break;
        default:
          break;
      }
      break;
    }

//from mesh event
  switch (event) {
  case ESP_BLE_MESH_SCAN_BLE_ADVERTISING_PKT_EVT: {
    mz_handle_scan_result(param->scan_ble_adv_pkt.data,
                          param->scan_ble_adv_pkt.length,
                          param->scan_ble_adv_pkt.addr,
                          param->scan_ble_adv_pkt.addr_type,
                          param->scan_ble_adv_pkt.rssi,
                          true);
    break;
  }

@maenkai
Copy link
Author

maenkai commented Jan 3, 2024

@maenkai ok, how to reproduce your issue?

Not 100% in the actual test.
It's code.

//handle
static void handle_scan_result(uint8_t *ble_data, uint16_t length, uint8_t addr[6], uint8_t addr_type, int8_t rssi, bool from_mesh) {
  uint16_t company_id = 0;
  uint32_t device_type = 0xffffffff;
  uint8_t *manuf_data = NULL;
  uint8_t manuf_data_len = 0;

  manuf_data = esp_ble_resolve_adv_data(ble_data,
                                        ESP_BLE_AD_MANUFACTURER_SPECIFIC_TYPE,
                                        &manuf_data_len);
  if (manuf_data == NULL) {
    return;
  }
  //doing..............
}

//from gap evetn
    case ESP_GAP_BLE_SCAN_RESULT_EVT: {
      esp_ble_gap_cb_param_t *scan_result = (esp_ble_gap_cb_param_t *)param;
      switch (scan_result->scan_rst.search_evt) {
        case ESP_GAP_SEARCH_INQ_RES_EVT:
          handle_scan_result(scan_result->scan_rst.ble_adv,
                                scan_result->scan_rst.adv_data_len,
                                scan_result->scan_rst.bda,
                                scan_result->scan_rst.ble_addr_type,
                                scan_result->scan_rst.rssi,
                                false);
          break;
        default:
          break;
      }
      break;
    }

//from mesh event
  switch (event) {
  case ESP_BLE_MESH_SCAN_BLE_ADVERTISING_PKT_EVT: {
    handle_scan_result(param->scan_ble_adv_pkt.data,
                          param->scan_ble_adv_pkt.length,
                          param->scan_ble_adv_pkt.addr,
                          param->scan_ble_adv_pkt.addr_type,
                          param->scan_ble_adv_pkt.rssi,
                          true);
    break;
  }

@esp-zhp
Copy link
Collaborator

esp-zhp commented Jan 3, 2024

Can you print the adv data before the crash?
#include "esp_log.h"
esp_log_buffer_hex("adv_rsp", scan_result->scan_rst.ble_adv, scan_result->scan_rst.adv_data_len);

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Jan 3, 2024
@maenkai
Copy link
Author

maenkai commented Jan 3, 2024

Can you print the adv data before the crash? #include "esp_log.h" esp_log_buffer_hex("adv_rsp", scan_result->scan_rst.ble_adv, scan_result->scan_rst.adv_data_len);

OK,I need time to recreate

@esp-zhp
Copy link
Collaborator

esp-zhp commented Jan 3, 2024

According to your description, there might be a crash issue when encountering specific adv data.
However, based on the logic of the code, I haven't identified any apparent problems.
I'm trying to construct specific ADV data to reproduce the issue on my end, but it hasn't occurred.such as :

uint8_t adv_data[] = {0x02,0x01,0x06,
0x03,0x03,0x27,0x18,
0x15,0x16,0x27,0x18,0x03,0xab,0x5c,0xcd,0x7c,0x57,0xc0,0x61,0x09,0x88,0x18,0x00,0x05,0x05,0x12,0x40,0x00,0x00,
0xb6,ESP_BLE_AD_MANUFACTURER_SPECIFIC_TYPE};
adv_name = esp_ble_resolve_adv_data(adv_data,
ESP_BLE_AD_MANUFACTURER_SPECIFIC_TYPE, &adv_name_len);

Therefore, if you can provide ADV data that may lead to a crash, it would be very helpful.

@maenkai
Copy link
Author

maenkai commented Jan 3, 2024

According to your description, there might be a crash issue when encountering specific adv data. However, based on the logic of the code, I haven't identified any apparent problems. I'm trying to construct specific ADV data to reproduce the issue on my end, but it hasn't occurred.such as :

uint8_t adv_data[] = {0x02,0x01,0x06,
0x03,0x03,0x27,0x18,
0x15,0x16,0x27,0x18,0x03,0xab,0x5c,0xcd,0x7c,0x57,0xc0,0x61,0x09,0x88,0x18,0x00,0x05,0x05,0x12,0x40,0x00,0x00,
0xb6,ESP_BLE_AD_MANUFACTURER_SPECIFIC_TYPE};
adv_name = esp_ble_resolve_adv_data(adv_data,
ESP_BLE_AD_MANUFACTURER_SPECIFIC_TYPE, &adv_name_len);

Therefore, if you can provide ADV data that may lead to a crash, it would be very helpful.

I didn't capture the broadcast that caused the crash in the real world.
But here are the data I simulated that can cause a crash

  uint8_t test_adv[ESP_BLE_ADV_DATA_LEN_MAX + ESP_BLE_SCAN_RSP_DATA_LEN_MAX] = {
    0x02, 0x01, 0x06,
    0x03, 0x03, 0x27, 0x18,
    0x15, 0x16, 0x27, 0x18, 0x03, 0xab, 0x5c, 0xcd, 0x7c, 0x57, 0xc0, 0x61, 0x09, 0x88, 0x18, 0x00, 0x05, 0x05, 0x12, 0x40, 0x00, 0x00,
    0xff, 0xfb
  };

  manuf_data = esp_ble_resolve_adv_data(test_adv,
                                        ESP_BLE_AD_MANUFACTURER_SPECIFIC_TYPE,
                                        29);

@esp-zhp
Copy link
Collaborator

esp-zhp commented Jan 3, 2024

param[out] length - return the length of ADV data not including type
/**

  • @brief This function is called to get ADV data for a specific type.
  • @param[in] adv_data - pointer of ADV data which to be resolved
  • @param[in] type - finding ADV data type
  • @param[out] length - return the length of ADV data not including type
  • @return pointer of ADV data

*/
uint8_t *esp_ble_resolve_adv_data(uint8_t *adv_data, uint8_t type, uint8_t *length);
image

@maenkai
Copy link
Author

maenkai commented Jan 3, 2024

In the mesh scan event callback, how to ensure that the data range is greater than or equal to BTM_BLE_CACHE_ADV_DATA_MAX?
Maybe that's where the problem lies

  • esp code
    struct {
        uint8_t  addr[6];   /*!< Device address */
        uint8_t  addr_type; /*!< Device address type */
        uint8_t  adv_type;  /*!< Advertising data type */
        uint8_t *data;      /*!< Advertising data */
        uint16_t length;    /*!< Advertising data length */
        int8_t   rssi;      /*!< RSSI of the advertising packet */
    } scan_ble_adv_pkt;     /*!< Event parameters of ESP_BLE_MESH_SCAN_BLE_ADVERTISING_PKT_EVT */
} esp_ble_mesh_ble_cb_param_t;
  • demo
  switch (event) {
  case ESP_BLE_MESH_SCAN_BLE_ADVERTISING_PKT_EVT: {
    handle_scan_result(param->scan_ble_adv_pkt.data,
                          param->scan_ble_adv_pkt.length,
                          param->scan_ble_adv_pkt.addr,
                          param->scan_ble_adv_pkt.addr_type,
                          param->scan_ble_adv_pkt.rssi,
                          true);
    break;
  }

@maenkai
Copy link
Author

maenkai commented Jan 3, 2024

param[out] length - return the length of ADV data not including type /**

  • @brief This function is called to get ADV data for a specific type.
  • @param[in] adv_data - pointer of ADV data which to be resolved
  • @param[in] type - finding ADV data type
  • @param[out] length - return the length of ADV data not including type
  • @return pointer of ADV data

*/ uint8_t *esp_ble_resolve_adv_data(uint8_t *adv_data, uint8_t type, uint8_t *length); image

Sorry for writing the wrong demo, after modifying it correctly, you can see that the pointer p still accesses addresses that are out of range, which is a serious problem.
At the same time, I'm confused why doesn't it trigger a crash

  • demo
  // ESP_BLE_ADV_DATA_LEN_MAX = 31 ESP_BLE_SCAN_RSP_DATA_LEN_MAX = 31
  uint8_t test_adv[ESP_BLE_ADV_DATA_LEN_MAX + ESP_BLE_SCAN_RSP_DATA_LEN_MAX] = {
    0x02, 0x01, 0x06,
    0x03, 0x03, 0x27, 0x18,
    0x15, 0x16, 0x27, 0x18, 0x03, 0xab, 0x5c, 0xcd, 0x7c, 0x57, 0xc0, 0x61, 0x09, 0x88, 0x18, 0x00, 0x05, 0x05, 0x12, 0x40, 0x00, 0x00,
    0xff, 0xfb
  };

  uint8_t manuf_data_len = 29;

  esp_ble_resolve_adv_data(test_adv,
                           ESP_BLE_AD_MANUFACTURER_SPECIFIC_TYPE,
                           &manuf_data_len);


UINT8 *BTM_CheckAdvData( UINT8 *p_adv, UINT8 type, UINT8 *p_length)
{
    UINT8 *p = p_adv;
    UINT8 length;
    UINT8 adv_type;
    BTM_TRACE_API("BTM_CheckAdvData type=0x%02X", type);

    STREAM_TO_UINT8(length, p);

    while ( length && (p - p_adv <= BTM_BLE_CACHE_ADV_DATA_MAX)) {
        STREAM_TO_UINT8(adv_type, p);

        if ( adv_type == type ) {
            /* length doesn't include itself */
            *p_length = length - 1; /* minus the length of type */
            return p;
        }
        p += length - 1; /* skip the length of data */
        printf("length is %d p = %p end = %p\r\n",length, p , p_adv + BTM_BLE_CACHE_ADV_DATA_MAX);
        STREAM_TO_UINT8(length, p);
    }

    *p_length = 0;
    return NULL;
}
  • LOG
17:17:03.788  length is 2 p = 0x3ffbcdf3 end = 0x3ffbce2e
17:17:03.794  length is 3 p = 0x3ffbcdf7 end = 0x3ffbce2e
17:17:03.800  length is 21 p = 0x3ffbce0d end = 0x3ffbce2e
17:17:03.802  length is 255 p = 0x3ffbcf0d end = 0x3ffbce2e

@esp-zhp
Copy link
Collaborator

esp-zhp commented Jan 3, 2024

@maenkai
With the added conditional statement, will there still be a possibility of a crash?
image

@maenkai
Copy link
Author

maenkai commented Jan 3, 2024

@maenkai With the added conditional statement, will there still be a possibility of a crash? image

I need to test it for a long time and get back to you with the results

@esp-zhp
Copy link
Collaborator

esp-zhp commented Jan 3, 2024

@maenkai
I am looking forward your feedback.

@maenkai With the added conditional statement, will there still be a possibility of a crash? image

I need to test it for a long time and get back to you with the results

@esp-zhp
Copy link
Collaborator

esp-zhp commented Jan 10, 2024

Hi,
I believe this is a bug, and I will fix this issue in the upcoming versions.
thanks for report the issue.
commit:
c66fc14

@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: In Progress Work is in progress labels Jan 10, 2024
@maenkai maenkai closed this as completed Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Bug bugs in IDF
Projects
None yet
Development

No branches or pull requests

4 participants