On Tue, Aug 13, 2013 at 6:32 PM, Geert Uytterhoeven
<geert@xxxxxxxxxxxxxx> wrote:
> On Fri, 9 Aug 2013, Zach Brown wrote:
>> On Fri, Aug 09, 2013 at 02:26:36PM +0200, Andreas Schwab wrote:
>> > Josef Bacik <jbacik@xxxxxxxxxxxx> writes:
>> >
>> > > So stripe_len shouldn't be 0, if it is you have bigger problems :).
>> >
>> > The bigger problem is that stripe_nr is u64, this is completely bogus.
>> > The first operand of do_div must be u32. This goes through the whole
>> > file.
>
> This was introduced by commit 53b381b3abeb86f12787a6c40fee9b2f71edc23b
> ("Btrfs: RAID5 and RAID6"), which changed the divisor from
> map->stripe_len (struct map_lookup.stripe_len is int) to a 64-bit
> temporary.
>
>> Definitely. Can we get some typeof() tricks in the macros to have the
>> build fail if (when, evidently) someone gets it wrong?
>
> Not using typeof, as there are way too many callsites where int is used
> instead of u32.
>
> However, checking that sizeof() equals to 4 seems to work.
> Below is a patch for asm-generic, which is untested, but it works when
> adding the same checks to arch/m68k/include/asm/div64.h
>
> This is not something we just want to drop in, as it has the potential of
> breaking lots of things (yes, it breaks btrfs :-)
Found so far:
- Several calls to sector_div() in blkdev_issue_discard()
- Two calls to do_div() in sd_completed_bytes()
Some of these even operate on dividends that never exceed 32-bit, tss...
> >From 7ccabf41beae38da514f3e09624219a9362375d7 Mon Sep 17 00:00:00 2001
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Date: Tue, 13 Aug 2013 18:04:40 +0200
> Subject: [PATCH] asm-generic: Check divisor size in do_div()
>
> The second parameter of do_div() should be a 32-bit number.
> Enforce this using BUILD_BUG_ON().
>
> The first parameter of do_div() should be a 64-bit number,
> enforce this on 64-bit architectures, just like is done on 32-bit
> architectures.
>
> Signed-off-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> ---
> include/asm-generic/div64.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h
> index 8f4e319..69c0307 100644
> --- a/include/asm-generic/div64.h
> +++ b/include/asm-generic/div64.h
> @@ -19,12 +19,15 @@
>
> #include <linux/types.h>
> #include <linux/compiler.h>
> +#include <linux/bug.h>
>
> #if BITS_PER_LONG == 64
>
> # define do_div(n,base) ({ \
> uint32_t __base = (base); \
> uint32_t __rem; \
> + (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
> + BUILD_BUG_ON(sizeof(base) != 4); \
> __rem = ((uint64_t)(n)) % __base; \
> (n) = ((uint64_t)(n)) / __base; \
> __rem; \
> @@ -41,6 +44,7 @@ extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
> uint32_t __base = (base); \
> uint32_t __rem; \
> (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
> + BUILD_BUG_ON(sizeof(base) != 4); \
> if (likely(((n) >> 32) == 0)) { \
> __rem = (uint32_t)(n) % __base; \
> (n) = (uint32_t)(n) / __base; \
> --
> 1.7.9.5
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
