[PATCH] scsi_debug: Fix off-by-one bug when unmapping region

[PATCH] scsi_debug: Fix off-by-one bug when unmapping region

[Lukas Czerner]

Currently it is possible to unmap one more block than user requested to
due to the off-by-one error in unmap_region(). This is probably due to
the fact that the end variable despite its name actually points to the
last block to unmap + 1. However in the condition it is handled as the
last block of the region to unmap.

The bug was not previously spotted probably due to the fact that the
region was not zeroed, which has changed with commit
be1dd78de5686c062bb3103f9e86d444a10ed783. With that commit we were able
to corrupt the ext4 file system on 256M scsi_debug device with LBPRZ
enabled using fstrim.

Since the 'end' semantic is the same in several functions there this
commit just fixes the condition to use the 'end' variable correctly in
that context.

Reported-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 drivers/scsi/scsi_debug.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 182d5a5..f4cc413 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2054,7 +2054,7 @@ static void unmap_region(sector_t lba, unsigned int len)
 		block = lba + alignment;
 		rem = do_div(block, granularity);
 
-		if (rem == 0 && lba + granularity <= end && block < map_size) {
+		if (rem == 0 && lba + granularity < end && block < map_size) {
 			clear_bit(block, map_storep);
 			if (scsi_debug_lbprz)
 				memset(fake_storep +
-- 
1.7.7.6

Re: [PATCH] scsi_debug: Fix off-by-one bug when unmapping region

[Martin K. Petersen]

>>>>> "Lukas" == Lukas Czerner <lczerner@redhat.com> writes:

Lukas> Currently it is possible to unmap one more block than user
Lukas> requested to due to the off-by-one error in unmap_region(). This
Lukas> is probably due to the fact that the end variable despite its
Lukas> name actually points to the last block to unmap + 1. However in
Lukas> the condition it is handled as the last block of the region to
Lukas> unmap.

Acked-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi_debug: Fix off-by-one bug when unmapping region

[Lukáš Czerner]

On Fri, 17 Aug 2012, Martin K. Petersen wrote:

> Date: Fri, 17 Aug 2012 12:11:50 -0400
> From: Martin K. Petersen <martin.petersen@oracle.com>
> To: Lukas Czerner <lczerner@redhat.com>
> Cc: linux-scsi@vger.kernel.org, JBottomley@parallels.com, tytso@mit.edu,
>     pbonzini@redhat.com, linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] scsi_debug: Fix off-by-one bug when unmapping region
> 
> >>>>> "Lukas" == Lukas Czerner <lczerner@redhat.com> writes:
> 
> Lukas> Currently it is possible to unmap one more block than user
> Lukas> requested to due to the off-by-one error in unmap_region(). This
> Lukas> is probably due to the fact that the end variable despite its
> Lukas> name actually points to the last block to unmap + 1. However in
> Lukas> the condition it is handled as the last block of the region to
> Lukas> unmap.
> 
> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
> 

I am still not seeing it in anywhere, is this going in any time soon
?

Thanks!
-Lukas

Re: [PATCH] scsi_debug: Fix off-by-one bug when unmapping region

[Douglas Gilbert]

On 12-08-17 12:11 PM, Martin K. Petersen wrote:
>>>>>> "Lukas" == Lukas Czerner <lczerner@redhat.com> writes:
>
> Lukas> Currently it is possible to unmap one more block than user
> Lukas> requested to due to the off-by-one error in unmap_region(). This
> Lukas> is probably due to the fact that the end variable despite its
> Lukas> name actually points to the last block to unmap + 1. However in
> Lukas> the condition it is handled as the last block of the region to
> Lukas> unmap.
>
> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>

Acked-by: Douglas Gilbert <dgilbert@interlog.com>

Re: [PATCH] scsi_debug: Fix off-by-one bug when unmapping region

[Lukáš Czerner]

On Wed, 5 Sep 2012, Douglas Gilbert wrote:

> Date: Wed, 05 Sep 2012 15:41:13 -0400
> From: Douglas Gilbert <dgilbert@interlog.com>
> To: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Lukas Czerner <lczerner@redhat.com>, linux-scsi@vger.kernel.org,
>     JBottomley@parallels.com, tytso@mit.edu, pbonzini@redhat.com,
>     linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] scsi_debug: Fix off-by-one bug when unmapping region
> 
> On 12-08-17 12:11 PM, Martin K. Petersen wrote:
> > > > > > > "Lukas" == Lukas Czerner <lczerner@redhat.com> writes:
> > 
> > Lukas> Currently it is possible to unmap one more block than user
> > Lukas> requested to due to the off-by-one error in unmap_region(). This
> > Lukas> is probably due to the fact that the end variable despite its
> > Lukas> name actually points to the last block to unmap + 1. However in
> > Lukas> the condition it is handled as the last block of the region to
> > Lukas> unmap.
> > 
> > Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
> 
> Acked-by: Douglas Gilbert <dgilbert@interlog.com>

James, could you please pick this one ?

Thanks!
-Lukas