Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 12 Sep 2015 09:56:24 +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: r287706 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys
Message-ID:  <201509120956.t8C9uOhZ069704@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: delphij
Date: Sat Sep 12 09:56:23 2015
New Revision: 287706
URL: https://svnweb.freebsd.org/changeset/base/287706

Log:
  MFV r287699: 6214 zpools going south
  
  In r286570 (MFV of r277426) an unprotected write to b_flags to
  set the compression mode was introduced.  This would open a race
  window where data is partially decompressed, modified, checksummed
  and written to the pool, resulting in pool corruption due to the
  partial decompression.
  
  Prevent this by reintroducing b_compress
  
  illumos/illumos-gate@d4cd038c92c36fd0ae35945831a8fc2975b5272c
  
  Illumos issues:
  
      6214 zpools going south
      https://www.illumos.org/issues/6214

Modified:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/arc.h
Directory Properties:
  head/sys/cddl/contrib/opensolaris/   (props changed)

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c	Sat Sep 12 09:28:02 2015	(r287705)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c	Sat Sep 12 09:56:23 2015	(r287706)
@@ -848,6 +848,7 @@ typedef struct l2arc_buf_hdr {
 	uint64_t		b_daddr;	/* disk address, offset byte */
 	/* real alloc'd buffer size depending on b_compress applied */
 	int32_t			b_asize;
+	uint8_t			b_compress;
 
 	list_node_t		b_l2node;
 } l2arc_buf_hdr_t;
@@ -927,15 +928,6 @@ static arc_buf_hdr_t arc_eviction_hdr;
 #define	HDR_HAS_L1HDR(hdr)	((hdr)->b_flags & ARC_FLAG_HAS_L1HDR)
 #define	HDR_HAS_L2HDR(hdr)	((hdr)->b_flags & ARC_FLAG_HAS_L2HDR)
 
-/* For storing compression mode in b_flags */
-#define	HDR_COMPRESS_OFFSET	24
-#define	HDR_COMPRESS_NBITS	7
-
-#define	HDR_GET_COMPRESS(hdr)	((enum zio_compress)BF32_GET(hdr->b_flags, \
-	    HDR_COMPRESS_OFFSET, HDR_COMPRESS_NBITS))
-#define	HDR_SET_COMPRESS(hdr, cmp) BF32_SET(hdr->b_flags, \
-	    HDR_COMPRESS_OFFSET, HDR_COMPRESS_NBITS, (cmp))
-
 /*
  * Other sizes
  */
@@ -2226,7 +2218,7 @@ arc_buf_l2_cdata_free(arc_buf_hdr_t *hdr
 	 * separately compressed buffer, so there's nothing to free (it
 	 * points to the same buffer as the arc_buf_t's b_data field).
 	 */
-	if (HDR_GET_COMPRESS(hdr) == ZIO_COMPRESS_OFF) {
+	if (hdr->b_l2hdr.b_compress == ZIO_COMPRESS_OFF) {
 		hdr->b_l1hdr.b_tmp_cdata = NULL;
 		return;
 	}
@@ -2235,12 +2227,12 @@ arc_buf_l2_cdata_free(arc_buf_hdr_t *hdr
 	 * There's nothing to free since the buffer was all zero's and
 	 * compressed to a zero length buffer.
 	 */
-	if (HDR_GET_COMPRESS(hdr) == ZIO_COMPRESS_EMPTY) {
+	if (hdr->b_l2hdr.b_compress == ZIO_COMPRESS_EMPTY) {
 		ASSERT3P(hdr->b_l1hdr.b_tmp_cdata, ==, NULL);
 		return;
 	}
 
-	ASSERT(L2ARC_IS_VALID_COMPRESS(HDR_GET_COMPRESS(hdr)));
+	ASSERT(L2ARC_IS_VALID_COMPRESS(hdr->b_l2hdr.b_compress));
 
 	arc_buf_free_on_write(hdr->b_l1hdr.b_tmp_cdata,
 	    hdr->b_size, zio_data_buf_free);
@@ -4464,7 +4456,7 @@ top:
 		    (vd = hdr->b_l2hdr.b_dev->l2ad_vdev) != NULL) {
 			devw = hdr->b_l2hdr.b_dev->l2ad_writing;
 			addr = hdr->b_l2hdr.b_daddr;
-			b_compress = HDR_GET_COMPRESS(hdr);
+			b_compress = hdr->b_l2hdr.b_compress;
 			b_asize = hdr->b_l2hdr.b_asize;
 			/*
 			 * Lock out device removal.
@@ -6025,6 +6017,8 @@ l2arc_read_done(zio_t *zio)
 	if (cb->l2rcb_compress != ZIO_COMPRESS_OFF)
 		l2arc_decompress_zio(zio, hdr, cb->l2rcb_compress);
 	ASSERT(zio->io_data != NULL);
+	ASSERT3U(zio->io_size, ==, hdr->b_size);
+	ASSERT3U(BP_GET_LSIZE(&cb->l2rcb_bp), ==, hdr->b_size);
 
 	/*
 	 * Check this survived the L2ARC journey.
@@ -6061,7 +6055,7 @@ l2arc_read_done(zio_t *zio)
 			ASSERT(!pio || pio->io_child_type == ZIO_CHILD_LOGICAL);
 
 			zio_nowait(zio_read(pio, cb->l2rcb_spa, &cb->l2rcb_bp,
-			    buf->b_data, zio->io_size, arc_read_done, buf,
+			    buf->b_data, hdr->b_size, arc_read_done, buf,
 			    zio->io_priority, cb->l2rcb_flags, &cb->l2rcb_zb));
 		}
 	}
@@ -6378,7 +6372,7 @@ l2arc_write_buffers(spa_t *spa, l2arc_de
 			 * can't access without holding the ARC list locks
 			 * (which we want to avoid during compression/writing).
 			 */
-			HDR_SET_COMPRESS(hdr, ZIO_COMPRESS_OFF);
+			hdr->b_l2hdr.b_compress = ZIO_COMPRESS_OFF;
 			hdr->b_l2hdr.b_asize = hdr->b_size;
 			hdr->b_l1hdr.b_tmp_cdata = hdr->b_l1hdr.b_buf->b_data;
 
@@ -6580,7 +6574,7 @@ l2arc_compress_buf(arc_buf_hdr_t *hdr)
 	l2arc_buf_hdr_t *l2hdr = &hdr->b_l2hdr;
 
 	ASSERT(HDR_HAS_L1HDR(hdr));
-	ASSERT(HDR_GET_COMPRESS(hdr) == ZIO_COMPRESS_OFF);
+	ASSERT3S(l2hdr->b_compress, ==, ZIO_COMPRESS_OFF);
 	ASSERT(hdr->b_l1hdr.b_tmp_cdata != NULL);
 
 	len = l2hdr->b_asize;
@@ -6592,7 +6586,7 @@ l2arc_compress_buf(arc_buf_hdr_t *hdr)
 	if (csize == 0) {
 		/* zero block, indicate that there's nothing to write */
 		zio_data_buf_free(cdata, len);
-		HDR_SET_COMPRESS(hdr, ZIO_COMPRESS_EMPTY);
+		l2hdr->b_compress = ZIO_COMPRESS_EMPTY;
 		l2hdr->b_asize = 0;
 		hdr->b_l1hdr.b_tmp_cdata = NULL;
 		ARCSTAT_BUMP(arcstat_l2_compress_zeros);
@@ -6610,7 +6604,7 @@ l2arc_compress_buf(arc_buf_hdr_t *hdr)
 			bzero((char *)cdata + csize, rounded - csize);
 			csize = rounded;
 		}
-		HDR_SET_COMPRESS(hdr, ZIO_COMPRESS_LZ4);
+		l2hdr->b_compress = ZIO_COMPRESS_LZ4;
 		l2hdr->b_asize = csize;
 		hdr->b_l1hdr.b_tmp_cdata = cdata;
 		ARCSTAT_BUMP(arcstat_l2_compress_successes);
@@ -6697,7 +6691,8 @@ l2arc_decompress_zio(zio_t *zio, arc_buf
 static void
 l2arc_release_cdata_buf(arc_buf_hdr_t *hdr)
 {
-	enum zio_compress comp = HDR_GET_COMPRESS(hdr);
+	ASSERT(HDR_HAS_L2HDR(hdr));
+	enum zio_compress comp = hdr->b_l2hdr.b_compress;
 
 	ASSERT(HDR_HAS_L1HDR(hdr));
 	ASSERT(comp == ZIO_COMPRESS_OFF || L2ARC_IS_VALID_COMPRESS(comp));

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/arc.h
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/arc.h	Sat Sep 12 09:28:02 2015	(r287705)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/arc.h	Sat Sep 12 09:56:23 2015	(r287706)
@@ -88,21 +88,6 @@ typedef enum arc_flags
 	/* Flags specifying whether optional hdr struct fields are defined */
 	ARC_FLAG_HAS_L1HDR		= 1 << 19,
 	ARC_FLAG_HAS_L2HDR		= 1 << 20,
-
-
-	/*
-	 * The arc buffer's compression mode is stored in the top 7 bits of the
-	 * flags field, so these dummy flags are included so that MDB can
-	 * interpret the enum properly.
-	 */
-	ARC_FLAG_COMPRESS_0		= 1 << 24,
-	ARC_FLAG_COMPRESS_1		= 1 << 25,
-	ARC_FLAG_COMPRESS_2		= 1 << 26,
-	ARC_FLAG_COMPRESS_3		= 1 << 27,
-	ARC_FLAG_COMPRESS_4		= 1 << 28,
-	ARC_FLAG_COMPRESS_5		= 1 << 29,
-	ARC_FLAG_COMPRESS_6		= 1 << 30
-
 } arc_flags_t;
 
 struct arc_buf {



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