Re: [V9fs-developer] [PATCH] p9_parse_header() validate PDU length

On 07/18/2018 07:13 AM, Dominique Martinet wrote:

Tomas Bortoli wrote on Thu, Jul 12, 2018:

quoted

This patch adds checks to the p9_parse_header() function to
verify that the length found within the header coincides with the actual
length of the PDU. Furthermore, it checks that the length stays within the
acceptable range. To do this the patch brings the actual length of the PDU
from the different transport layers (rdma and virtio). For TCP (trans_fd.c)
the length is not know before, so we get it from the header but we check it
anyway that it's within the valid range.

Signed-off-by: Tomas Bortoli <redacted>
Reported-by: syzbot+65c6b72f284a39d416b4@syzkaller.appspotmail.com
---
[..]

@@ -498,6 +489,21 @@ p9_parse_header(struct p9_fcall *pdu, int32_t *size, int8_t *type, int16_t *tag,
 	if (size)
 		*size = r_size;
 
+	if (pdu->size != r_size) {
+		err = -EINVAL;
+		goto rewind_and_exit;
+	}
+	if (pdu->size >= pdu->capacity || pdu->size < 7) {
+		p9_debug(P9_DEBUG_ERROR,
+			 "requested packet size too big or too small: %d\n",
+			 pdu->size);
+		return -EIO;
+	}

Actually, I've been bad advice - this breaks on virtio with zc packets -
a read or ls in a big directory fails with this in dmesg
[ 1006.853775] 9pnet: -- p9_parse_header (17123): requested packet size too big or too small: 4306
[ 1006.853780] 9pnet: -- p9_check_zc_errors (17123): couldn't parse header -5

I haven't given this any thought yet, but dropping the patch for now

It seems that pdu->capacity is set to less than 4306 at that point, we
can just increase it.