Skip to content

Commit 0ab19d0

Browse files
committed
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@d4cd038 Illumos issues: 6214 zpools going south https://www.illumos.org/issues/6214
1 parent e9ff0b2 commit 0ab19d0

File tree

2 files changed

+14
-34
lines changed
  • sys/cddl/contrib/opensolaris/uts/common/fs/zfs

2 files changed

+14
-34
lines changed

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

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -848,6 +848,7 @@ typedef struct l2arc_buf_hdr {
848848
uint64_t b_daddr; /* disk address, offset byte */
849849
/* real alloc'd buffer size depending on b_compress applied */
850850
int32_t b_asize;
851+
uint8_t b_compress;
851852

852853
list_node_t b_l2node;
853854
} l2arc_buf_hdr_t;
@@ -927,15 +928,6 @@ static arc_buf_hdr_t arc_eviction_hdr;
927928
#define HDR_HAS_L1HDR(hdr) ((hdr)->b_flags & ARC_FLAG_HAS_L1HDR)
928929
#define HDR_HAS_L2HDR(hdr) ((hdr)->b_flags & ARC_FLAG_HAS_L2HDR)
929930

930-
/* For storing compression mode in b_flags */
931-
#define HDR_COMPRESS_OFFSET 24
932-
#define HDR_COMPRESS_NBITS 7
933-
934-
#define HDR_GET_COMPRESS(hdr) ((enum zio_compress)BF32_GET(hdr->b_flags, \
935-
HDR_COMPRESS_OFFSET, HDR_COMPRESS_NBITS))
936-
#define HDR_SET_COMPRESS(hdr, cmp) BF32_SET(hdr->b_flags, \
937-
HDR_COMPRESS_OFFSET, HDR_COMPRESS_NBITS, (cmp))
938-
939931
/*
940932
* Other sizes
941933
*/
@@ -2226,7 +2218,7 @@ arc_buf_l2_cdata_free(arc_buf_hdr_t *hdr)
22262218
* separately compressed buffer, so there's nothing to free (it
22272219
* points to the same buffer as the arc_buf_t's b_data field).
22282220
*/
2229-
if (HDR_GET_COMPRESS(hdr) == ZIO_COMPRESS_OFF) {
2221+
if (hdr->b_l2hdr.b_compress == ZIO_COMPRESS_OFF) {
22302222
hdr->b_l1hdr.b_tmp_cdata = NULL;
22312223
return;
22322224
}
@@ -2235,12 +2227,12 @@ arc_buf_l2_cdata_free(arc_buf_hdr_t *hdr)
22352227
* There's nothing to free since the buffer was all zero's and
22362228
* compressed to a zero length buffer.
22372229
*/
2238-
if (HDR_GET_COMPRESS(hdr) == ZIO_COMPRESS_EMPTY) {
2230+
if (hdr->b_l2hdr.b_compress == ZIO_COMPRESS_EMPTY) {
22392231
ASSERT3P(hdr->b_l1hdr.b_tmp_cdata, ==, NULL);
22402232
return;
22412233
}
22422234

2243-
ASSERT(L2ARC_IS_VALID_COMPRESS(HDR_GET_COMPRESS(hdr)));
2235+
ASSERT(L2ARC_IS_VALID_COMPRESS(hdr->b_l2hdr.b_compress));
22442236

22452237
arc_buf_free_on_write(hdr->b_l1hdr.b_tmp_cdata,
22462238
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,
44644456
(vd = hdr->b_l2hdr.b_dev->l2ad_vdev) != NULL) {
44654457
devw = hdr->b_l2hdr.b_dev->l2ad_writing;
44664458
addr = hdr->b_l2hdr.b_daddr;
4467-
b_compress = HDR_GET_COMPRESS(hdr);
4459+
b_compress = hdr->b_l2hdr.b_compress;
44684460
b_asize = hdr->b_l2hdr.b_asize;
44694461
/*
44704462
* Lock out device removal.
@@ -6025,6 +6017,8 @@ l2arc_read_done(zio_t *zio)
60256017
if (cb->l2rcb_compress != ZIO_COMPRESS_OFF)
60266018
l2arc_decompress_zio(zio, hdr, cb->l2rcb_compress);
60276019
ASSERT(zio->io_data != NULL);
6020+
ASSERT3U(zio->io_size, ==, hdr->b_size);
6021+
ASSERT3U(BP_GET_LSIZE(&cb->l2rcb_bp), ==, hdr->b_size);
60286022

60296023
/*
60306024
* Check this survived the L2ARC journey.
@@ -6061,7 +6055,7 @@ l2arc_read_done(zio_t *zio)
60616055
ASSERT(!pio || pio->io_child_type == ZIO_CHILD_LOGICAL);
60626056

60636057
zio_nowait(zio_read(pio, cb->l2rcb_spa, &cb->l2rcb_bp,
6064-
buf->b_data, zio->io_size, arc_read_done, buf,
6058+
buf->b_data, hdr->b_size, arc_read_done, buf,
60656059
zio->io_priority, cb->l2rcb_flags, &cb->l2rcb_zb));
60666060
}
60676061
}
@@ -6378,7 +6372,7 @@ l2arc_write_buffers(spa_t *spa, l2arc_dev_t *dev, uint64_t target_sz,
63786372
* can't access without holding the ARC list locks
63796373
* (which we want to avoid during compression/writing).
63806374
*/
6381-
HDR_SET_COMPRESS(hdr, ZIO_COMPRESS_OFF);
6375+
hdr->b_l2hdr.b_compress = ZIO_COMPRESS_OFF;
63826376
hdr->b_l2hdr.b_asize = hdr->b_size;
63836377
hdr->b_l1hdr.b_tmp_cdata = hdr->b_l1hdr.b_buf->b_data;
63846378

@@ -6580,7 +6574,7 @@ l2arc_compress_buf(arc_buf_hdr_t *hdr)
65806574
l2arc_buf_hdr_t *l2hdr = &hdr->b_l2hdr;
65816575

65826576
ASSERT(HDR_HAS_L1HDR(hdr));
6583-
ASSERT(HDR_GET_COMPRESS(hdr) == ZIO_COMPRESS_OFF);
6577+
ASSERT3S(l2hdr->b_compress, ==, ZIO_COMPRESS_OFF);
65846578
ASSERT(hdr->b_l1hdr.b_tmp_cdata != NULL);
65856579

65866580
len = l2hdr->b_asize;
@@ -6592,7 +6586,7 @@ l2arc_compress_buf(arc_buf_hdr_t *hdr)
65926586
if (csize == 0) {
65936587
/* zero block, indicate that there's nothing to write */
65946588
zio_data_buf_free(cdata, len);
6595-
HDR_SET_COMPRESS(hdr, ZIO_COMPRESS_EMPTY);
6589+
l2hdr->b_compress = ZIO_COMPRESS_EMPTY;
65966590
l2hdr->b_asize = 0;
65976591
hdr->b_l1hdr.b_tmp_cdata = NULL;
65986592
ARCSTAT_BUMP(arcstat_l2_compress_zeros);
@@ -6610,7 +6604,7 @@ l2arc_compress_buf(arc_buf_hdr_t *hdr)
66106604
bzero((char *)cdata + csize, rounded - csize);
66116605
csize = rounded;
66126606
}
6613-
HDR_SET_COMPRESS(hdr, ZIO_COMPRESS_LZ4);
6607+
l2hdr->b_compress = ZIO_COMPRESS_LZ4;
66146608
l2hdr->b_asize = csize;
66156609
hdr->b_l1hdr.b_tmp_cdata = cdata;
66166610
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)
66976691
static void
66986692
l2arc_release_cdata_buf(arc_buf_hdr_t *hdr)
66996693
{
6700-
enum zio_compress comp = HDR_GET_COMPRESS(hdr);
6694+
ASSERT(HDR_HAS_L2HDR(hdr));
6695+
enum zio_compress comp = hdr->b_l2hdr.b_compress;
67016696

67026697
ASSERT(HDR_HAS_L1HDR(hdr));
67036698
ASSERT(comp == ZIO_COMPRESS_OFF || L2ARC_IS_VALID_COMPRESS(comp));

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

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -88,21 +88,6 @@ typedef enum arc_flags
8888
/* Flags specifying whether optional hdr struct fields are defined */
8989
ARC_FLAG_HAS_L1HDR = 1 << 19,
9090
ARC_FLAG_HAS_L2HDR = 1 << 20,
91-
92-
93-
/*
94-
* The arc buffer's compression mode is stored in the top 7 bits of the
95-
* flags field, so these dummy flags are included so that MDB can
96-
* interpret the enum properly.
97-
*/
98-
ARC_FLAG_COMPRESS_0 = 1 << 24,
99-
ARC_FLAG_COMPRESS_1 = 1 << 25,
100-
ARC_FLAG_COMPRESS_2 = 1 << 26,
101-
ARC_FLAG_COMPRESS_3 = 1 << 27,
102-
ARC_FLAG_COMPRESS_4 = 1 << 28,
103-
ARC_FLAG_COMPRESS_5 = 1 << 29,
104-
ARC_FLAG_COMPRESS_6 = 1 << 30
105-
10691
} arc_flags_t;
10792

10893
struct arc_buf {

0 commit comments

Comments
 (0)