| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <47BC3CD2.4060808@redhat.com>
Date: Wed, 20 Feb 2008 08:44:34 -0600
From: Eric Sandeen <sandeen@...hat.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
CC: tytso@....edu, cmm@...ibm.com, linux-ext4@...r.kernel.org
Subject: Re: [PATCH] ext4: ext4_find_next_zero_bit needs an aligned address
on some arch
Aneesh Kumar K.V wrote:
> ext4_find_next_zero_bit and ext4_find_next_bit needs a long aligned
> address on x8_64. Add mb_find_next_zero_bit and mb_find_next_bit
> and use them in the mballoc.
>
> Fix: https://bugzilla.redhat.com/show_bug.cgi?id=433286
>
> Eric Sandeen debugged the problem and suggested the fix.
Thanks for fixing this up, Aneesh.
Thanks for getting rid of the magic looks-like-a-function-but-isn't
#define, too. :)
The testcase I reproduced this with (basically just rebuilding a kernel
src.rpm on ext4) seems to pass with change.
As we had it, the direct call of find_next_zero_bit seemed to almost do
the right thing, except it scanned more bits than we asked for, up to 64
bits' worth, and so a) wandered off our page and b) returned an answer
that was > the size we asked it to scan in some cases.
(It's unfortunate that the alignment code for this got taken out in the
first place, only to put it back now, but I guess I didn't speak up
then, either...)
Really, I think the core bitmap functions could use some fixing, or at
least some warnings if it's given an address it can't cope with properly.
But for now....
Acked-by: Eric Sandeen <sandeen@...hat.com>
Thanks,
-Eric
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@...ux.vnet.ibm.com>
> CC:Eric Sandeen <sandeen@...hat.com>
> ---
> fs/ext4/mballoc.c | 62 ++++++++++++++++++++++++++++++++++------------------
> 1 files changed, 40 insertions(+), 22 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 89772b9..ccddd21 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -627,21 +627,19 @@ static ext4_fsblk_t ext4_grp_offs_to_block(struct super_block *sb,
> return block;
> }
>
> +static inline void *mb_correct_addr_and_bit(int *bit, void *addr)
> +{
> #if BITS_PER_LONG == 64
> -#define mb_correct_addr_and_bit(bit, addr) \
> -{ \
> - bit += ((unsigned long) addr & 7UL) << 3; \
> - addr = (void *) ((unsigned long) addr & ~7UL); \
> -}
> + *bit += ((unsigned long) addr & 7UL) << 3;
> + addr = (void *) ((unsigned long) addr & ~7UL);
> #elif BITS_PER_LONG == 32
> -#define mb_correct_addr_and_bit(bit, addr) \
> -{ \
> - bit += ((unsigned long) addr & 3UL) << 3; \
> - addr = (void *) ((unsigned long) addr & ~3UL); \
> -}
> + *bit += ((unsigned long) addr & 3UL) << 3;
> + addr = (void *) ((unsigned long) addr & ~3UL);
> #else
> #error "how many bits you are?!"
> #endif
> + return addr;
> +}
>
> static inline int mb_test_bit(int bit, void *addr)
> {
> @@ -649,34 +647,54 @@ static inline int mb_test_bit(int bit, void *addr)
> * ext4_test_bit on architecture like powerpc
> * needs unsigned long aligned address
> */
> - mb_correct_addr_and_bit(bit, addr);
> + addr = mb_correct_addr_and_bit(&bit, addr);
> return ext4_test_bit(bit, addr);
> }
>
> static inline void mb_set_bit(int bit, void *addr)
> {
> - mb_correct_addr_and_bit(bit, addr);
> + addr = mb_correct_addr_and_bit(&bit, addr);
> ext4_set_bit(bit, addr);
> }
>
> static inline void mb_set_bit_atomic(spinlock_t *lock, int bit, void *addr)
> {
> - mb_correct_addr_and_bit(bit, addr);
> + addr = mb_correct_addr_and_bit(&bit, addr);
> ext4_set_bit_atomic(lock, bit, addr);
> }
>
> static inline void mb_clear_bit(int bit, void *addr)
> {
> - mb_correct_addr_and_bit(bit, addr);
> + addr = mb_correct_addr_and_bit(&bit, addr);
> ext4_clear_bit(bit, addr);
> }
>
> static inline void mb_clear_bit_atomic(spinlock_t *lock, int bit, void *addr)
> {
> - mb_correct_addr_and_bit(bit, addr);
> + addr = mb_correct_addr_and_bit(&bit, addr);
> ext4_clear_bit_atomic(lock, bit, addr);
> }
>
> +static inline int mb_find_next_zero_bit(void *addr, int max, int start)
> +{
> + int fix = 0;
> + addr = mb_correct_addr_and_bit(&fix, addr);
> + max += fix;
> + start += fix;
> +
> + return ext4_find_next_zero_bit(addr, max, start) - fix;
> +}
> +
> +static inline int mb_find_next_bit(void *addr, int max, int start)
> +{
> + int fix = 0;
> + addr = mb_correct_addr_and_bit(&fix, addr);
> + max += fix;
> + start += fix;
> +
> + return ext4_find_next_bit(addr, max, start) - fix;
> +}
> +
> static void *mb_find_buddy(struct ext4_buddy *e4b, int order, int *max)
> {
> char *bb;
> @@ -946,12 +964,12 @@ static void ext4_mb_generate_buddy(struct super_block *sb,
>
> /* initialize buddy from bitmap which is aggregation
> * of on-disk bitmap and preallocations */
> - i = ext4_find_next_zero_bit(bitmap, max, 0);
> + i = mb_find_next_zero_bit(bitmap, max, 0);
> grp->bb_first_free = i;
> while (i < max) {
> fragments++;
> first = i;
> - i = ext4_find_next_bit(bitmap, max, i);
> + i = mb_find_next_bit(bitmap, max, i);
> len = i - first;
> free += len;
> if (len > 1)
> @@ -959,7 +977,7 @@ static void ext4_mb_generate_buddy(struct super_block *sb,
> else
> grp->bb_counters[0]++;
> if (i < max)
> - i = ext4_find_next_zero_bit(bitmap, max, i);
> + i = mb_find_next_zero_bit(bitmap, max, i);
> }
> grp->bb_fragments = fragments;
>
> @@ -1783,7 +1801,7 @@ static void ext4_mb_simple_scan_group(struct ext4_allocation_context *ac,
> buddy = mb_find_buddy(e4b, i, &max);
> BUG_ON(buddy == NULL);
>
> - k = ext4_find_next_zero_bit(buddy, max, 0);
> + k = mb_find_next_zero_bit(buddy, max, 0);
> BUG_ON(k >= max);
>
> ac->ac_found++;
> @@ -1823,7 +1841,7 @@ static void ext4_mb_complex_scan_group(struct ext4_allocation_context *ac,
> i = e4b->bd_info->bb_first_free;
>
> while (free && ac->ac_status == AC_STATUS_CONTINUE) {
> - i = ext4_find_next_zero_bit(bitmap,
> + i = mb_find_next_zero_bit(bitmap,
> EXT4_BLOCKS_PER_GROUP(sb), i);
> if (i >= EXT4_BLOCKS_PER_GROUP(sb)) {
> /*
> @@ -3744,10 +3762,10 @@ static noinline int ext4_mb_release_inode_pa(struct ext4_buddy *e4b,
> }
>
> while (bit < end) {
> - bit = ext4_find_next_zero_bit(bitmap_bh->b_data, end, bit);
> + bit = mb_find_next_zero_bit(bitmap_bh->b_data, end, bit);
> if (bit >= end)
> break;
> - next = ext4_find_next_bit(bitmap_bh->b_data, end, bit);
> + next = mb_find_next_bit(bitmap_bh->b_data, end, bit);
> if (next > end)
> next = end;
> start = group * EXT4_BLOCKS_PER_GROUP(sb) + bit +
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists