Thanks, I found this patch in upstream.
2015-03-19 10:57 GMT+08:00 Ethan <ethancsge@xxxxxxxxx>:
> Hi Luiz,
>
> Good news, issue is unable to reproduce with your patch. And may I
> know why struct avrcp *session got invalid address in this case?
> And does this patch will commit to upstream? Thanks.
>
> Regards,
> Ethan
>
> 2015-03-18 14:08 GMT+08:00 Ethan <ethancsge@xxxxxxxxx>:
>> Hi Luiz,
>>
>> Good news, issue is unable to reproduce with your patch. And may I know why
>> struct avrcp *session got invalid address in this case?
>>
>> Regards,
>> Ethan
>>
>> 2015-03-17 19:44 GMT+08:00 Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>:
>>>
>>> Hi Ethan,
>>>
>>> On Tue, Mar 17, 2015 at 12:51 PM, Luiz Augusto von Dentz
>>> <luiz.dentz@xxxxxxxxx> wrote:
>>> > Hi Ethan,
>>> >
>>> > On Tue, Mar 17, 2015 at 12:13 PM, Ethan <ethancsge@xxxxxxxxx> wrote:
>>> >> Hi Luiz,
>>> >>
>>> >> OK, I will follow the rule.
>>> >> And actually, there have three crashes in function as
>>> >> "avrcp_player_value_rsp", "avrcp_get_play_status_rsp" and
>>> >> "avrcp_get_element_attributes_rsp". I tried to mark all code of
>>> >> function
>>> >> "avrcp_get_play_status_rsp" and return FALSE while finding first crash.
>>> >> Then I built bluetoothd, and it crashed again in
>>> >> avrcp_get_play_status_rsp.
>>> >> The same way, next crash is in avrcp_get_element_attributes_rsp.
>>> >>
>>> >> I traced code and check issue log as attached message file, it seems
>>> >> that
>>> >> code "struct avrcp *session = user_data;" get invalid address in
>>> >> function
>>> >> avrcp_get_capabilities_resp. Also, I tried to create a same type
>>> >> structure
>>> >> and assign to session as below, and issue can not be reproduced. Hope
>>> >> these
>>> >> information can help you to find root cause. Thanks.
>>> >>
>>> >> --- a/profiles/audio/avrcp.c
>>> >> +++ b/profiles/audio/avrcp.c
>>> >> @@ -3222,10 +3222,12 @@ static gboolean
>>> >> avrcp_get_capabilities_resp(struct
>>> >> avctp *conn,
>>> >> uint8_t *operands, size_t operand_count,
>>> >> void *user_data)
>>> >> {
>>> >> - struct avrcp *session = user_data;
>>> >> + struct avrcp test;
>>> >> + struct avrcp *session = &test;
>>> >>
>>> >>
>>> >> The attached file is backtrace for three crash by GDB
>>> >>
>>> >>
>>> >> static gboolean avrcp_get_play_status_rsp(struct avctp *conn,
>>> >> uint8_t code, uint8_t subunit,
>>> >> uint8_t *operands, size_t operand_count,
>>> >> void *user_data)
>>> >> {
>>> >> struct avrcp *session = user_data;
>>> >> struct avrcp_player *player = session->controller->player;
>>> >> struct media_player *mp = player->user_data; /*
>>> >> --->crash */
>>> >>
>>> >>
>>> >>
>>> >> static gboolean avrcp_get_element_attributes_rsp(struct avctp *conn,
>>> >> uint8_t code, uint8_t subunit,
>>> >> uint8_t *operands,
>>> >> size_t operand_count,
>>> >> void *user_data)
>>> >> {
>>> >> struct avrcp *session = user_data;
>>> >> struct avrcp_player *player = session->controller->player; /*
>>> >> --->crash */
>>> >>
>>> >> static gboolean avrcp_player_value_rsp(struct avctp *conn,
>>> >> uint8_t code, uint8_t subunit,
>>> >> uint8_t *operands, size_t operand_count,
>>> >> void *user_data)
>>> >> {
>>> >> struct avrcp *session = user_data;
>>> >> struct avrcp_player *player = session->controller->player;
>>> >> struct media_player *mp = player->user_data; /*
>>> >> --->crash */
>>> >>
>>> >>
>>> >> 2015-03-17T20:52:23.347640+11:00 DEBUG bluetoothd[21717]:
>>> >> profiles/audio/avctp.c:avctp_connect_cb() AVCTP: connected to
>>> >> A0:B4:A5:1F:56:B9
>>> >> 2015-03-17T20:52:23.348292+11:00 DEBUG bluetoothd[21717]:
>>> >> profiles/audio/avctp.c:init_uinput() AVRCP: uinput initialized for
>>> >> A0:B4:A5:1F:56:B9
>>> >> 2015-03-17T20:52:23.348337+11:00 DEBUG bluetoothd[21717]:
>>> >> profiles/audio/avrcp.c:target_init() 0x7f601c964a20 version 0x0105
>>> >
>>> > Here seems to be the problem, it seems we only are initiating the
>>> > target not the controller, which should be fine except that the remote
>>> > will not be able to qualify with support of absolute volume control
>>> > since that requires both records. Anyway there is no reason for us to
>>> > crash even if the remote device is doing some strange stuff, we might
>>> > need to check if controller is not initialized just volume control
>>> > should be enabled.
>>>
>>> Could you try with these changes:
>>>
>>> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
>>> index 414ee25..cc26eed 100644
>>> --- a/profiles/audio/avrcp.c
>>> +++ b/profiles/audio/avrcp.c
>>> @@ -3252,12 +3252,18 @@ static gboolean
>>> avrcp_get_capabilities_resp(struct avctp *conn,
>>> case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
>>> case AVRCP_EVENT_UIDS_CHANGED:
>>> case AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED:
>>> + /* These events above are controller specific */
>>> + if (!session->controller)
>>> + break;
>>> case AVRCP_EVENT_VOLUME_CHANGED:
>>> avrcp_register_notification(session, event);
>>> break;
>>> }
>>> }
>>>
>>> + if (!session->controller)
>>> + return FALSE;
>>> +
>>> if (!(events & (1 << AVRCP_EVENT_SETTINGS_CHANGED)))
>>> avrcp_list_player_attributes(session);
>>>
>>>
>>>
>>> --
>>> Luiz Augusto von Dentz
>>
>>
>>
--
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
