Date: Thu, 3 Sep 2015 20:45:23 +0300 From: Andriy Gapon <avg@FreeBSD.org> To: Xin LI <delphij@FreeBSD.org>, src-committers@FreeBSD.org, svn-src-all@FreeBSD.org, svn-src-head@FreeBSD.org Subject: Re: svn commit: r287283 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs Message-ID: <55E88733.5010403@FreeBSD.org> In-Reply-To: <201508290922.t7T9MXhF007620@repo.freebsd.org> References: <201508290922.t7T9MXhF007620@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?55E88733.5010403>