From owner-svn-src-all@freebsd.org Sat Oct 3 11:24:48 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 95E4DA0F535; Sat, 3 Oct 2015 11:24:48 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 7A90B1FFF; Sat, 3 Oct 2015 11:24:48 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.70]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id t93BOmto065980; Sat, 3 Oct 2015 11:24:48 GMT (envelope-from mav@FreeBSD.org) Received: (from mav@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id t93BOmgU065979; Sat, 3 Oct 2015 11:24:48 GMT (envelope-from mav@FreeBSD.org) Message-Id: <201510031124.t93BOmgU065979@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mav set sender to mav@FreeBSD.org using -f From: Alexander Motin Date: Sat, 3 Oct 2015 11:24:48 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r288592 - stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs X-SVN-Group: stable-10 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 03 Oct 2015 11:24:48 -0000 Author: mav Date: Sat Oct 3 11:24:47 2015 New Revision: 288592 URL: https://svnweb.freebsd.org/changeset/base/288592 Log: MFC r287283 (by delphij): 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. Modified: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c ============================================================================== --- stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c Sat Oct 3 11:23:08 2015 (r288591) +++ stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c Sat Oct 3 11:24:47 2015 (r288592) @@ -6532,13 +6532,6 @@ l2arc_compress_buf(arc_buf_hdr_t *hdr) csize = zio_compress_data(ZIO_COMPRESS_LZ4, hdr->b_l1hdr.b_tmp_cdata, cdata, l2hdr->b_asize); - rounded = P2ROUNDUP(csize, - (size_t)1 << l2hdr->b_dev->l2ad_vdev->vdev_ashift); - if (rounded > csize) { - bzero((char *)cdata + csize, rounded - csize); - csize = rounded; - } - if (csize == 0) { /* zero block, indicate that there's nothing to write */ zio_data_buf_free(cdata, len); @@ -6547,11 +6540,19 @@ l2arc_compress_buf(arc_buf_hdr_t *hdr) hdr->b_l1hdr.b_tmp_cdata = NULL; ARCSTAT_BUMP(arcstat_l2_compress_zeros); return (B_TRUE); - } else if (csize > 0 && csize < len) { + } + + rounded = P2ROUNDUP(csize, + (size_t)1 << l2hdr->b_dev->l2ad_vdev->vdev_ashift); + if (rounded < len) { /* * Compression succeeded, we'll keep the cdata around for * writing and release it afterwards. */ + if (rounded > csize) { + bzero((char *)cdata + csize, rounded - csize); + csize = rounded; + } HDR_SET_COMPRESS(hdr, ZIO_COMPRESS_LZ4); l2hdr->b_asize = csize; hdr->b_l1hdr.b_tmp_cdata = cdata;