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@xxxxxxxxxx> > Signed-off-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx> 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 > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
