Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1235648 > unrolled thread
| Started by | Laura Abbott <labbott@fedoraproject.org> |
|---|---|
| First post | 2015-09-30 02:20 +0200 |
| Last post | 2015-10-06 00:40 +0200 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 2/2] si2157: Bounds check firmware Laura Abbott <labbott@fedoraproject.org> - 2015-09-30 02:20 +0200
Re: [PATCH 2/2] si2157: Bounds check firmware Laura Abbott <labbott@redhat.com> - 2015-10-06 00:30 +0200
[PATCHv2] si2157: Bounds check firmware Laura Abbott <labbott@fedoraproject.org> - 2015-10-06 00:40 +0200
Re: [PATCHv2] si2157: Bounds check firmware Olli Salonen <olli.salonen@iki.fi> - 2015-10-08 12:50 +0200
Re: [PATCH 2/2] si2157: Bounds check firmware Olli Salonen <olli.salonen@iki.fi> - 2015-10-06 00:40 +0200
| From | Laura Abbott <labbott@fedoraproject.org> |
|---|---|
| Date | 2015-09-30 02:20 +0200 |
| Subject | [PATCH 2/2] si2157: Bounds check firmware |
| Message-ID | <qedhM-5nT-5@gated-at.bofh.it> |
When reading the firmware and sending commands, the length
must be bounds checked to avoid overrunning the size of the command
buffer and smashing the stack if the firmware is not in the
expected format. Add the proper check.
Cc: stable@kernel.org
Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
---
drivers/media/tuners/si2157.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 5073821..ce157ed 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -166,6 +166,10 @@ static int si2157_init(struct dvb_frontend *fe)
for (remaining = fw->size; remaining > 0; remaining -= 17) {
len = fw->data[fw->size - remaining];
+ if (len > SI2157_ARGLEN) {
+ dev_err(&client->dev, "Bad firmware length\n");
+ goto err_release_firmware;
+ }
memcpy(cmd.args, &fw->data[(fw->size - remaining) + 1], len);
cmd.wlen = len;
cmd.rlen = 1;
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Laura Abbott <labbott@redhat.com> |
|---|---|
| Date | 2015-10-06 00:30 +0200 |
| Message-ID | <qgmqC-3Uq-13@gated-at.bofh.it> |
| In reply to | #1235648 |
On 10/05/2015 03:24 PM, Olli Salonen wrote:
> Hi Laura,
>
> While the patch itself does what it says, the return code for the
> si2157_init will be 0 even if there's a faulty firmware file. Wouldn't
> it be better to set the return code as -EINVAL like done a few lines
> earlier in the code (see below)?
>
> if (fw->size % 17 != 0) {
> dev_err(&client->dev, "firmware file '%s' is invalid\n",
> fw_name);
> ret = -EINVAL;
> goto err_release_firmware;
> }
>
> Cheers,
> -olli
>
Right, I'll update with v2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Laura Abbott <labbott@fedoraproject.org> |
|---|---|
| Date | 2015-10-06 00:40 +0200 |
| Subject | [PATCHv2] si2157: Bounds check firmware |
| Message-ID | <qgmAh-48q-13@gated-at.bofh.it> |
| In reply to | #1240001 |
When reading the firmware and sending commands, the length
must be bounds checked to avoid overrunning the size of the command
buffer and smashing the stack if the firmware is not in the
expected format. Add the proper check.
Cc: stable@kernel.org
Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
---
v2: Set the return code properly
---
drivers/media/tuners/si2157.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 5073821..0e1ca2b 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -166,6 +166,11 @@ static int si2157_init(struct dvb_frontend *fe)
for (remaining = fw->size; remaining > 0; remaining -= 17) {
len = fw->data[fw->size - remaining];
+ if (len > SI2157_ARGLEN) {
+ dev_err(&client->dev, "Bad firmware length\n");
+ ret = -EINVAL;
+ goto err_release_firmware;
+ }
memcpy(cmd.args, &fw->data[(fw->size - remaining) + 1], len);
cmd.wlen = len;
cmd.rlen = 1;
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Olli Salonen <olli.salonen@iki.fi> |
|---|---|
| Date | 2015-10-08 12:50 +0200 |
| Subject | Re: [PATCHv2] si2157: Bounds check firmware |
| Message-ID | <qhgVQ-12b-19@gated-at.bofh.it> |
| In reply to | #1240008 |
Reviewed-by: Olli Salonen <olli.salonen@iki.fi>
Tested-by: Olli Salonen <olli.salonen@iki.fi>
On 6 October 2015 at 01:33, Laura Abbott <labbott@fedoraproject.org> wrote:
>
> When reading the firmware and sending commands, the length
> must be bounds checked to avoid overrunning the size of the command
> buffer and smashing the stack if the firmware is not in the
> expected format. Add the proper check.
>
> Cc: stable@kernel.org
> Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
> ---
> v2: Set the return code properly
> ---
> drivers/media/tuners/si2157.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index 5073821..0e1ca2b 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -166,6 +166,11 @@ static int si2157_init(struct dvb_frontend *fe)
>
> for (remaining = fw->size; remaining > 0; remaining -= 17) {
> len = fw->data[fw->size - remaining];
> + if (len > SI2157_ARGLEN) {
> + dev_err(&client->dev, "Bad firmware length\n");
> + ret = -EINVAL;
> + goto err_release_firmware;
> + }
> memcpy(cmd.args, &fw->data[(fw->size - remaining) + 1], len);
> cmd.wlen = len;
> cmd.rlen = 1;
> --
> 2.4.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Olli Salonen <olli.salonen@iki.fi> |
|---|---|
| Date | 2015-10-06 00:40 +0200 |
| Message-ID | <qgmqC-3Uq-15@gated-at.bofh.it> |
| In reply to | #1235648 |
Hi Laura,
While the patch itself does what it says, the return code for the
si2157_init will be 0 even if there's a faulty firmware file. Wouldn't
it be better to set the return code as -EINVAL like done a few lines
earlier in the code (see below)?
if (fw->size % 17 != 0) {
dev_err(&client->dev, "firmware file '%s' is invalid\n",
fw_name);
ret = -EINVAL;
goto err_release_firmware;
}
Cheers,
-olli
On 30 September 2015 at 02:10, Laura Abbott <labbott@fedoraproject.org> wrote:
> When reading the firmware and sending commands, the length
> must be bounds checked to avoid overrunning the size of the command
> buffer and smashing the stack if the firmware is not in the
> expected format. Add the proper check.
>
> Cc: stable@kernel.org
> Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
> ---
> drivers/media/tuners/si2157.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index 5073821..ce157ed 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -166,6 +166,10 @@ static int si2157_init(struct dvb_frontend *fe)
>
> for (remaining = fw->size; remaining > 0; remaining -= 17) {
> len = fw->data[fw->size - remaining];
> + if (len > SI2157_ARGLEN) {
> + dev_err(&client->dev, "Bad firmware length\n");
> + goto err_release_firmware;
> + }
> memcpy(cmd.args, &fw->data[(fw->size - remaining) + 1], len);
> cmd.wlen = len;
> cmd.rlen = 1;
> --
> 2.4.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web