Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 29 Aug 2015 09:22:33 +0000 (UTC)
From:      Xin LI <delphij@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r287283 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Message-ID:  <201508290922.t7T9MXhF007620@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: delphij
Date: Sat Aug 29 09:22:32 2015
New Revision: 287283
URL: https://svnweb.freebsd.org/changeset/base/287283

Log:
  Fix a buffer overrun which may lead to data corruption, introduced in
  r286951 by reinstating changes in r274628.
  
  In l2arc_compress_buf(), we allocate a buffer to stash away the compressed
  data in 'cdata', allocated of l2hdr->b_asize bytes.
  
  We then ask zio_compress_data() to compress the buffer, b_l1hdr.b_tmp_cdata,
  which is of l2hdr->b_asize bytes, and have the compressed size (or original
  size, if compress didn't gain enough) stored in csize.
  
  To pad the buffer to fit the optimal write size, we round up the compressed
  size to L2 device's vdev_ashift.
  
  Illumos code rounds up the size by at most SPA_MINBLOCKSIZE.  Because we
  know csize <= b_asize, and b_asize is integer multiple of SPA_MINBLOCKSIZE,
  we are guaranteed that the rounded up csize would be <= b_asize. However,
  this is not necessarily true when we round up to 1 << vdev_ashift, because
  it could be larger than SPA_MINBLOCKSIZE.
  
  So, in the worst case scenario, we are overwriting at most
  
  	(1 << vdev_ashift - SPA_MINBLOCKSIZE)
  
  bytes of memory next to the compressed data buffer.
  
  Andriy's original change in r274628 reorganized the code a little bit,
  by moving the padding to after we determined that the compression was
  beneficial.  At which point, we would check rounded size against the
  allocated buffer size, and the buffer overrun would not be possible.

Modified:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c	Sat Aug 29 09:14:32 2015	(r287282)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c	Sat Aug 29 09:22:32 2015	(r287283)
@@ -6529,13 +6529,6 @@ l2arc_compress_buf(arc_buf_hdr_t *hdr)
 	csize = zio_compress_data(ZIO_COMPRESS_LZ4, hdr->b_l1hdr.b_tmp_cdata,
 	    cdata, l2hdr->b_asize);
 
-	rounded = P2ROUNDUP(csize,
-	    (size_t)1 << l2hdr->b_dev->l2ad_vdev->vdev_ashift);
-	if (rounded > csize) {
-		bzero((char *)cdata + csize, rounded - csize);
-		csize = rounded;
-	}
-
 	if (csize == 0) {
 		/* zero block, indicate that there's nothing to write */
 		zio_data_buf_free(cdata, len);
@@ -6544,11 +6537,19 @@ l2arc_compress_buf(arc_buf_hdr_t *hdr)
 		hdr->b_l1hdr.b_tmp_cdata = NULL;
 		ARCSTAT_BUMP(arcstat_l2_compress_zeros);
 		return (B_TRUE);
-	} else if (csize > 0 && csize < len) {
+	}
+
+	rounded = P2ROUNDUP(csize,
+	    (size_t)1 << l2hdr->b_dev->l2ad_vdev->vdev_ashift);
+	if (rounded < len) {
 		/*
 		 * Compression succeeded, we'll keep the cdata around for
 		 * writing and release it afterwards.
 		 */
+		if (rounded > csize) {
+			bzero((char *)cdata + csize, rounded - csize);
+			csize = rounded;
+		}
 		HDR_SET_COMPRESS(hdr, ZIO_COMPRESS_LZ4);
 		l2hdr->b_asize = csize;
 		hdr->b_l1hdr.b_tmp_cdata = cdata;



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