svn commit: r287283 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Andriy Gapon
avg at FreeBSD.org
Thu Sep 3 17:46:25 UTC 2015
On 29/08/2015 12:22, Xin LI wrote:
> 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.
Thank you very much for this fix!
I completely forgot why I had that code moved (and it was exactly to avoid the
buffer overrun) and so I thought that it was a non-essential difference from
upstream.
--
Andriy Gapon
More information about the svn-src-head
mailing list