All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Luo Likang <luolikang@nsfocus.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
	Ben Hutchings <ben@decadent.org.uk>
Subject: [PATCH 4.9 2/9] media: firewire: firedtv-avc: fix a buffer overflow in avc_ca_pmt()
Date: Thu, 27 Jan 2022 19:08:20 +0100	[thread overview]
Message-ID: <20220127180257.302466439@linuxfoundation.org> (raw)
In-Reply-To: <20220127180257.225641300@linuxfoundation.org>

From: Dan Carpenter <dan.carpenter@oracle.com>

commit 35d2969ea3c7d32aee78066b1f3cf61a0d935a4e upstream.

The bounds checking in avc_ca_pmt() is not strict enough.  It should
be checking "read_pos + 4" because it's reading 5 bytes.  If the
"es_info_length" is non-zero then it reads a 6th byte so there needs to
be an additional check for that.

I also added checks for the "write_pos".  I don't think these are
required because "read_pos" and "write_pos" are tied together so
checking one ought to be enough.  But they make the code easier to
understand for me.  The check on write_pos is:

	if (write_pos + 4 >= sizeof(c->operand) - 4) {

The first "+ 4" is because we're writing 5 bytes and the last " - 4"
is to leave space for the CRC.

The other problem is that "length" can be invalid.  It comes from
"data_length" in fdtv_ca_pmt().

Cc: stable@vger.kernel.org
Reported-by: Luo Likang <luolikang@nsfocus.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
[bwh: Backported to 4.9: adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/media/firewire/firedtv-avc.c |   14 +++++++++++---
 drivers/media/firewire/firedtv-ci.c  |    2 ++
 2 files changed, 13 insertions(+), 3 deletions(-)

--- a/drivers/media/firewire/firedtv-avc.c
+++ b/drivers/media/firewire/firedtv-avc.c
@@ -1169,7 +1169,11 @@ int avc_ca_pmt(struct firedtv *fdtv, cha
 		read_pos += program_info_length;
 		write_pos += program_info_length;
 	}
-	while (read_pos < length) {
+	while (read_pos + 4 < length) {
+		if (write_pos + 4 >= sizeof(c->operand) - 4) {
+			ret = -EINVAL;
+			goto out;
+		}
 		c->operand[write_pos++] = msg[read_pos++];
 		c->operand[write_pos++] = msg[read_pos++];
 		c->operand[write_pos++] = msg[read_pos++];
@@ -1181,13 +1185,17 @@ int avc_ca_pmt(struct firedtv *fdtv, cha
 		c->operand[write_pos++] = es_info_length >> 8;
 		c->operand[write_pos++] = es_info_length & 0xff;
 		if (es_info_length > 0) {
+			if (read_pos >= length) {
+				ret = -EINVAL;
+				goto out;
+			}
 			pmt_cmd_id = msg[read_pos++];
 			if (pmt_cmd_id != 1 && pmt_cmd_id != 4)
 				dev_err(fdtv->device, "invalid pmt_cmd_id %d "
 					"at stream level\n", pmt_cmd_id);
 
-			if (es_info_length > sizeof(c->operand) - 4 -
-					     write_pos) {
+			if (es_info_length > sizeof(c->operand) - 4 - write_pos ||
+			    es_info_length > length - read_pos) {
 				ret = -EINVAL;
 				goto out;
 			}
--- a/drivers/media/firewire/firedtv-ci.c
+++ b/drivers/media/firewire/firedtv-ci.c
@@ -138,6 +138,8 @@ static int fdtv_ca_pmt(struct firedtv *f
 	} else {
 		data_length = msg->msg[3];
 	}
+	if (data_length > sizeof(msg->msg) - data_pos)
+		return -EINVAL;
 
 	return avc_ca_pmt(fdtv, &msg->msg[data_pos], data_length);
 }



  parent reply	other threads:[~2022-01-27 18:08 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-27 18:08 [PATCH 4.9 0/9] 4.9.299-rc1 review Greg Kroah-Hartman
2022-01-27 18:08 ` [PATCH 4.9 1/9] drm/i915: Flush TLBs before releasing backing store Greg Kroah-Hartman
2022-01-27 18:08 ` Greg Kroah-Hartman [this message]
2022-01-27 18:08 ` [PATCH 4.9 3/9] NFSv4: Initialise connection to the server in nfs4_alloc_client() Greg Kroah-Hartman
2022-01-27 18:08 ` [PATCH 4.9 4/9] KVM: nVMX: fix EPT permissions as reported in exit qualification Greg Kroah-Hartman
2022-01-27 18:08 ` [PATCH 4.9 5/9] KVM: X86: MMU: Use the correct inherited permissions to get shadow page Greg Kroah-Hartman
2022-01-27 18:08 ` [PATCH 4.9 6/9] ARM: 8800/1: use choice for kernel unwinders Greg Kroah-Hartman
2022-01-27 18:08 ` [PATCH 4.9 7/9] ion: Fix use after free during ION_IOC_ALLOC Greg Kroah-Hartman
2022-01-27 18:08 ` [PATCH 4.9 8/9] ion: Protect kref from userspace manipulation Greg Kroah-Hartman
2022-01-27 18:08 ` [PATCH 4.9 9/9] ion: Do not put ION handle until after its final use Greg Kroah-Hartman
2022-01-27 18:35 ` [PATCH 4.9 0/9] 4.9.299-rc1 review Florian Fainelli
2022-01-28  1:18 ` Shuah Khan
2022-01-28 11:19 ` Jon Hunter
2022-01-29  1:04 ` Guenter Roeck
2022-01-29  9:30 ` Naresh Kamboju

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220127180257.302466439@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=ben@decadent.org.uk \
    --cc=dan.carpenter@oracle.com \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luolikang@nsfocus.com \
    --cc=mchehab+huawei@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.