Hello Jason,
+-- On Fri, 26 Feb 2016, Jason Wang wrote --+
| Should we count mac header here? Did "plen + hlen >= length" imply "14 +
| hlen + csum_offset + 1" < length?
|
| Looks not. Consider a TCP packet can report evil plen (e.g 20) but just
| have 10 bytes payload in fact. In this case:
|
| hlen = 20, plen = 20, csum_offset = 16, length = 44 which can pass all
| the above tests, but 14 + hlen + csum_offset = 50 which is greater than
| length.
Ummn, not sure. hlen + plen = total length(IP + TCP/UDP), 20+20 != 44.
'length' is also used to read and allocate requisite buffers in other parts
from where net_checksum_calculate() is called,
===
etsec->tx_buffer = g_realloc(etsec->tx_buffer,
etsec->tx_buffer_len + bd->length);
...
/* Update buffer length */
etsec->tx_buffer_len += bd->length;
process_tx_fcb(etsec);
-> net_checksum_calculate(etsec->tx_buffer + 8,
etsec->tx_buffer_len - 8);
OR
memcpy(tmpbuf, page + txreq.offset, txreq.size);
net_checksum_calculate(tmpbuf, txreq.size);
===
- With 'length < 14 + 20', we ensure that L2 & L3 headers are complete.
44 - 34 = 10, TCP/UDP header is incomplete.
I think 'plen=20 != 10' hints at an error in other parts of the code? Also if
data buffer has more space than actual data, OOB access may not occur.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
