Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

media: firewire: firedtv-avc: fix a buffer overflow in avc_ca_pmt()

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>

authored by

Dan Carpenter and committed by
Mauro Carvalho Chehab
35d2969e 9031d6b3

+13 -3
+11 -3
drivers/media/firewire/firedtv-avc.c
··· 1165 1165 read_pos += program_info_length; 1166 1166 write_pos += program_info_length; 1167 1167 } 1168 - while (read_pos < length) { 1168 + while (read_pos + 4 < length) { 1169 + if (write_pos + 4 >= sizeof(c->operand) - 4) { 1170 + ret = -EINVAL; 1171 + goto out; 1172 + } 1169 1173 c->operand[write_pos++] = msg[read_pos++]; 1170 1174 c->operand[write_pos++] = msg[read_pos++]; 1171 1175 c->operand[write_pos++] = msg[read_pos++]; ··· 1181 1177 c->operand[write_pos++] = es_info_length >> 8; 1182 1178 c->operand[write_pos++] = es_info_length & 0xff; 1183 1179 if (es_info_length > 0) { 1180 + if (read_pos >= length) { 1181 + ret = -EINVAL; 1182 + goto out; 1183 + } 1184 1184 pmt_cmd_id = msg[read_pos++]; 1185 1185 if (pmt_cmd_id != 1 && pmt_cmd_id != 4) 1186 1186 dev_err(fdtv->device, "invalid pmt_cmd_id %d at stream level\n", 1187 1187 pmt_cmd_id); 1188 1188 1189 - if (es_info_length > sizeof(c->operand) - 4 - 1190 - write_pos) { 1189 + if (es_info_length > sizeof(c->operand) - 4 - write_pos || 1190 + es_info_length > length - read_pos) { 1191 1191 ret = -EINVAL; 1192 1192 goto out; 1193 1193 }
+2
drivers/media/firewire/firedtv-ci.c
··· 134 134 } else { 135 135 data_length = msg->msg[3]; 136 136 } 137 + if (data_length > sizeof(msg->msg) - data_pos) 138 + return -EINVAL; 137 139 138 140 return avc_ca_pmt(fdtv, &msg->msg[data_pos], data_length); 139 141 }