[alsa-devel] [PATCH] ALSA: firewire-lib/bebob/oxfw: improve response evaluation for AV/C commands
Takashi Iwai
tiwai at suse.de
Wed Apr 5 21:38:49 CEST 2017
On Mon, 03 Apr 2017 14:13:55 +0200,
Takashi Sakamoto wrote:
>
> In ALSA firewire stack, some AV/C commands are supported, including
> vendor's extensions. Drivers includes response parser of each command,
> according to its requirements, while the parser is written with loose
> fashion in two points; error check and length check. This doesn't cause
> any issues such as kernel corruption, but should be improved.
>
> This commit modifies evaluations of return value on each parsers.
>
> Reported-by: Dan Carpenter <dan.carpenter at oracle.com>
> Signed-off-by: Takashi Sakamoto <o-takashi at sakamocchi.jp>
Applied to for-next branch now. Thanks.
Takashi
> ---
> sound/firewire/bebob/bebob_command.c | 30 ++++++++++++++++++++++--------
> sound/firewire/fcp.c | 12 +++++++++---
> sound/firewire/oxfw/oxfw-command.c | 12 +++++++++---
> 3 files changed, 40 insertions(+), 14 deletions(-)
>
> diff --git a/sound/firewire/bebob/bebob_command.c b/sound/firewire/bebob/bebob_command.c
> index 9402cc1..f9b4225 100644
> --- a/sound/firewire/bebob/bebob_command.c
> +++ b/sound/firewire/bebob/bebob_command.c
> @@ -31,13 +31,15 @@ int avc_audio_set_selector(struct fw_unit *unit, unsigned int subunit_id,
> err = fcp_avc_transaction(unit, buf, 12, buf, 12,
> BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(5) |
> BIT(6) | BIT(7) | BIT(8));
> - if (err > 0 && err < 9)
> + if (err < 0)
> + ;
> + else if (err < 9)
> err = -EIO;
> else if (buf[0] == 0x08) /* NOT IMPLEMENTED */
> err = -ENOSYS;
> else if (buf[0] == 0x0a) /* REJECTED */
> err = -EINVAL;
> - else if (err > 0)
> + else
> err = 0;
>
> kfree(buf);
> @@ -67,7 +69,9 @@ int avc_audio_get_selector(struct fw_unit *unit, unsigned int subunit_id,
> err = fcp_avc_transaction(unit, buf, 12, buf, 12,
> BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(5) |
> BIT(6) | BIT(8));
> - if (err > 0 && err < 9)
> + if (err < 0)
> + ;
> + else if (err < 9)
> err = -EIO;
> else if (buf[0] == 0x08) /* NOT IMPLEMENTED */
> err = -ENOSYS;
> @@ -120,7 +124,9 @@ int avc_bridgeco_get_plug_type(struct fw_unit *unit,
> err = fcp_avc_transaction(unit, buf, 12, buf, 12,
> BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(5) |
> BIT(6) | BIT(7) | BIT(9));
> - if ((err >= 0) && (err < 8))
> + if (err < 0)
> + ;
> + else if (err < 11)
> err = -EIO;
> else if (buf[0] == 0x08) /* NOT IMPLEMENTED */
> err = -ENOSYS;
> @@ -150,7 +156,9 @@ int avc_bridgeco_get_plug_ch_pos(struct fw_unit *unit,
> err = fcp_avc_transaction(unit, buf, 12, buf, 256,
> BIT(1) | BIT(2) | BIT(3) | BIT(4) |
> BIT(5) | BIT(6) | BIT(7) | BIT(9));
> - if ((err >= 0) && (err < 8))
> + if (err < 0)
> + ;
> + else if (err < 11)
> err = -EIO;
> else if (buf[0] == 0x08) /* NOT IMPLEMENTED */
> err = -ENOSYS;
> @@ -187,7 +195,9 @@ int avc_bridgeco_get_plug_section_type(struct fw_unit *unit,
> err = fcp_avc_transaction(unit, buf, 12, buf, 12,
> BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(5) |
> BIT(6) | BIT(7) | BIT(9) | BIT(10));
> - if ((err >= 0) && (err < 8))
> + if (err < 0)
> + ;
> + else if (err < 12)
> err = -EIO;
> else if (buf[0] == 0x08) /* NOT IMPLEMENTED */
> err = -ENOSYS;
> @@ -221,7 +231,9 @@ int avc_bridgeco_get_plug_input(struct fw_unit *unit,
> err = fcp_avc_transaction(unit, buf, 16, buf, 16,
> BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(5) |
> BIT(6) | BIT(7));
> - if ((err >= 0) && (err < 8))
> + if (err < 0)
> + ;
> + else if (err < 16)
> err = -EIO;
> else if (buf[0] == 0x08) /* NOT IMPLEMENTED */
> err = -ENOSYS;
> @@ -260,7 +272,9 @@ int avc_bridgeco_get_plug_strm_fmt(struct fw_unit *unit,
> err = fcp_avc_transaction(unit, buf, 12, buf, *len,
> BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(5) |
> BIT(6) | BIT(7) | BIT(10));
> - if ((err >= 0) && (err < 12))
> + if (err < 0)
> + ;
> + else if (err < 12)
> err = -EIO;
> else if (buf[0] == 0x08) /* NOT IMPLEMENTED */
> err = -ENOSYS;
> diff --git a/sound/firewire/fcp.c b/sound/firewire/fcp.c
> index cce1976..61dda82 100644
> --- a/sound/firewire/fcp.c
> +++ b/sound/firewire/fcp.c
> @@ -63,7 +63,9 @@ int avc_general_set_sig_fmt(struct fw_unit *unit, unsigned int rate,
> /* do transaction and check buf[1-5] are the same against command */
> err = fcp_avc_transaction(unit, buf, 8, buf, 8,
> BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(5));
> - if (err >= 0 && err < 8)
> + if (err < 0)
> + ;
> + else if (err < 8)
> err = -EIO;
> else if (buf[0] == 0x08) /* NOT IMPLEMENTED */
> err = -ENOSYS;
> @@ -106,7 +108,9 @@ int avc_general_get_sig_fmt(struct fw_unit *unit, unsigned int *rate,
> /* do transaction and check buf[1-4] are the same against command */
> err = fcp_avc_transaction(unit, buf, 8, buf, 8,
> BIT(1) | BIT(2) | BIT(3) | BIT(4));
> - if (err >= 0 && err < 8)
> + if (err < 0)
> + ;
> + else if (err < 8)
> err = -EIO;
> else if (buf[0] == 0x08) /* NOT IMPLEMENTED */
> err = -ENOSYS;
> @@ -154,7 +158,9 @@ int avc_general_get_plug_info(struct fw_unit *unit, unsigned int subunit_type,
> buf[3] = 0xff & subfunction;
>
> err = fcp_avc_transaction(unit, buf, 8, buf, 8, BIT(1) | BIT(2));
> - if (err >= 0 && err < 8)
> + if (err < 0)
> + ;
> + else if (err < 8)
> err = -EIO;
> else if (buf[0] == 0x08) /* NOT IMPLEMENTED */
> err = -ENOSYS;
> diff --git a/sound/firewire/oxfw/oxfw-command.c b/sound/firewire/oxfw/oxfw-command.c
> index 12ef325..ac3e2e3 100644
> --- a/sound/firewire/oxfw/oxfw-command.c
> +++ b/sound/firewire/oxfw/oxfw-command.c
> @@ -34,7 +34,9 @@ int avc_stream_set_format(struct fw_unit *unit, enum avc_general_plug_dir dir,
> err = fcp_avc_transaction(unit, buf, len + 10, buf, len + 10,
> BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(5) |
> BIT(6) | BIT(7) | BIT(8));
> - if ((err > 0) && (err < len + 10))
> + if (err < 0)
> + ;
> + else if (err < len + 10)
> err = -EIO;
> else if (buf[0] == 0x08) /* NOT IMPLEMENTED */
> err = -ENOSYS;
> @@ -77,7 +79,9 @@ int avc_stream_get_format(struct fw_unit *unit,
> err = fcp_avc_transaction(unit, buf, 12, buf, *len,
> BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(5) |
> BIT(6) | BIT(7));
> - if ((err > 0) && (err < 10))
> + if (err < 0)
> + ;
> + else if (err < 12)
> err = -EIO;
> else if (buf[0] == 0x08) /* NOT IMPLEMENTED */
> err = -ENOSYS;
> @@ -139,7 +143,9 @@ int avc_general_inquiry_sig_fmt(struct fw_unit *unit, unsigned int rate,
> /* do transaction and check buf[1-5] are the same against command */
> err = fcp_avc_transaction(unit, buf, 8, buf, 8,
> BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(5));
> - if ((err > 0) && (err < 8))
> + if (err < 0)
> + ;
> + else if (err < 8)
> err = -EIO;
> else if (buf[0] == 0x08) /* NOT IMPLEMENTED */
> err = -ENOSYS;
> --
> 2.9.3
>
More information about the Alsa-devel
mailing list