d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

MFV r287699: 6214 zpools going south

Browse Source 
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@d4cd038c92

Illumos issues:

    6214 zpools going south
    https://www.illumos.org/issues/6214
This commit is contained in:
Xin LI 2015-09-12 09:56:23 +00:00
commit 653809335f
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=287706
2 changed files with 14 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
View File

@ -848,6 +848,7 @@ typedef struct l2arc_buf_hdr {
uint64_t b_daddr; /* disk address, offset byte */ uint64_t b_daddr; /* disk address, offset byte */
/* real alloc'd buffer size depending on b_compress applied */ /* real alloc'd buffer size depending on b_compress applied */
int32_t b_asize; int32_t b_asize;
uint8_t b_compress;
list_node_t b_l2node; list_node_t b_l2node;
} l2arc_buf_hdr_t; } 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_L1HDR(hdr) ((hdr)->b_flags & ARC_FLAG_HAS_L1HDR)
#define HDR_HAS_L2HDR(hdr) ((hdr)->b_flags & ARC_FLAG_HAS_L2HDR) #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 * 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 * separately compressed buffer, so there's nothing to free (it
* points to the same buffer as the arc_buf_t's b_data field). * 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; hdr->b_l1hdr.b_tmp_cdata = NULL;
return; 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 * There's nothing to free since the buffer was all zero's and
* compressed to a zero length buffer. * 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); ASSERT3P(hdr->b_l1hdr.b_tmp_cdata, ==, NULL);
return; 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, arc_buf_free_on_write(hdr->b_l1hdr.b_tmp_cdata,
hdr->b_size, zio_data_buf_free); hdr->b_size, zio_data_buf_free);
@ -4464,7 +4456,7 @@ arc_read(zio_t *pio, spa_t *spa, const blkptr_t *bp, arc_done_func_t *done,
(vd = hdr->b_l2hdr.b_dev->l2ad_vdev) != NULL) { (vd = hdr->b_l2hdr.b_dev->l2ad_vdev) != NULL) {
devw = hdr->b_l2hdr.b_dev->l2ad_writing; devw = hdr->b_l2hdr.b_dev->l2ad_writing;
addr = hdr->b_l2hdr.b_daddr; 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; b_asize = hdr->b_l2hdr.b_asize;
/* /*
* Lock out device removal. * Lock out device removal.
@ -6025,6 +6017,8 @@ l2arc_read_done(zio_t *zio)
if (cb->l2rcb_compress != ZIO_COMPRESS_OFF) if (cb->l2rcb_compress != ZIO_COMPRESS_OFF)
l2arc_decompress_zio(zio, hdr, cb->l2rcb_compress); l2arc_decompress_zio(zio, hdr, cb->l2rcb_compress);
ASSERT(zio->io_data != NULL); 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. * 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); ASSERT(!pio || pio->io_child_type == ZIO_CHILD_LOGICAL);
zio_nowait(zio_read(pio, cb->l2rcb_spa, &cb->l2rcb_bp, 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)); zio->io_priority, cb->l2rcb_flags, &cb->l2rcb_zb));
} }
} }
@ -6378,7 +6372,7 @@ l2arc_write_buffers(spa_t *spa, l2arc_dev_t *dev, uint64_t target_sz,
* can't access without holding the ARC list locks * can't access without holding the ARC list locks
* (which we want to avoid during compression/writing). * (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_l2hdr.b_asize = hdr->b_size;
hdr->b_l1hdr.b_tmp_cdata = hdr->b_l1hdr.b_buf->b_data; 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; l2arc_buf_hdr_t *l2hdr = &hdr->b_l2hdr;
ASSERT(HDR_HAS_L1HDR(hdr)); 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); ASSERT(hdr->b_l1hdr.b_tmp_cdata != NULL);
len = l2hdr->b_asize; len = l2hdr->b_asize;
@ -6592,7 +6586,7 @@ l2arc_compress_buf(arc_buf_hdr_t *hdr)
if (csize == 0) { if (csize == 0) {
/* zero block, indicate that there's nothing to write */ /* zero block, indicate that there's nothing to write */
zio_data_buf_free(cdata, len); zio_data_buf_free(cdata, len);
HDR_SET_COMPRESS(hdr, ZIO_COMPRESS_EMPTY); l2hdr->b_compress = ZIO_COMPRESS_EMPTY;
l2hdr->b_asize = 0; l2hdr->b_asize = 0;
hdr->b_l1hdr.b_tmp_cdata = NULL; hdr->b_l1hdr.b_tmp_cdata = NULL;
ARCSTAT_BUMP(arcstat_l2_compress_zeros); ARCSTAT_BUMP(arcstat_l2_compress_zeros);
@ -6610,7 +6604,7 @@ l2arc_compress_buf(arc_buf_hdr_t *hdr)
bzero((char *)cdata + csize, rounded - csize); bzero((char *)cdata + csize, rounded - csize);
csize = rounded; csize = rounded;
} }
HDR_SET_COMPRESS(hdr, ZIO_COMPRESS_LZ4); l2hdr->b_compress = ZIO_COMPRESS_LZ4;
l2hdr->b_asize = csize; l2hdr->b_asize = csize;
hdr->b_l1hdr.b_tmp_cdata = cdata; hdr->b_l1hdr.b_tmp_cdata = cdata;
ARCSTAT_BUMP(arcstat_l2_compress_successes); ARCSTAT_BUMP(arcstat_l2_compress_successes);
@ -6697,7 +6691,8 @@ l2arc_decompress_zio(zio_t *zio, arc_buf_hdr_t *hdr, enum zio_compress c)
static void static void
l2arc_release_cdata_buf(arc_buf_hdr_t *hdr) 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(HDR_HAS_L1HDR(hdr));
ASSERT(comp == ZIO_COMPRESS_OFF || L2ARC_IS_VALID_COMPRESS(comp)); ASSERT(comp == ZIO_COMPRESS_OFF || L2ARC_IS_VALID_COMPRESS(comp));

15
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/arc.h
View File

@ -88,21 +88,6 @@ typedef enum arc_flags
/* Flags specifying whether optional hdr struct fields are defined */ /* Flags specifying whether optional hdr struct fields are defined */
ARC_FLAG_HAS_L1HDR = 1 << 19, ARC_FLAG_HAS_L1HDR = 1 << 19,
ARC_FLAG_HAS_L2HDR = 1 << 20, 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; } arc_flags_t;
struct arc_buf { struct arc_buf {