Hi Szymon,
On Mon, Dec 22, 2014 at 02:13:53PM +0100, Szymon Janc wrote:
> Hi Andrei,
>
> On Monday 22 of December 2014 13:49:13 Andrei Emeltchenko wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@xxxxxxxxx>
> >
> > Fixes use after free memory bug.
> > req is assigned to user_data and then freed with destroy_gatt_req(req)
> > ---
> > android/scpp.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/android/scpp.c b/android/scpp.c
> > index 77f48cd..9f60c9f 100644
> > --- a/android/scpp.c
> > +++ b/android/scpp.c
> > @@ -301,8 +301,6 @@ static void refresh_discovered_cb(uint8_t status, GSList
> > *chars, uint16_t start, end;
> > bt_uuid_t uuid;
> >
> > - destroy_gatt_req(req);
> > -
> > if (status) {
> > error("Scan Refresh %s", att_ecode2str(status));
> > return;
> > @@ -329,6 +327,8 @@ static void refresh_discovered_cb(uint8_t status, GSList
> > *chars,
> >
> > discover_desc(scan, scan->attrib, start, end, &uuid,
> > discover_descriptor_cb, user_data);
> > +
> > + destroy_gatt_req(req);
> > }
> >
> > static void iwin_discovered_cb(uint8_t status, GSList *chars, void
> > *user_data)
>
> This patch doesn't fix the bug (actually it introduces some memleaks).
>
> From what I see in code gatt_request is packing userdata passed to
> discover_desc() so I think fix should be like:
>
> --- a/android/scpp.c
> +++ b/android/scpp.c
> @@ -328,7 +328,7 @@ static void refresh_discovered_cb(uint8_t status, GSList
> *chars,
> bt_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
>
> discover_desc(scan, scan->attrib, start, end, &uuid,
> - discover_descriptor_cb, user_data);
> + discover_descriptor_cb, scan);
This would access wrong memory since scan = req->user_data and req is
freed as I mentioned in the patch above.
Best regards
Andrei Emeltchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
