Skip site navigation (1)Skip section navigation (2) 
Date:      Sun, 17 Feb 2019 03:35:15 +0000 (UTC)
From:      Patrick Kelsey <pkelsey@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r344226 - head/stand/libsa/zfs
Message-ID:  <201902170335.x1H3ZFPH031621@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: pkelsey
Date: Sun Feb 17 03:35:15 2019
New Revision: 344226
URL: https://svnweb.freebsd.org/changeset/base/344226

Log:
  Fix memory corruption bug introduced in r325310
  
  The bug occurred when a bounce buffer was used and the requested read
  size was greater than the size of the bounce buffer.  This commit also
  rewrites the read logic so that it is easier to systematically verify
  all alignment and size cases.
  
  Reviewed by:	allanjude, tsoome
  MFC after:	3 months
  Differential Revision:	https://reviews.freebsd.org/D19140

Modified:
  head/stand/libsa/zfs/zfs.c

Modified: head/stand/libsa/zfs/zfs.c
==============================================================================
--- head/stand/libsa/zfs/zfs.c	Sun Feb 17 03:21:42 2019	(r344225)
+++ head/stand/libsa/zfs/zfs.c	Sun Feb 17 03:35:15 2019	(r344226)
@@ -363,51 +363,100 @@ static int
 vdev_read(vdev_t *vdev, void *priv, off_t offset, void *buf, size_t bytes)
 {
 	int fd, ret;
-	size_t res, size, remainder, rb_size, blksz;
-	unsigned secsz;
-	off_t off;
-	char *bouncebuf, *rb_buf;
+	size_t res, head, tail, total_size, full_sec_size;
+	unsigned secsz, do_tail_read;
+	off_t start_sec;
+	char *outbuf, *bouncebuf;
 
 	fd = (uintptr_t) priv;
+	outbuf = (char *) buf;
 	bouncebuf = NULL;
 
 	ret = ioctl(fd, DIOCGSECTORSIZE, &secsz);
 	if (ret != 0)
 		return (ret);
 
-	off = offset / secsz;
-	remainder = offset % secsz;
-	if (lseek(fd, off * secsz, SEEK_SET) == -1)
-		return (errno);
+	/*
+	 * Handling reads of arbitrary offset and size - multi-sector case
+	 * and single-sector case.
+	 *
+	 *                        Multi-sector Case
+	 *                (do_tail_read = true if tail > 0)
+	 *
+	 *   |<----------------------total_size--------------------->|
+	 *   |                                                       |
+	 *   |<--head-->|<--------------bytes------------>|<--tail-->|
+	 *   |          |                                 |          |
+	 *   |          |       |<~full_sec_size~>|       |          |
+	 *   +------------------+                 +------------------+
+	 *   |          |0101010|     .  .  .     |0101011|          |
+	 *   +------------------+                 +------------------+
+	 *         start_sec                         start_sec + n
+	 *
+	 *
+	 *                      Single-sector Case
+	 *                    (do_tail_read = false)
+	 *
+	 *              |<------total_size = secsz----->|
+	 *              |                               |
+	 *              |<-head->|<---bytes--->|<-tail->|
+	 *              +-------------------------------+
+	 *              |        |0101010101010|        |
+	 *              +-------------------------------+
+	 *                          start_sec
+	 */
+	start_sec = offset / secsz;
+	head = offset % secsz;
+	total_size = roundup2(head + bytes, secsz);
+	tail = total_size - (head + bytes);
+	do_tail_read = ((tail > 0) && (head + bytes > secsz));
+	full_sec_size = total_size;
+	if (head > 0)
+		full_sec_size -= secsz;
+	if (do_tail_read)
+		full_sec_size -= secsz;
 
-	rb_buf = buf;
-	rb_size = bytes;
-	size = roundup2(bytes + remainder, secsz);
-	blksz = size;
-	if (remainder != 0 || size != bytes) {
+	/* Return of partial sector data requires a bounce buffer. */
+	if ((head > 0) || do_tail_read) {
 		bouncebuf = zfs_alloc(secsz);
 		if (bouncebuf == NULL) {
 			printf("vdev_read: out of memory\n");
 			return (ENOMEM);
 		}
-		rb_buf = bouncebuf;
-		blksz = rb_size - remainder;
 	}
 
-	while (bytes > 0) {
-		res = read(fd, rb_buf, rb_size);
-		if (res != rb_size) {
+	if (lseek(fd, start_sec * secsz, SEEK_SET) == -1)
+		return (errno);
+
+	/* Partial data return from first sector */
+	if (head > 0) {
+		res = read(fd, bouncebuf, secsz);
+		if (res != secsz) {
 			ret = EIO;
 			goto error;
 		}
-		if (bytes < blksz)
-			blksz = bytes;
-		if (bouncebuf != NULL)
-			memcpy(buf, rb_buf + remainder, blksz);
-		buf = (void *)((uintptr_t)buf + blksz);
-		bytes -= blksz;
-		remainder = 0;
-		blksz = rb_size;
+		memcpy(outbuf, bouncebuf + head, secsz - head);
+		outbuf += secsz - head;
+	}
+
+	/* Full data return from read sectors */
+	if (full_sec_size > 0) {
+		res = read(fd, outbuf, full_sec_size);
+		if (res != full_sec_size) {
+			ret = EIO;
+			goto error;
+		}
+		outbuf += full_sec_size;
+	}
+
+	/* Partial data return from last sector */
+	if (do_tail_read) {
+		res = read(fd, bouncebuf, secsz);
+		if (res != secsz) {
+			ret = EIO;
+			goto error;
+		}
+		memcpy(outbuf, bouncebuf, secsz - tail);
 	}
 
 	ret = 0;

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201902170335.x1H3ZFPH031621>