BLE Host - Free mbufs on exec-write-failure This was an mbuf leak. If the device receives an exec-write request, and the attribute callback fails (or the attribute doesn't allow writes at all), the mbufs holding the data intended to be written to that attribute never got freed.
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/a2d04082 Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/a2d04082 Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/a2d04082 Branch: refs/heads/phyrx_no_mbuf Commit: a2d04082bb1bdb84f14bdb33546162ce5856f3d5 Parents: ab25d84 Author: Christopher Collins <[email protected]> Authored: Sat Aug 6 16:04:17 2016 -0700 Committer: William San Filippo <[email protected]> Committed: Thu Aug 11 14:26:25 2016 -0700 ---------------------------------------------------------------------- net/nimble/host/src/ble_att_svr.c | 6 ++++-- net/nimble/host/src/ble_gattc.c | 8 ++++++++ net/nimble/host/src/test/ble_att_svr_test.c | 14 +++++++++++++- 3 files changed, 25 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/a2d04082/net/nimble/host/src/ble_att_svr.c ---------------------------------------------------------------------- diff --git a/net/nimble/host/src/ble_att_svr.c b/net/nimble/host/src/ble_att_svr.c index 2e1f158..b3dd2f3 100644 --- a/net/nimble/host/src/ble_att_svr.c +++ b/net/nimble/host/src/ble_att_svr.c @@ -2347,9 +2347,11 @@ ble_att_svr_prep_write(uint16_t conn_handle, *err_handle = attr_handle; } else { rc = ble_att_svr_write(conn_handle, attr, 0, &om, &att_err); + os_mbuf_free_chain(om); + if (rc != 0) { + return att_err; + } } - - os_mbuf_free_chain(om); } return 0; http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/a2d04082/net/nimble/host/src/ble_gattc.c ---------------------------------------------------------------------- diff --git a/net/nimble/host/src/ble_gattc.c b/net/nimble/host/src/ble_gattc.c index 84ecff3..67c13d6 100644 --- a/net/nimble/host/src/ble_gattc.c +++ b/net/nimble/host/src/ble_gattc.c @@ -177,6 +177,7 @@ struct ble_gattc_proc { struct { struct ble_gatt_attr attr; uint16_t length; + uint8_t exec_sent:1; ble_gatt_attr_fn *cb; void *cb_arg; } write_long; @@ -186,6 +187,7 @@ struct ble_gattc_proc { uint8_t num_attrs; uint8_t cur_attr; uint16_t length; + uint8_t exec_sent:1; ble_gatt_reliable_attr_fn *cb; void *cb_arg; } write_reliable; @@ -3528,6 +3530,12 @@ static int ble_gattc_write_long_rx_exec(struct ble_gattc_proc *proc, int status) { ble_gattc_dbg_assert_proc_not_inserted(proc); + + /* Ignore the response if we haven't sent the corresponding request yet. */ + if (!proc->write_long.exec_sent) { + return 0; + } + ble_gattc_write_long_cb(proc, status, 0); return BLE_HS_EDONE; } http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/a2d04082/net/nimble/host/src/test/ble_att_svr_test.c ---------------------------------------------------------------------- diff --git a/net/nimble/host/src/test/ble_att_svr_test.c b/net/nimble/host/src/test/ble_att_svr_test.c index e5968d5..09e7bf9 100644 --- a/net/nimble/host/src/test/ble_att_svr_test.c +++ b/net/nimble/host/src/test/ble_att_svr_test.c @@ -2083,12 +2083,16 @@ TEST_CASE(ble_att_svr_test_prep_write) data[i] = i; } - /* Register two attributes. */ + /* Register two writable attributes. */ ble_att_svr_test_misc_register_uuid16(0x1234, HA_FLAG_PERM_RW, 1, ble_att_svr_test_misc_attr_fn_w_1); ble_att_svr_test_misc_register_uuid16(0x8989, HA_FLAG_PERM_RW, 2, ble_att_svr_test_misc_attr_fn_w_2); + /* Register a third attribute that is not writable. */ + ble_att_svr_test_misc_register_uuid16(0xabab, BLE_ATT_F_READ, 3, + ble_att_svr_test_misc_attr_fn_r_1); + /*** Empty write succeeds. */ ble_att_svr_test_misc_exec_write(conn_handle, BLE_ATT_EXEC_WRITE_F_CONFIRM, 0, 0); @@ -2132,6 +2136,14 @@ TEST_CASE(ble_att_svr_test_prep_write) BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN, 1); ble_att_svr_test_misc_verify_w_1(NULL, 0); + /*** Failure due to attribute callback. */ + ble_att_svr_test_misc_prep_write(conn_handle, 3, 0, data, 35, 0); + ble_att_svr_test_misc_prep_write(conn_handle, 3, 35, data + 35, 43, 0); + ble_att_svr_test_misc_prep_write(conn_handle, 3, 78, data + 78, 1, 0); + ble_att_svr_test_misc_exec_write(conn_handle, BLE_ATT_EXEC_WRITE_F_CONFIRM, + BLE_ATT_ERR_WRITE_NOT_PERMITTED, 0); + ble_att_svr_test_misc_verify_w_1(NULL, 0); + /*** Successful two part write. */ ble_att_svr_test_misc_prep_write(conn_handle, 1, 0, data, 20, 0); ble_att_svr_test_misc_prep_write(conn_handle, 1, 20, data + 20, 20, 0);
