2011-10-06 11:49:44

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH 1/2] AVRCP: fix loop over number of application settings

---
audio/avrcp.c | 13 +++++--------
1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index 96a0d36..c36af5d 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -715,12 +715,11 @@ static uint8_t avrcp_handle_set_player_value(struct avrcp_player *player,
{
uint16_t len = ntohs(pdu->params_len);
unsigned int i;
+ uint8_t *param;

- if (len < 3)
+ if (len < 3 || len > 2 * pdu->params[0] + 1U)
goto err;

- len = 0;
-
/*
* From sec. 5.7 of AVRCP 1.3 spec, we should igore non-existent IDs
* and set the existent ones. Sec. 5.2.4 is not clear however how to
@@ -728,11 +727,9 @@ static uint8_t avrcp_handle_set_player_value(struct avrcp_player *player,
* attribute is valid, we respond with no parameters. Otherwise an
* E_INVALID_PARAM is sent.
*/
- for (i = 1; i <= pdu->params[0]; i += 2) {
- uint8_t attr = pdu->params[i];
- uint8_t val = pdu->params[i + 1];
-
- if (player_set_attribute(player, attr, val) < 0)
+ for (len = 0, i = 0, param = &pdu->params[1]; i < pdu->params[0];
+ i++, param += 2) {
+ if (player_set_attribute(player, param[0], param[1]) < 0)
continue;

len++;
--
1.7.7



2011-10-06 12:38:20

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 1/2] AVRCP: fix loop over number of application settings

Hi Lucas,

On Thu, Oct 06, 2011, Lucas De Marchi wrote:
> ---
> audio/avrcp.c | 13 +++++--------
> 1 files changed, 5 insertions(+), 8 deletions(-)

Both patches applied, but I left out the media-api.txt change in the
second patch since it's not related and should be separate.

Johan

2011-10-06 12:21:15

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 2/2] AVRCP: do not allow CT to set a property not supported

Hi Lucas,

On Thu, Oct 6, 2011 at 2:49 PM, Lucas De Marchi
<[email protected]> wrote:
> If player never set a property, it means it doesn't support it. Doesn't
> allow the remote side to set it and send a REJECTED response.
> ---
> ?audio/media.c ? ? | ? ?3 +++
> ?doc/media-api.txt | ? ?2 +-
> ?2 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/audio/media.c b/audio/media.c
> index 9057d70..8d7b65b 100644
> --- a/audio/media.c
> +++ b/audio/media.c
> @@ -1126,6 +1126,9 @@ static int set_setting(uint8_t attr, uint8_t val, void *user_data)
> ? ? ? ?if (property == NULL || value == NULL)
> ? ? ? ? ? ? ? ?return -EINVAL;
>
> + ? ? ? if (!g_hash_table_lookup(mp->settings, GUINT_TO_POINTER(attr)))
> + ? ? ? ? ? ? ? return -EINVAL;
> +
> ? ? ? ?msg = dbus_message_new_method_call(mp->sender, mp->path,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?MEDIA_PLAYER_INTERFACE,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"SetProperty");
> diff --git a/doc/media-api.txt b/doc/media-api.txt
> index af4cfa0..7dc7661 100644
> --- a/doc/media-api.txt
> +++ b/doc/media-api.txt
> @@ -133,7 +133,7 @@ Object path freely definable
> ?Methods ? ? ? ? ? ? ? ?void SetProperty(string property, variant value)
>
> ? ? ? ? ? ? ? ? ? ? ? ?Changes the value of the specified property. Only
> - ? ? ? ? ? ? ? ? ? ? ? properties that are listed a read-write can be changed.
> + ? ? ? ? ? ? ? ? ? ? ? properties that are listed as read-write can be changed.
>
> ? ? ? ? ? ? ? ? ? ? ? ?On success this will emit a PropertyChanged signal.
>
> --
> 1.7.7

Ack.

--
Luiz Augusto von Dentz

2011-10-06 12:19:40

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/2] AVRCP: fix loop over number of application settings

Hi Lucas,

On Thu, Oct 6, 2011 at 2:49 PM, Lucas De Marchi
<[email protected]> wrote:
> ---
> ?audio/avrcp.c | ? 13 +++++--------
> ?1 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index 96a0d36..c36af5d 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -715,12 +715,11 @@ static uint8_t avrcp_handle_set_player_value(struct avrcp_player *player,
> ?{
> ? ? ? ?uint16_t len = ntohs(pdu->params_len);
> ? ? ? ?unsigned int i;
> + ? ? ? uint8_t *param;
>
> - ? ? ? if (len < 3)
> + ? ? ? if (len < 3 || len > 2 * pdu->params[0] + 1U)
> ? ? ? ? ? ? ? ?goto err;
>
> - ? ? ? len = 0;
> -
> ? ? ? ?/*
> ? ? ? ? * From sec. 5.7 of AVRCP 1.3 spec, we should igore non-existent IDs
> ? ? ? ? * and set the existent ones. Sec. 5.2.4 is not clear however how to
> @@ -728,11 +727,9 @@ static uint8_t avrcp_handle_set_player_value(struct avrcp_player *player,
> ? ? ? ? * attribute is valid, we respond with no parameters. Otherwise an
> ? ? ? ? * E_INVALID_PARAM is sent.
> ? ? ? ? */
> - ? ? ? for (i = 1; i <= pdu->params[0]; i += 2) {
> - ? ? ? ? ? ? ? uint8_t attr = pdu->params[i];
> - ? ? ? ? ? ? ? uint8_t val = pdu->params[i + 1];
> -
> - ? ? ? ? ? ? ? if (player_set_attribute(player, attr, val) < 0)
> + ? ? ? for (len = 0, i = 0, param = &pdu->params[1]; i < pdu->params[0];
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? i++, param += 2) {
> + ? ? ? ? ? ? ? if (player_set_attribute(player, param[0], param[1]) < 0)
> ? ? ? ? ? ? ? ? ? ? ? ?continue;
>
> ? ? ? ? ? ? ? ?len++;
> --
> 1.7.7

Ack.

--
Luiz Augusto von Dentz

2011-10-06 11:49:45

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH 2/2] AVRCP: do not allow CT to set a property not supported

If player never set a property, it means it doesn't support it. Doesn't
allow the remote side to set it and send a REJECTED response.
---
audio/media.c | 3 +++
doc/media-api.txt | 2 +-
2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/audio/media.c b/audio/media.c
index 9057d70..8d7b65b 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -1126,6 +1126,9 @@ static int set_setting(uint8_t attr, uint8_t val, void *user_data)
if (property == NULL || value == NULL)
return -EINVAL;

+ if (!g_hash_table_lookup(mp->settings, GUINT_TO_POINTER(attr)))
+ return -EINVAL;
+
msg = dbus_message_new_method_call(mp->sender, mp->path,
MEDIA_PLAYER_INTERFACE,
"SetProperty");
diff --git a/doc/media-api.txt b/doc/media-api.txt
index af4cfa0..7dc7661 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -133,7 +133,7 @@ Object path freely definable
Methods void SetProperty(string property, variant value)

Changes the value of the specified property. Only
- properties that are listed a read-write can be changed.
+ properties that are listed as read-write can be changed.

On success this will emit a PropertyChanged signal.

--
1.7.7