* [PATCH 1/3] avrcp: Avoid unneeded calculation
@ 2014-01-30 16:12 Andrei Emeltchenko
2014-01-30 16:12 ` [PATCH 2/3] avrcp: Fix using incorrect buffer for SetVolume Andrei Emeltchenko
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Andrei Emeltchenko @ 2014-01-30 16:12 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
There no need to calculate those values
---
profiles/audio/avrcp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index fa5adab..2e1a940 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -3177,7 +3177,7 @@ static void avrcp_register_notification(struct avrcp *session, uint8_t event)
pdu->params[0] = event;
pdu->params_len = htons(AVRCP_REGISTER_NOTIFICATION_PARAM_LENGTH);
- length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
+ length = AVRCP_HEADER_LENGTH + AVRCP_REGISTER_NOTIFICATION_PARAM_LENGTH;
avctp_send_vendordep_req(session->conn, AVC_CTYPE_NOTIFY,
AVC_SUBUNIT_PANEL, buf, length,
@@ -3250,7 +3250,7 @@ static void avrcp_get_capabilities(struct avrcp *session)
pdu->params[0] = CAP_EVENTS_SUPPORTED;
pdu->params_len = htons(AVRCP_GET_CAPABILITIES_PARAM_LENGTH);
- length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
+ length = AVRCP_HEADER_LENGTH + AVRCP_GET_CAPABILITIES_PARAM_LENGTH;
avctp_send_vendordep_req(session->conn, AVC_CTYPE_STATUS,
AVC_SUBUNIT_PANEL, buf, length,
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/3] avrcp: Fix using incorrect buffer for SetVolume 2014-01-30 16:12 [PATCH 1/3] avrcp: Avoid unneeded calculation Andrei Emeltchenko @ 2014-01-30 16:12 ` Andrei Emeltchenko 2014-02-04 14:08 ` Andrei Emeltchenko 2014-01-30 16:12 ` [PATCH 3/3] avrcp: Fix possible buffer overflow and correct length Andrei Emeltchenko 2014-02-04 11:55 ` [PATCH 1/3] avrcp: Avoid unneeded calculation Andrei Emeltchenko 2 siblings, 1 reply; 10+ messages in thread From: Andrei Emeltchenko @ 2014-01-30 16:12 UTC (permalink / raw) To: linux-bluetooth From: Andrei Emeltchenko <andrei.emeltchenko@intel.com> The command requires one parameter. --- profiles/audio/avrcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c index 2e1a940..128f7d3 100644 --- a/profiles/audio/avrcp.c +++ b/profiles/audio/avrcp.c @@ -3706,7 +3706,7 @@ int avrcp_set_volume(struct btd_device *dev, uint8_t volume) { struct avrcp_server *server; struct avrcp *session; - uint8_t buf[AVRCP_HEADER_LENGTH + 2]; + uint8_t buf[AVRCP_HEADER_LENGTH + 1]; struct avrcp_header *pdu = (void *) buf; server = find_server(servers, device_get_adapter(dev)); -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] avrcp: Fix using incorrect buffer for SetVolume 2014-01-30 16:12 ` [PATCH 2/3] avrcp: Fix using incorrect buffer for SetVolume Andrei Emeltchenko @ 2014-02-04 14:08 ` Andrei Emeltchenko 2014-02-11 11:15 ` Andrei Emeltchenko 0 siblings, 1 reply; 10+ messages in thread From: Andrei Emeltchenko @ 2014-02-04 14:08 UTC (permalink / raw) To: linux-bluetooth On Thu, Jan 30, 2014 at 06:12:55PM +0200, Andrei Emeltchenko wrote: > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com> > > The command requires one parameter. ping > --- > profiles/audio/avrcp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c > index 2e1a940..128f7d3 100644 > --- a/profiles/audio/avrcp.c > +++ b/profiles/audio/avrcp.c > @@ -3706,7 +3706,7 @@ int avrcp_set_volume(struct btd_device *dev, uint8_t volume) > { > struct avrcp_server *server; > struct avrcp *session; > - uint8_t buf[AVRCP_HEADER_LENGTH + 2]; > + uint8_t buf[AVRCP_HEADER_LENGTH + 1]; > struct avrcp_header *pdu = (void *) buf; > > server = find_server(servers, device_get_adapter(dev)); > -- > 1.8.3.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] avrcp: Fix using incorrect buffer for SetVolume 2014-02-04 14:08 ` Andrei Emeltchenko @ 2014-02-11 11:15 ` Andrei Emeltchenko 2014-02-11 18:37 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 10+ messages in thread From: Andrei Emeltchenko @ 2014-02-11 11:15 UTC (permalink / raw) To: linux-bluetooth On Tue, Feb 04, 2014 at 04:08:17PM +0200, Andrei Emeltchenko wrote: > On Thu, Jan 30, 2014 at 06:12:55PM +0200, Andrei Emeltchenko wrote: > > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com> > > > > The command requires one parameter. > > ping ping > > > --- > > profiles/audio/avrcp.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c > > index 2e1a940..128f7d3 100644 > > --- a/profiles/audio/avrcp.c > > +++ b/profiles/audio/avrcp.c > > @@ -3706,7 +3706,7 @@ int avrcp_set_volume(struct btd_device *dev, uint8_t volume) > > { > > struct avrcp_server *server; > > struct avrcp *session; > > - uint8_t buf[AVRCP_HEADER_LENGTH + 2]; > > + uint8_t buf[AVRCP_HEADER_LENGTH + 1]; > > struct avrcp_header *pdu = (void *) buf; > > > > server = find_server(servers, device_get_adapter(dev)); > > -- > > 1.8.3.2 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] avrcp: Fix using incorrect buffer for SetVolume 2014-02-11 11:15 ` Andrei Emeltchenko @ 2014-02-11 18:37 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 10+ messages in thread From: Luiz Augusto von Dentz @ 2014-02-11 18:37 UTC (permalink / raw) To: Andrei Emeltchenko, linux-bluetooth@vger.kernel.org Hi Andrei, On Tue, Feb 11, 2014 at 1:15 PM, Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> wrote: > On Tue, Feb 04, 2014 at 04:08:17PM +0200, Andrei Emeltchenko wrote: >> On Thu, Jan 30, 2014 at 06:12:55PM +0200, Andrei Emeltchenko wrote: >> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com> >> > >> > The command requires one parameter. >> >> ping > > ping > >> >> > --- >> > profiles/audio/avrcp.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c >> > index 2e1a940..128f7d3 100644 >> > --- a/profiles/audio/avrcp.c >> > +++ b/profiles/audio/avrcp.c >> > @@ -3706,7 +3706,7 @@ int avrcp_set_volume(struct btd_device *dev, uint8_t volume) >> > { >> > struct avrcp_server *server; >> > struct avrcp *session; >> > - uint8_t buf[AVRCP_HEADER_LENGTH + 2]; >> > + uint8_t buf[AVRCP_HEADER_LENGTH + 1]; >> > struct avrcp_header *pdu = (void *) buf; >> > >> > server = find_server(servers, device_get_adapter(dev)); >> > -- >> > 1.8.3.2 Ive applied this one, I leave 1/3 for when we have the code properly decoupled and patch 3/3 I end up redoing most of it to check for invalid attributes. -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] avrcp: Fix possible buffer overflow and correct length 2014-01-30 16:12 [PATCH 1/3] avrcp: Avoid unneeded calculation Andrei Emeltchenko 2014-01-30 16:12 ` [PATCH 2/3] avrcp: Fix using incorrect buffer for SetVolume Andrei Emeltchenko @ 2014-01-30 16:12 ` Andrei Emeltchenko 2014-02-04 14:08 ` Andrei Emeltchenko 2014-02-04 11:55 ` [PATCH 1/3] avrcp: Avoid unneeded calculation Andrei Emeltchenko 2 siblings, 1 reply; 10+ messages in thread From: Andrei Emeltchenko @ 2014-01-30 16:12 UTC (permalink / raw) To: linux-bluetooth From: Andrei Emeltchenko <andrei.emeltchenko@intel.com> Wrong length was given and it was also possible to crash. --- profiles/audio/avrcp.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c index 128f7d3..f9fce5c 100644 --- a/profiles/audio/avrcp.c +++ b/profiles/audio/avrcp.c @@ -1899,8 +1899,12 @@ static void avrcp_get_current_player_value(struct avrcp *session, { uint8_t buf[AVRCP_HEADER_LENGTH + 5]; struct avrcp_header *pdu = (void *) buf; + uint16_t length = AVRCP_HEADER_LENGTH + count + 1; int i; + if (count + 1 > 5) + return; + memset(buf, 0, sizeof(buf)); set_company_id(pdu->company_id, IEEEID_BTSIG); @@ -1913,7 +1917,7 @@ static void avrcp_get_current_player_value(struct avrcp *session, pdu->params[i + 1] = attrs[i]; avctp_send_vendordep_req(session->conn, AVC_CTYPE_STATUS, - AVC_SUBUNIT_PANEL, buf, sizeof(buf), + AVC_SUBUNIT_PANEL, buf, length, avrcp_player_value_rsp, session); } -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] avrcp: Fix possible buffer overflow and correct length 2014-01-30 16:12 ` [PATCH 3/3] avrcp: Fix possible buffer overflow and correct length Andrei Emeltchenko @ 2014-02-04 14:08 ` Andrei Emeltchenko 0 siblings, 0 replies; 10+ messages in thread From: Andrei Emeltchenko @ 2014-02-04 14:08 UTC (permalink / raw) To: linux-bluetooth On Thu, Jan 30, 2014 at 06:12:56PM +0200, Andrei Emeltchenko wrote: > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com> > > Wrong length was given and it was also possible to crash. ping > --- > profiles/audio/avrcp.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c > index 128f7d3..f9fce5c 100644 > --- a/profiles/audio/avrcp.c > +++ b/profiles/audio/avrcp.c > @@ -1899,8 +1899,12 @@ static void avrcp_get_current_player_value(struct avrcp *session, > { > uint8_t buf[AVRCP_HEADER_LENGTH + 5]; > struct avrcp_header *pdu = (void *) buf; > + uint16_t length = AVRCP_HEADER_LENGTH + count + 1; > int i; > > + if (count + 1 > 5) > + return; > + > memset(buf, 0, sizeof(buf)); > > set_company_id(pdu->company_id, IEEEID_BTSIG); > @@ -1913,7 +1917,7 @@ static void avrcp_get_current_player_value(struct avrcp *session, > pdu->params[i + 1] = attrs[i]; > > avctp_send_vendordep_req(session->conn, AVC_CTYPE_STATUS, > - AVC_SUBUNIT_PANEL, buf, sizeof(buf), > + AVC_SUBUNIT_PANEL, buf, length, > avrcp_player_value_rsp, session); > } > > -- > 1.8.3.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] avrcp: Avoid unneeded calculation 2014-01-30 16:12 [PATCH 1/3] avrcp: Avoid unneeded calculation Andrei Emeltchenko 2014-01-30 16:12 ` [PATCH 2/3] avrcp: Fix using incorrect buffer for SetVolume Andrei Emeltchenko 2014-01-30 16:12 ` [PATCH 3/3] avrcp: Fix possible buffer overflow and correct length Andrei Emeltchenko @ 2014-02-04 11:55 ` Andrei Emeltchenko 2014-02-04 12:49 ` Luiz Augusto von Dentz 2 siblings, 1 reply; 10+ messages in thread From: Andrei Emeltchenko @ 2014-02-04 11:55 UTC (permalink / raw) To: linux-bluetooth On Thu, Jan 30, 2014 at 06:12:54PM +0200, Andrei Emeltchenko wrote: > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com> > > There no need to calculate those values ping > --- > profiles/audio/avrcp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c > index fa5adab..2e1a940 100644 > --- a/profiles/audio/avrcp.c > +++ b/profiles/audio/avrcp.c > @@ -3177,7 +3177,7 @@ static void avrcp_register_notification(struct avrcp *session, uint8_t event) > pdu->params[0] = event; > pdu->params_len = htons(AVRCP_REGISTER_NOTIFICATION_PARAM_LENGTH); > > - length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len); > + length = AVRCP_HEADER_LENGTH + AVRCP_REGISTER_NOTIFICATION_PARAM_LENGTH; > > avctp_send_vendordep_req(session->conn, AVC_CTYPE_NOTIFY, > AVC_SUBUNIT_PANEL, buf, length, > @@ -3250,7 +3250,7 @@ static void avrcp_get_capabilities(struct avrcp *session) > pdu->params[0] = CAP_EVENTS_SUPPORTED; > pdu->params_len = htons(AVRCP_GET_CAPABILITIES_PARAM_LENGTH); > > - length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len); > + length = AVRCP_HEADER_LENGTH + AVRCP_GET_CAPABILITIES_PARAM_LENGTH; > > avctp_send_vendordep_req(session->conn, AVC_CTYPE_STATUS, > AVC_SUBUNIT_PANEL, buf, length, > -- > 1.8.3.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] avrcp: Avoid unneeded calculation 2014-02-04 11:55 ` [PATCH 1/3] avrcp: Avoid unneeded calculation Andrei Emeltchenko @ 2014-02-04 12:49 ` Luiz Augusto von Dentz 2014-02-04 13:05 ` Andrei Emeltchenko 0 siblings, 1 reply; 10+ messages in thread From: Luiz Augusto von Dentz @ 2014-02-04 12:49 UTC (permalink / raw) To: Andrei Emeltchenko, linux-bluetooth@vger.kernel.org Hi Andrei, On Tue, Feb 4, 2014 at 1:55 PM, Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> wrote: > On Thu, Jan 30, 2014 at 06:12:54PM +0200, Andrei Emeltchenko wrote: >> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com> >> >> There no need to calculate those values > > ping > >> --- >> profiles/audio/avrcp.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c >> index fa5adab..2e1a940 100644 >> --- a/profiles/audio/avrcp.c >> +++ b/profiles/audio/avrcp.c >> @@ -3177,7 +3177,7 @@ static void avrcp_register_notification(struct avrcp *session, uint8_t event) >> pdu->params[0] = event; >> pdu->params_len = htons(AVRCP_REGISTER_NOTIFICATION_PARAM_LENGTH); >> >> - length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len); >> + length = AVRCP_HEADER_LENGTH + AVRCP_REGISTER_NOTIFICATION_PARAM_LENGTH; >> >> avctp_send_vendordep_req(session->conn, AVC_CTYPE_NOTIFY, >> AVC_SUBUNIT_PANEL, buf, length, >> @@ -3250,7 +3250,7 @@ static void avrcp_get_capabilities(struct avrcp *session) >> pdu->params[0] = CAP_EVENTS_SUPPORTED; >> pdu->params_len = htons(AVRCP_GET_CAPABILITIES_PARAM_LENGTH); >> >> - length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len); >> + length = AVRCP_HEADER_LENGTH + AVRCP_GET_CAPABILITIES_PARAM_LENGTH; >> >> avctp_send_vendordep_req(session->conn, AVC_CTYPE_STATUS, >> AVC_SUBUNIT_PANEL, buf, length, >> -- >> 1.8.3.2 I will leave this as it for now, we will probably create a avrcp_send helper to have this in common place. -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] avrcp: Avoid unneeded calculation 2014-02-04 12:49 ` Luiz Augusto von Dentz @ 2014-02-04 13:05 ` Andrei Emeltchenko 0 siblings, 0 replies; 10+ messages in thread From: Andrei Emeltchenko @ 2014-02-04 13:05 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org Hi Luiz, On Tue, Feb 04, 2014 at 02:49:02PM +0200, Luiz Augusto von Dentz wrote: > Hi Andrei, > > On Tue, Feb 4, 2014 at 1:55 PM, Andrei Emeltchenko > <Andrei.Emeltchenko.news@gmail.com> wrote: > > On Thu, Jan 30, 2014 at 06:12:54PM +0200, Andrei Emeltchenko wrote: > >> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com> > >> > >> There no need to calculate those values > > > > ping > > > >> --- > >> profiles/audio/avrcp.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c > >> index fa5adab..2e1a940 100644 > >> --- a/profiles/audio/avrcp.c > >> +++ b/profiles/audio/avrcp.c > >> @@ -3177,7 +3177,7 @@ static void avrcp_register_notification(struct avrcp *session, uint8_t event) > >> pdu->params[0] = event; > >> pdu->params_len = htons(AVRCP_REGISTER_NOTIFICATION_PARAM_LENGTH); > >> > >> - length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len); > >> + length = AVRCP_HEADER_LENGTH + AVRCP_REGISTER_NOTIFICATION_PARAM_LENGTH; > >> > >> avctp_send_vendordep_req(session->conn, AVC_CTYPE_NOTIFY, > >> AVC_SUBUNIT_PANEL, buf, length, > >> @@ -3250,7 +3250,7 @@ static void avrcp_get_capabilities(struct avrcp *session) > >> pdu->params[0] = CAP_EVENTS_SUPPORTED; > >> pdu->params_len = htons(AVRCP_GET_CAPABILITIES_PARAM_LENGTH); > >> > >> - length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len); > >> + length = AVRCP_HEADER_LENGTH + AVRCP_GET_CAPABILITIES_PARAM_LENGTH; > >> > >> avctp_send_vendordep_req(session->conn, AVC_CTYPE_STATUS, > >> AVC_SUBUNIT_PANEL, buf, length, > >> -- > >> 1.8.3.2 > > I will leave this as it for now, we will probably create a avrcp_send > helper to have this in common place. OK, though those a = ntohs(b); c = htons(a); looks IMO really ugly Best regards Andrei Emeltchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-02-11 18:37 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-01-30 16:12 [PATCH 1/3] avrcp: Avoid unneeded calculation Andrei Emeltchenko 2014-01-30 16:12 ` [PATCH 2/3] avrcp: Fix using incorrect buffer for SetVolume Andrei Emeltchenko 2014-02-04 14:08 ` Andrei Emeltchenko 2014-02-11 11:15 ` Andrei Emeltchenko 2014-02-11 18:37 ` Luiz Augusto von Dentz 2014-01-30 16:12 ` [PATCH 3/3] avrcp: Fix possible buffer overflow and correct length Andrei Emeltchenko 2014-02-04 14:08 ` Andrei Emeltchenko 2014-02-04 11:55 ` [PATCH 1/3] avrcp: Avoid unneeded calculation Andrei Emeltchenko 2014-02-04 12:49 ` Luiz Augusto von Dentz 2014-02-04 13:05 ` Andrei Emeltchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox