xfs
[Top] [All Lists]

Re: [PATCH] Fix off by one error in page_region_mask()

To: lachlan@xxxxxxx
Subject: Re: [PATCH] Fix off by one error in page_region_mask()
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Thu, 04 Dec 2008 09:44:45 -0600
Cc: xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <49378B60.1060603@xxxxxxx>
References: <49378B60.1060603@xxxxxxx>
User-agent: Thunderbird 2.0.0.18 (X11/20081119)
Lachlan McIlroy wrote:
> final is calculated to be the last bit to set (ie inclusive) but when we
> do the mask shifting final really needs to be first bit not to set.
> 
> For example if first and final are both bit 0 (ie only first bit to be set)
> then mask is completely shifted and becomes all zeroes.
> 
> Or if first is 0 and final is 63 then the mask is shifted one bit when it
> shouldn't be shifted at all.

Lachlan, what's the end result of this bug?  What's the broken behavior?

Thanks,
-Eric

> --- xfs-fix.orig/fs/xfs/linux-2.6/xfs_buf.c
> +++ xfs-fix/fs/xfs/linux-2.6/xfs_buf.c
> @@ -129,15 +129,17 @@ page_region_mask(
>       int             first, final;
> 
>       first = BTOPR(offset);
> -     final = BTOPRT(offset + length - 1);
> -     first = min(first, final);
> +     final = BTOPRT(offset + length);
> +
> +     if (first >= final)
> +             return 0UL;
> 
>       mask = ~0UL;
>       mask <<= BITS_PER_LONG - (final - first);
>       mask >>= BITS_PER_LONG - (final);
> 
>       ASSERT(offset + length <= PAGE_CACHE_SIZE);
> -     ASSERT((final - first) < BITS_PER_LONG && (final - first) >= 0);
> +     ASSERT((final - first) <= BITS_PER_LONG && (final - first) > 0);
> 
>       return mask;
>   }
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

<Prev in Thread] Current Thread [Next in Thread>