[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/block/nvme: slba equal to nsze is out of bounds if nlb is
From: |
Keith Busch |
Subject: |
Re: [PATCH] hw/block/nvme: slba equal to nsze is out of bounds if nlb is 1-based |
Date: |
Sat, 10 Apr 2021 00:30:17 +0900 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
On Fri, Apr 09, 2021 at 01:55:01PM +0200, Klaus Jensen wrote:
> On Apr 9 20:05, Minwoo Im wrote:
> > On 21-04-09 13:14:02, Gollu Appalanaidu wrote:
> > > NSZE is the total size of the namespace in logical blocks. So the max
> > > addressable logical block is NLB minus 1. So your starting logical
> > > block is equal to NSZE it is a out of range.
> > >
> > > Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
> > > ---
> > > hw/block/nvme.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > > index 953ec64729..be9edb1158 100644
> > > --- a/hw/block/nvme.c
> > > +++ b/hw/block/nvme.c
> > > @@ -2527,7 +2527,7 @@ static uint16_t nvme_dsm(NvmeCtrl *n, NvmeRequest
> > > *req)
> > > uint64_t slba = le64_to_cpu(range[i].slba);
> > > uint32_t nlb = le32_to_cpu(range[i].nlb);
> > >
> > > - if (nvme_check_bounds(ns, slba, nlb)) {
> > > + if (nvme_check_bounds(ns, slba, nlb) || slba ==
> > > ns->id_ns.nsze) {
> >
> > This patch also looks like check the boundary about slba. Should it be
> > also checked inside of nvme_check_bounds() ?
>
> The catch here is that DSM is like the only command where the number of
> logical blocks is a 1s-based value. Otherwise we always have nlb > 0, which
> means that nvme_check_bounds() will always "do the right thing".
>
> My main gripe here is that (in my mind), by definition, a "zero length
> range" does not reference any LBAs at all. So how can it result in LBA Out
> of Range?
So what's the problem? If the request is to discard 0 blocks starting
from the last block, then that's valid. Is this patch actually fixing
anything?