btrfs zero divide

Commit Message

Joe Perches July 30, 2013, 9:05 p.m. UTC

On Tue, 2013-07-30 at 16:40 -0400, Josef Bacik wrote:
> So stripe_len shouldn't be 0, if it is you have bigger problems :).  Is this a
> corrupt fs or something?  If there was some sort of corruption that occured then
> I suppose stripe_len could be 0 and we'd need to catch that somewhere higher up
> the stack and error out.  Is there a way you could check and see if that's the
> case?  Thanks,

Maybe use a temporary check in do_div
Something like this maybe. (uncompiled/untested)
---
 include/asm-generic/div64.h | 43 +++++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 14 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Thorsten Glaser July 30, 2013, 9:25 p.m. UTC | #1

Josef Bacik dixit:

>So stripe_len shouldn't be 0, if it is you have bigger problems :).

?

>Is this a corrupt fs or something? If there was some sort of

I don’t think so, I can access and use that filesystem under 3.2
just fine (it’s what I created it under, too, so it’s possible
that it’s indeed corrupt and Linux 3.2 is just the same corrupt
to happen to make it work, e.g. wrong endianness used for stripe_len
which makes the upper 32 bit of that 64-bit value (usually 0) become
the lower 32 bit, or something like that).

I have access to that system, and it’s currently running as a
Debian/m68k buildd using said filesystem, but I can run commands
you tell me to diagnose/analyse it if it won’t get corrupted by
those.


Joe Perches dixit:

>Maybe use a temporary check in do_div

Mh. If nobody finds anything I’ll try that. (Doing things like
compiling a kernel and testing it takes about two days timeboxed
and some hours of active human effort, though, so I’d like to
avoid guessing. Plus it’ll disrupt running the Debian buildd…)

On second thoughts, this sort of check sounds like a good idea
to add to that file in general, depending on some debugging
CPPFLAG or Kconfig option. But I’m not the authority on that.

bye,
//mirabilos

Patch

diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h
index 8f4e319..cce75fe 100644
--- a/include/asm-generic/div64.h
+++ b/include/asm-generic/div64.h
@@ -19,16 +19,25 @@ 
 
 #include <linux/types.h>
 #include <linux/compiler.h>
+#include <linux/bug.h>
+#include <linux/printk.h>
 
 #if BITS_PER_LONG == 64
 
-# define do_div(n,base) ({					\
+# define do_div(n, base)					\
+({								\
 	uint32_t __base = (base);				\
 	uint32_t __rem;						\
-	__rem = ((uint64_t)(n)) % __base;			\
-	(n) = ((uint64_t)(n)) / __base;				\
+	if (__base == 0) {					\
+		WARN(1, "Attempted division by 0\n");		\
+		dump_stack();					\
+		__rem = 0;					\
+	} else {						\
+		__rem = ((uint64_t)(n)) % __base;		\
+		(n) = ((uint64_t)(n)) / __base;			\
+	}							\
 	__rem;							\
- })
+})
 
 #elif BITS_PER_LONG == 32
 
@@ -37,16 +46,22 @@  extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
 /* The unnecessary pointer compare is there
  * to check for type safety (n must be 64bit)
  */
-# define do_div(n,base) ({				\
-	uint32_t __base = (base);			\
-	uint32_t __rem;					\
-	(void)(((typeof((n)) *)0) == ((uint64_t *)0));	\
-	if (likely(((n) >> 32) == 0)) {			\
-		__rem = (uint32_t)(n) % __base;		\
-		(n) = (uint32_t)(n) / __base;		\
-	} else 						\
-		__rem = __div64_32(&(n), __base);	\
-	__rem;						\
+# define do_div(n, base)					\
+({								\
+	uint32_t __base = (base);				\
+	uint32_t __rem;						\
+	(void)(((typeof((n)) *)0) == ((uint64_t *)0));		\
+	if (__base == 0) {					\
+		WARN(1, "Attempted division by 0\n");		\
+		dump_stack();					\
+		__rem = 0;					\
+	} else if (likely(((n) >> 32) == 0)) {			\
+		__rem = (uint32_t)(n) % __base;			\
+		(n) = (uint32_t)(n) / __base;			\
+	} else {						\
+		__rem = __div64_32(&(n), __base);		\
+	}							\
+	__rem;							\
  })
 
 #else /* BITS_PER_LONG == ?? */