xfs
[Top] [All Lists]

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

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH] Fix off by one error in page_region_mask()
From: Lachlan McIlroy <lachlan@xxxxxxx>
Date: Fri, 05 Dec 2008 11:59:10 +1100
Cc: xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <4937FAED.7060503@xxxxxxxxxxx>
References: <49378B60.1060603@xxxxxxx> <4937FAED.7060503@xxxxxxxxxxx>
Reply-to: lachlan@xxxxxxx
User-agent: Thunderbird 2.0.0.18 (X11/20081105)
Eric Sandeen wrote:
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?

There was no observed bug - well nothing I can tie directly to this code.
I found this by inspection while investigating the page bitmap stuff.
We have a problem with ia64 going to 64K page size with filesystems that
use a filesystem sector size of 512 bytes - we don't have the granularity
we need in the bitmap.

I suppose it is possible this bug could indicate a page region is not up
to date when it actually is and we might re-read something from disk and
overwrite the more up to date in-memory version.


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>