Date: Thu, 20 Oct 2011 10:15:09 +0300 From: Andriy Gapon <avg@FreeBSD.org> To: Pawel Jakub Dawidek <pjd@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r226552 - head/sys/boot/zfs Message-ID: <4E9FCA7D.8010900@FreeBSD.org> In-Reply-To: <201110192340.p9JNebOH047566@svn.freebsd.org> References: <201110192340.p9JNebOH047566@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4E9FCA7D.8010900>