Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e6a3ad8c authored by Mike J. Chen's avatar Mike J. Chen
Browse files

Fix GKI exception of calling free on an already freed buffer 



Various parts of btif_hh.c were creating GKI buffers and
keeping references to them and freeing them in odd and
unnecessary ways.  The buffer is freed by lower levels
of the stack once the buffer has been sent to the chip
at the l2c layer and shouldn't be freed by btif_hh itself
since it's possible to double free, and there could
also be race conditions with other threads already processing
the buffer while the reference is freed if the API calls
are invoked again before the previous invocation was completely
processed.

Also added a helper routine to simplify buffer creation and
initialization.

Change-Id: Ia6039983502e2670b2325d90310244edf843b692
Signed-off-by: default avatarMike J. Chen <mjchen@google.com>
parent a76d1e26

system/btif/include/btif_hh.h

+0 −1
Original line number Diff line number Diff line
@@ -63,7 +63,6 @@ typedef struct 
    UINT8                         sub_class;

    UINT8                         app_id;

    int                           fd;

    BT_HDR                        *p_buf;

    UINT32                        hh_poll_thread_id;

    UINT8                         hh_keep_polling;

    BOOLEAN                       vup_timer_active;

system/btif/src/btif_hh.c

+49 −86
Original line number Diff line number Diff line
@@ -247,6 +247,29 @@ static void toggle_os_keylockstates(int fd, int changedlockstates) 
    bta_hh_co_write(fd , hidreport, sizeof(hidreport));

}



/*******************************************************************************

**

** Function         create_pbuf

**

** Description      Helper function to create p_buf for send_data or set_report

**

*******************************************************************************/

static BT_HDR *create_pbuf(UINT16 len, UINT8 *data)

{

    BT_HDR* p_buf = GKI_getbuf((UINT16) (len + BTA_HH_MIN_OFFSET + sizeof(BT_HDR)));



    if (p_buf) {

        UINT8* pbuf_data;



        p_buf->len = len;

        p_buf->offset = BTA_HH_MIN_OFFSET;



        pbuf_data = (UINT8*) (p_buf + 1) + p_buf->offset;

        memcpy(pbuf_data, data, len);

    }

    return p_buf;

}



/*******************************************************************************

**

** Function         update_keyboard_lockstates
@@ -258,30 +281,20 @@ static void update_keyboard_lockstates(btif_hh_device_t *p_dev) 
{

    UINT8 len = 2;  /* reportid + 1 byte report*/

    BD_ADDR* bda;

    BT_HDR* p_buf;

    UINT8 data[] = {0x01, /* report id */

                    btif_hh_keylockstates}; /* keystate */



    /* Set report for other keyboards */

    BTIF_TRACE_EVENT3("%s: setting report on dev_handle %d to 0x%x",

         __FUNCTION__, p_dev->dev_handle, btif_hh_keylockstates);



    if (p_dev->p_buf != NULL) {

        GKI_freebuf(p_dev->p_buf);

    }

    /* Get SetReport buffer */

    p_dev->p_buf = GKI_getbuf((UINT16) (len + BTA_HH_MIN_OFFSET +

        sizeof(BT_HDR)));

    if (p_dev->p_buf != NULL) {

        p_dev->p_buf->len = len;

        p_dev->p_buf->offset = BTA_HH_MIN_OFFSET;

        p_dev->p_buf->layer_specific = BTA_HH_RPTT_OUTPUT;



        /* LED status updated by data event */

        UINT8 *pbuf_data  = (UINT8 *)(p_dev->p_buf + 1)

            + p_dev->p_buf->offset;

        pbuf_data[0]=0x01; /*report id */

        pbuf_data[1]=btif_hh_keylockstates; /*keystate*/

    p_buf = create_pbuf(len, data);

    if (p_buf != NULL) {

        p_buf->layer_specific = BTA_HH_RPTT_OUTPUT;

        bda = (BD_ADDR*) (&p_dev->bd_addr);

        BTA_HhSendData(p_dev->dev_handle, *bda,

            p_dev->p_buf);

        BTA_HhSendData(p_dev->dev_handle, *bda, p_buf);

    }

}
@@ -545,10 +558,6 @@ void btif_hh_remove_device(bt_bdaddr_t bd_addr) 
    else {

        BTIF_TRACE_WARNING1("%s: device_num = 0", __FUNCTION__);

    }

    if (p_dev->p_buf != NULL) {

        GKI_freebuf(p_dev->p_buf);

        p_dev->p_buf = NULL;

    }



    p_dev->hh_keep_polling = 0;

    p_dev->hh_poll_thread_id = -1;
@@ -720,35 +729,15 @@ void btif_hh_disconnect(bt_bdaddr_t *bd_addr) 
** Returns          void

**

*******************************************************************************/



void btif_hh_setreport(btif_hh_device_t *p_dev, bthh_report_type_t r_type, UINT16 size,

                            UINT8* report)

{

    UINT8  hexbuf[20];

    UINT16 len = size;

    int i = 0;

    if (p_dev->p_buf != NULL) {

        GKI_freebuf(p_dev->p_buf);

    }

    p_dev->p_buf = GKI_getbuf((UINT16) (len + BTA_HH_MIN_OFFSET + sizeof(BT_HDR)));

    if (p_dev->p_buf == NULL) {

        APPL_TRACE_ERROR2("%s: Error, failed to allocate RPT buffer, len = %d", __FUNCTION__, len);

    BT_HDR* p_buf = create_pbuf(size, report);

    if (p_buf == NULL) {

        APPL_TRACE_ERROR2("%s: Error, failed to allocate RPT buffer, size = %d", __FUNCTION__, size);

        return;

    }



    p_dev->p_buf->len = len;

    p_dev->p_buf->offset = BTA_HH_MIN_OFFSET;



    //Build a SetReport data buffer

    memset(hexbuf, 0, 20);

    for(i=0; i<len; i++)

        hexbuf[i] = report[i];



    UINT8* pbuf_data;

    pbuf_data = (UINT8*) (p_dev->p_buf + 1) + p_dev->p_buf->offset;

    memcpy(pbuf_data, hexbuf, len);

    BTA_HhSetReport(p_dev->dev_handle, r_type, p_dev->p_buf);



    BTA_HhSetReport(p_dev->dev_handle, r_type, p_buf);

}



/*****************************************************************************
@@ -897,12 +886,6 @@ static void btif_hh_upstreams_evt(UINT16 event, char* p_param) 
        case BTA_HH_SET_RPT_EVT:

            BTIF_TRACE_DEBUG2("BTA_HH_SET_RPT_EVT: status = %d, handle = %d",

            p_data->dev_status.status, p_data->dev_status.handle);

            p_dev = btif_hh_find_connected_dev_by_handle(p_data->dev_status.handle);

            if (p_dev != NULL && p_dev->p_buf != NULL) {

                BTIF_TRACE_DEBUG0("Freeing buffer..." );

                GKI_freebuf(p_dev->p_buf);

                p_dev->p_buf = NULL;

            }

            break;



        case BTA_HH_GET_PROTO_EVT:
@@ -1630,33 +1613,22 @@ static bt_status_t set_report (bt_bdaddr_t *bd_addr, bthh_report_type_t reportTy 
        UINT8  hexbuf[200];

        UINT16 len = (strlen(report) + 1) / 2;



        if (p_dev->p_buf != NULL) {

            GKI_freebuf(p_dev->p_buf);

        }

        p_dev->p_buf = GKI_getbuf((UINT16) (len + BTA_HH_MIN_OFFSET + sizeof(BT_HDR)));

        if (p_dev->p_buf == NULL) {

            BTIF_TRACE_ERROR2("%s: Error, failed to allocate RPT buffer, len = %d", __FUNCTION__, len);

            return BT_STATUS_FAIL;

        }



        p_dev->p_buf->len = len;

        p_dev->p_buf->offset = BTA_HH_MIN_OFFSET;



        /* Build a SetReport data buffer */

        memset(hexbuf, 0, 200);

        //TODO

        hex_bytes_filled = ascii_2_hex(report, len, hexbuf);

        ALOGI("Hex bytes filled, hex value: %d", hex_bytes_filled);

        if (hex_bytes_filled) {

            UINT8* pbuf_data;

            pbuf_data = (UINT8*) (p_dev->p_buf + 1) + p_dev->p_buf->offset;

            memcpy(pbuf_data, hexbuf, hex_bytes_filled);

            BTA_HhSetReport(p_dev->dev_handle, reportType, p_dev->p_buf);

            BT_HDR* p_buf = create_pbuf(hex_bytes_filled, hexbuf);

            if (p_buf == NULL) {

                BTIF_TRACE_ERROR2("%s: Error, failed to allocate RPT buffer, len = %d",

                                  __FUNCTION__, hex_bytes_filled);

                return BT_STATUS_FAIL;

            }

            BTA_HhSetReport(p_dev->dev_handle, reportType, p_buf);

        }

        return BT_STATUS_SUCCESS;

    }





}



/*******************************************************************************
@@ -1696,29 +1668,20 @@ static bt_status_t send_data (bt_bdaddr_t *bd_addr, char* data) 
        UINT8  hexbuf[200];

        UINT16 len = (strlen(data) + 1) / 2;



        if (p_dev->p_buf != NULL) {

            GKI_freebuf(p_dev->p_buf);

        }

        p_dev->p_buf = GKI_getbuf((UINT16) (len + BTA_HH_MIN_OFFSET + sizeof(BT_HDR)));

        if (p_dev->p_buf == NULL) {

            BTIF_TRACE_ERROR2("%s: Error, failed to allocate RPT buffer, len = %d", __FUNCTION__, len);

            return BT_STATUS_FAIL;

        }



        p_dev->p_buf->len = len;

        p_dev->p_buf->offset = BTA_HH_MIN_OFFSET;



        /* Build a SetReport data buffer */

        memset(hexbuf, 0, 200);

        hex_bytes_filled = ascii_2_hex(data, len, hexbuf);

        BTIF_TRACE_ERROR2("Hex bytes filled, hex value: %d, %d", hex_bytes_filled, len);



        if (hex_bytes_filled) {

            UINT8* pbuf_data;

            pbuf_data = (UINT8*) (p_dev->p_buf + 1) + p_dev->p_buf->offset;

            memcpy(pbuf_data, hexbuf, hex_bytes_filled);

            p_dev->p_buf->layer_specific = BTA_HH_RPTT_OUTPUT;

            BTA_HhSendData(p_dev->dev_handle, *bda, p_dev->p_buf);

            BT_HDR* p_buf = create_pbuf(hex_bytes_filled, hexbuf);

            if (p_buf == NULL) {

                BTIF_TRACE_ERROR2("%s: Error, failed to allocate RPT buffer, len = %d",

                                  __FUNCTION__, hex_bytes_filled);

                return BT_STATUS_FAIL;

            }

            p_buf->layer_specific = BTA_HH_RPTT_OUTPUT;

            BTA_HhSendData(p_dev->dev_handle, *bda, p_buf);

            return BT_STATUS_SUCCESS;

        }