linux-kernel - Re: btrfs zero divide

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 13 Aug 2013 18:32:59 +0200 (CEST)
From:	Geert Uytterhoeven <geert@...ux-m68k.org>
To:	Zach Brown <zab@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>
cc:	Andreas Schwab <schwab@...ux-m68k.org>,
	Josef Bacik <jbacik@...ionio.com>,
	Thorsten Glaser <tg@...bsd.de>, Joe Perches <joe@...ches.com>,
	Debian GNU/Linux m68k <debian-68k@...ts.debian.org>,
	linux-btrfs@...r.kernel.org,
	Linux Kernel Development <linux-kernel@...r.kernel.org>,
	David Woodhouse <David.Woodhouse@...el.com>,
	Chris Mason <chris.mason@...ionio.com>
Subject: Re: btrfs zero divide

On Fri, 9 Aug 2013, Zach Brown wrote:
> On Fri, Aug 09, 2013 at 02:26:36PM +0200, Andreas Schwab wrote:
> > Josef Bacik <jbacik@...ionio.com> 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 :-)

>From 7ccabf41beae38da514f3e09624219a9362375d7 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert@...ux-m68k.org>
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@...ux-m68k.org>
---
 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@...ux-m68k.org

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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/