From owner-svn-src-all@FreeBSD.ORG Thu Oct 20 07:15:13 2011 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 354F0106564A; Thu, 20 Oct 2011 07:15:13 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id E079D8FC0C; Thu, 20 Oct 2011 07:15:11 +0000 (UTC) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id KAA12777; Thu, 20 Oct 2011 10:15:10 +0300 (EEST) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1RGmqM-000JGS-58; Thu, 20 Oct 2011 10:15:10 +0300 Message-ID: <4E9FCA7D.8010900@FreeBSD.org> Date: Thu, 20 Oct 2011 10:15:09 +0300 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:7.0.1) Gecko/20111002 Thunderbird/7.0.1 MIME-Version: 1.0 To: Pawel Jakub Dawidek References: <201110192340.p9JNebOH047566@svn.freebsd.org> In-Reply-To: <201110192340.p9JNebOH047566@svn.freebsd.org> X-Enigmail-Version: undefined Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r226552 - head/sys/boot/zfs X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 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: Thu, 20 Oct 2011 07:15:13 -0000 on 20/10/2011 02:40 Pawel Jakub Dawidek said the following: > Author: pjd > Date: Wed Oct 19 23:40:37 2011 > New Revision: 226552 > URL: http://svn.freebsd.org/changeset/base/226552 > > Log: > Never pass NULL block pointer when reading. This is neither expected nor > handled by lower layers like vdev_raidz, which uses bp for checksum > verification. This bug could lead to NULL pointer reference and resets > during boot. > > MFC after: 3 days > > Modified: > head/sys/boot/zfs/zfsimpl.c > > Modified: head/sys/boot/zfs/zfsimpl.c > ============================================================================== > --- head/sys/boot/zfs/zfsimpl.c Wed Oct 19 23:37:30 2011 (r226551) > +++ head/sys/boot/zfs/zfsimpl.c Wed Oct 19 23:40:37 2011 (r226552) > @@ -988,7 +988,8 @@ zio_read_gang(spa_t *spa, const blkptr_t > break; > if (!vdev || !vdev->v_read) > return (EIO); > - if (vdev->v_read(vdev, NULL, &zio_gb, offset, SPA_GANGBLOCKSIZE)) > + > + if (vdev->v_read(vdev, bp, &zio_gb, offset, SPA_GANGBLOCKSIZE)) > return (EIO); > > for (i = 0; i < SPA_GBH_NBLKPTRS; i++) { I believe that this commit is incorrect. It does appear to do the right thing for raidz, but I believe that it breaks the simple case when v_read is vdev_read_phys. Now because the bp argument is not NULL, the latter will try to read BP_GET_PSIZE(bp) bytes instead of SPA_GANGBLOCKSIZE bytes. I believe that my patch that I shared (much) earlier handles this issue correctly and integrally: http://article.gmane.org/gmane.os.freebsd.devel.file-systems/13130 Did you find anything wrong with it? If not, then maybe we can get back to discussing it? -- Andriy Gapon