Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <155000668075.348031.9371497273408112600.stgit@dwillia2-desk3.amr.corp.intel.com>
 <155000669646.348031.16690970886357498896.stgit@dwillia2-desk3.amr.corp.intel.com>
 <20190213102202.GA13313@quack2.suse.cz>
In-Reply-To: <20190213102202.GA13313@quack2.suse.cz>
From:   Dan Williams <dan.j.williams@intel.com>
Date:   Wed, 13 Feb 2019 08:49:47 -0800
Message-ID: <CAPcyv4jwMx2ohs9QobTohL8jxc5G74XhmY5QFpmiVF9siWLuwA@mail.gmail.com>
Subject: Re: [PATCH 3/7] dax: Check the end of the block-device capacity with dax_direct_access()
To:     Jan Kara <jack@suse.cz>
Cc:     linux-nvdimm <linux-nvdimm@lists.01.org>,
        "Darrick J. Wong" <darrick.wong@oracle.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Vishal L Verma <vishal.l.verma@intel.com>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Feb 13, 2019 at 2:22 AM Jan Kara <jack@suse.cz> wrote:
>
> On Tue 12-02-19 13:24:56, Dan Williams wrote:
> > The checks in __bdev_dax_supported() helped mitigate a potential data
> > corruption bug in the pmem driver's handling of section alignment
> > padding. Strengthen the checks, including checking the end of the range,
> > to validate the dev_pagemap, Xarray entries, and sector-to-pfn
> > translation established for pmem namespaces.
> >
> > Cc: Jan Kara <jack@suse.cz>
> > Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >  drivers/dax/super.c |   39 +++++++++++++++++++++++++++++----------
> >  1 file changed, 29 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/dax/super.c b/drivers/dax/super.c
> > index 6e928f37d084..a27395cfcec6 100644
> > --- a/drivers/dax/super.c
> > +++ b/drivers/dax/super.c
> > @@ -86,12 +86,14 @@ bool __bdev_dax_supported(struct block_device *bdev, int blocksize)
> >  {
> >       struct dax_device *dax_dev;
> >       bool dax_enabled = false;
> > +     pgoff_t pgoff, pgoff_end;
> >       struct request_queue *q;
> > -     pgoff_t pgoff;
> > -     int err, id;
> > -     pfn_t pfn;
> > -     long len;
> >       char buf[BDEVNAME_SIZE];
> > +     void *kaddr, *end_kaddr;
> > +     pfn_t pfn, end_pfn;
> > +     sector_t last_page;
> > +     long len, len2;
> > +     int err, id;
> >
> >       if (blocksize != PAGE_SIZE) {
> >               pr_debug("%s: error: unsupported blocksize for dax\n",
> > @@ -113,6 +115,15 @@ bool __bdev_dax_supported(struct block_device *bdev, int blocksize)
> >               return false;
> >       }
> >
> > +     last_page = ALIGN_DOWN(part_nr_sects_read(bdev->bd_part)
> > +                     - PAGE_SIZE / 512, PAGE_SIZE / 512);
>
> Why not just (i_size_read(bdev->bd_inode) - 1) >> PAGE_SHIFT?

Because that would be too elegant and straightforward?

> Otherwise the patch looks good to me.

Thanks, I'll fix up the last page calculation.