possible memory corruption prior to r268075 [Was: r268075 (MFV r267565) undone FreeBSD-specific changes]

Andriy Gapon avg at FreeBSD.org
Wed Sep 17 09:29:55 UTC 2014


While looking at the changes discussed in the previous email another issue has
caught my attention.
Both l2arc_compress_buf() and zio_write_bp_init() allocate a buffer for
compressed data using an original / logical buffer size for the size.  The
buffer allocation is done using zio_buf_alloc() and zio_data_buf_alloc().  So,
the size would be increased to one of the standard buffer sizes if needed.  The
standard buffer sizes are: 512B - 4KB with a step of 512B, 4KB - 8KB with a step
of 1KB, 8KB - 16KB with a step of 2KB, 16KB - 128KB with a step of 4KB.
The size for compressed data should always be sufficient because the compressed
data is supposed to never be larger than the original data even taking into
account rounding up to a multiple of 512B.  In other words, psize <= lsize.

But in FreeBSD prior to commit r268075 we used to round up the compressed size
to a multiple of vdev sector size.

Let's consider the following example. Original lsize is 10KB, so that data is
stored in a 10KB buffer, because this is one of the standard buffer sizes.
Thus, a 10KB buffer would also be allocated for the compressed data. Let's
assume that the data gets compressed to 8KB + 1B size (reduction by almost 20%).
 Let's further assume that the sector size is 4KB.  So the compressed size would
be rounded up to 12KB.  That is, psize > lsize.  The code zeroes out a trailing
part of the buffer, so it would overwrite 2KB beyond the end of the buffer.

I think that in general our code had and may still has a bit of confusion about
psize (possibly compressed data size) and asize (actual disk allocation size).
I think that psize should always be not greater than lsize.  While asize is
always not less than psize.
  psize <= lsize
  psize <= asize
Obviously, there is no rule on the relation between lsize and asize.
But our code used to force ashift on psize which made it kind of asize, or
something in between the classic psize and asize.

What do you think?

-- 
Andriy Gapon


More information about the zfs-devel mailing list