Return-Path: Message-id: <8BF3576D9B1446A3AF8908F58AF314CE@sisodomain.com> From: Jaganath Kanakkassery To: Jaganath Kanakkassery , Anderson Lizardo Cc: linux-bluetooth@vger.kernel.org References: <1362402872-13080-1-git-send-email-jaganath.k@samsung.com> <1362402872-13080-2-git-send-email-jaganath.k@samsung.com> In-reply-to: Subject: Re: [PATCH 2/3] attrib: Fix use after free of attrib Date: Fri, 08 Mar 2013 17:33:14 +0530 MIME-version: 1.0 Content-type: text/plain; format=flowed; charset=iso-8859-1; reply-type=response Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi, -------------------------------------------------- From: "Jaganath Kanakkassery" Sent: Monday, March 04, 2013 7:50 PM To: "Anderson Lizardo" Cc: Subject: Re: [PATCH 2/3] attrib: Fix use after free of attrib > Hi Anderson, > > -------------------------------------------------- > From: "Anderson Lizardo" > Sent: Monday, March 04, 2013 7:14 PM > To: "Jaganath Kanakkassery" > Cc: > Subject: Re: [PATCH 2/3] attrib: Fix use after free of attrib > >> Hi Jaganath, >> >> On Mon, Mar 4, 2013 at 9:14 AM, Jaganath Kanakkassery >> wrote: >>> If attrib is freed in cmd->func(), then it will be used if either >>> request or response queue has some data to send. >> >> As far as I know, attrib was not supposed to be freed on the >> cmd->func() callback. Do you have an example/testcase where this is >> happening? To me, looks like a refcount issue (i.e. g_attrib_unref() >> is deleting the attrib because a reference was not properly kept where >> necessary). >> > > I got this issue in BlueZ 4.101 when I called "DiscoverCharacteristics". > "GetProperty" and "SetProperty" of characteristics. Please see the logs > > daemon.debug bluetoothd[4567]: src/adapter.c:adapter_get_device() > E8:E8:66:E0:C0:01 > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_ref() 0xb6f47808: > ref=1 > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_ref() 0xb6f47808: > ref=2 > daemon.debug bluetoothd[4567]: src/device.c:btd_device_ref() 0xb6f47268: > ref=9 > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_ref() 0xb6f47808: > ref=3 > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_ref() 0xb6f47808: > ref=4 > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_ref() 0xb6f47808: > ref=5 > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_unref() > 0xb6f47808: ref=4 > daemon.debug bluetoothd[4567]: attrib/client.c:register_characteristic() > Registered: > /org/bluez/4567/hci0/dev_E8_E8_66_E0_C0_01/service0012/characteristic0014 > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_ref() 0xb6f47808: > ref=5 > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_unref() > 0xb6f47808: ref=4 > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_unref() > 0xb6f47808: ref=3 > daemon.debug bluetoothd[4567]: gdbus/object.c:generic_message() /: > org.bluez.Manager.DefaultAdapter() > daemon.debug bluetoothd[4567]: gdbus/object.c:generic_message() > /org/bluez/4567/hci0/dev_E8_E8_66_E0_C0_01/service0012/characteristic0014: > org.bluez.Characteristic.GetProperties() > daemon.debug bluetoothd[4567]: gdbus/object.c:generic_message() /: > org.bluez.Manager.DefaultAdapter() > daemon.debug bluetoothd[4567]: gdbus/object.c:generic_message() > /org/bluez/4567/hci0/dev_E8_E8_66_E0_C0_01/service0012/characteristic0014: > org.bluez.Characteristic.SetProperty() > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_ref() 0xb6f47808: > ref=4 > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_unref() > 0xb6f47808: ref=3 > daemon.debug bluetoothd[4567]: src/device.c:btd_device_unref() 0xb6f47268: > ref=8 > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_unref() > 0xb6f47808: ref=2 > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_unref() > 0xb6f47808: ref=1 > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_unref() > 0xb6f47808: ref=0 > daemon.debug bluetoothd[4567]: attrib/gattrib.c:g_attrib_ref() 0xb6f47808: > ref=1 > daemon.err bluetoothd[4567]: crashed [1357010043] processname=bluetoothd, > pid=4567, tid=4567, signal=11 > > I did not give this log in commit since it is based on 4.101 > > What I understood is cmd->func() is doing g_attrib_unref() which frees the > attrib > during last unref. Then since data is there in queue it tries to ref in > wake_up_sender(). > Ping. Thanks, Jaganath