Date: Fri, 26 Jul 2019 01:45:00 +0000 (UTC) From: Kyle Evans <kevans@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r350341 - stable/12/stand/libsa/zfs Message-ID: <201907260145.x6Q1j00Z081043@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kevans Date: Fri Jul 26 01:45:00 2019 New Revision: 350341 URL: https://svnweb.freebsd.org/changeset/base/350341 Log: MFC r344226, r344234: stand: zfs memory corruption bug r344226: 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. r344234: It turns out r344226 narrowed the overrun bug but did not eliminate it entirely This commit fixes a remaining output buffer overrun in the single-sector case when there is a non-zero tail. Modified: stable/12/stand/libsa/zfs/zfs.c Directory Properties: stable/12/ (props changed) Modified: stable/12/stand/libsa/zfs/zfs.c ============================================================================== --- stable/12/stand/libsa/zfs/zfs.c Fri Jul 26 01:42:24 2019 (r350340) +++ stable/12/stand/libsa/zfs/zfs.c Fri Jul 26 01:45:00 2019 (r350341) @@ -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, min(secsz - head, bytes)); + outbuf += min(secsz - head, bytes); + } + + /* 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?201907260145.x6Q1j00Z081043>