linux-ext4 - Re: [PATCH] e2fsprogs: m68k bitops fixes

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Sun, 16 Dec 2012 22:39:32 +0100
From:	Mikael Pettersson <mikpe@...uu.se>
To:	Mikael Pettersson <mikpe@...uu.se>
Cc:	linux-ext4@...r.kernel.org, Andreas Schwab <schwab@...ux-m68k.org>
Subject: Re: [PATCH] e2fsprogs: m68k bitops fixes

Mikael Pettersson writes:
 > This patch fixes two problems with the m68k-specific bitops
 > in e2fsprogs.
 > 
 > The first problem is that tst_bitops SEGVs:
 > 
 > LD_LIBRARY_PATH=../../lib DYLD_LIBRARY_PATH=../../lib ./tst_bitops
 > make[1]: *** [check] Segmentation fault
 > 
 > Post-mortem debugging shows that this occurs in the m68k-specific
 > ext2fs_set_bit() when called with BIG_TEST_BIT as bit number:
 > 
 > Core was generated by `./tst_bitops'.
 > Program terminated with signal 11, Segmentation fault.
 > #0  0x80000820 in ext2fs_set_bit (addr=0xc01ba008, nr=2147483690) at ../../lib/ext2fs/bitops.h:357
 > 357             __asm__ __volatile__ ("bfset %2@{%1:#1}; sne %0"
 > (gdb) bt
 > #0  0x80000820 in ext2fs_set_bit (addr=0xc01ba008, nr=2147483690) at ../../lib/ext2fs/bitops.h:357
 > #1  main (argc=1, argv=0xefc31174) at tst_bitops.c:112
 > (gdb) up
 > #1  main (argc=1, argv=0xefc31174) at tst_bitops.c:112
 > 112             ext2fs_set_bit(BIG_TEST_BIT, bigarray);
 > 
 > The SEGV occurs because bfset et al interpret the bit number as a
 > _signed_ value, so 2147483690 is actually -2147483606, causing an
 > access to a location about 1/4 GB before 'bigarray'.
 > 
 > To fix this I adjust the base address to point to the correct byte,
 > and limit the bit number to be within that byte, before performing
 > the bfset (and likewise in the clear_bit and test_bit functions).
 > This is similar to what the x86-specific bitops do.
 > 
 > With this in place the SEGV is gone and the tests pass, but there
 > is still some strange output from tst_bitops:
 > 
 > big bit number (2147483690) test: 0, expected 4
 > big bit number (2147483690) test: 4, expected 0
 > ext2fs_set_bit big_test successful
 > 
 > The test and expected values differ, yet the test case doesn't fail.
 > 
 > A second look at the m68k bitop asm() statements makes the reason clear:
 > there are _no_ dependencies whatsoever between the asm()s and the
 > memory locations being accessed (a dependency on the address isn't
 > enough to create a dependency on memory locations reachable from it,
 > and __volatile__ doesn't create memory dependencies either).  The
 > simplest fix is to add "memory" clobbers (the kernel's bitops also
 > have memory clobbers), and with those in place the tests still pass
 > and the debug output looks sane:
 > 
 > big bit number (2147483690) test: 4, expected 4
 > big bit number (2147483690) test: 0, expected 0
 > ext2fs_set_bit big_test successful
 > 
 > Tested on m68k-linux with e2fsprogs-1.42.6 and gcc-4.6.3.

Andreas Schwab suggested that there was no longer an advantage
to using inline asm for these operations, so this new patch
deletes the m68k-specific code from lib/ext2fs/bitops.h (leaving
only 32-bit x86 to still use inline asm there).

Tested on m68k-linux with e2fsprogs-1.42.6 and gcc-4.6.3 as before.
All tests pass and the debug output looks sane.

I compared the e2fsck binaries from the previous build with this
one.  They had identical .text sizes, and almost the same number
of bit field instructions (obviously compiler-generated), so this
change should have no serious performance implications.

Signed-off-by: Mikael Pettersson <mikpe@...uu.se>
Cc: Andreas Schwab <schwab@...ux-m68k.org>
---
 lib/ext2fs/bitops.h |   39 +--------------------------------------
 1 files changed, 1 insertion(+), 38 deletions(-)

--- e2fsprogs-1.42.6/lib/ext2fs/bitops.h.~1~	2012-09-10 04:53:00.000000000 +0200
+++ e2fsprogs-1.42.6/lib/ext2fs/bitops.h	2012-12-16 17:41:09.000000000 +0100
@@ -200,7 +200,7 @@ extern errcode_t ext2fs_find_first_zero_
  */
 #ifdef NO_INLINE_FUNCS
 #if (defined(__GNUC__) && (defined(__i386__) || defined(__i486__) || \
-			   defined(__i586__) || defined(__mc68000__)))
+			   defined(__i586__)))
 	/* This prevents bitops.c from trying to include the C */
 	/* function version of these functions */
 #define _EXT2_HAVE_ASM_BITOPS_
@@ -345,43 +345,6 @@ _INLINE_ __u16 ext2fs_swab16(__u16 val)
 
 #endif	/* i386 */
 
-#if ((defined __GNUC__) && !defined(_EXT2_USE_C_VERSIONS_) && \
-     (defined(__mc68000__)))
-
-#define _EXT2_HAVE_ASM_BITOPS_
-
-_INLINE_ int ext2fs_set_bit(unsigned int nr,void * addr)
-{
-	char retval;
-
-	__asm__ __volatile__ ("bfset %2@{%1:#1}; sne %0"
-	     : "=d" (retval) : "d" (nr^7), "a" (addr));
-
-	return retval;
-}
-
-_INLINE_ int ext2fs_clear_bit(unsigned int nr, void * addr)
-{
-	char retval;
-
-	__asm__ __volatile__ ("bfclr %2@{%1:#1}; sne %0"
-	     : "=d" (retval) : "d" (nr^7), "a" (addr));
-
-	return retval;
-}
-
-_INLINE_ int ext2fs_test_bit(unsigned int nr, const void * addr)
-{
-	char retval;
-
-	__asm__ __volatile__ ("bftst %2@{%1:#1}; sne %0"
-	     : "=d" (retval) : "d" (nr^7), "a" (addr));
-
-	return retval;
-}
-
-#endif /* __mc68000__ */
-
 
 #if !defined(_EXT2_HAVE_ASM_SWAB_)
 
--
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